fix(quota): address edge cases from code review

- Add isPausingAccount disabled state to pause/resume dropdown (#30)
- Add rapid click prevention guard in cliproxy.tsx (#31)
- Add request deduplication via pendingFetches Map in quota-manager (#8)
- Add JSON parse error handler middleware in web-server (#26)
This commit is contained in:
kaitranntt
2026-01-06 13:05:45 -05:00
parent 4ad7292700
commit a32fdc8cfb
8 changed files with 81 additions and 29 deletions
+41 -25
View File
@@ -36,6 +36,9 @@ interface CacheEntry {
const CACHE_TTL_MS = 30_000; // 30 seconds
const quotaCache = new Map<string, CacheEntry>();
// Request deduplication: track in-flight fetch promises to avoid parallel duplicate requests
const pendingFetches = new Map<string, Promise<QuotaResult>>();
function getCacheKey(provider: CLIProxyProvider, accountId: string): string {
return `${provider}:${accountId}`;
}
@@ -76,6 +79,39 @@ export function clearQuotaCache(): void {
quotaCache.clear();
}
/**
* Fetch quota with request deduplication
* If a fetch for this account is already in progress, return the existing promise
*/
async function fetchQuotaWithDedup(
provider: CLIProxyProvider,
accountId: string
): Promise<QuotaResult> {
const key = getCacheKey(provider, accountId);
// Check if fetch already in progress
const pending = pendingFetches.get(key);
if (pending) {
return pending;
}
// Start new fetch and track it
const fetchPromise = fetchAccountQuota(provider, accountId)
.then((result) => {
setCachedQuota(provider, accountId, result);
return result;
})
.catch((): QuotaResult => {
return { success: false, models: [], lastUpdated: Date.now() };
})
.finally(() => {
pendingFetches.delete(key);
});
pendingFetches.set(key, fetchPromise);
return fetchPromise;
}
// ============================================================================
// COOLDOWN TRACKING
// ============================================================================
@@ -176,17 +212,12 @@ export async function findHealthyAccount(
if (available.length === 0) return null;
// Fetch quota for each available account (with caching)
// Fetch quota for each available account (with caching and deduplication)
const withQuotas = await Promise.all(
available.map(async (account) => {
let quota = getCachedQuota(provider, account.id);
if (!quota) {
try {
quota = await fetchAccountQuota(provider, account.id);
setCachedQuota(provider, account.id, quota);
} catch {
quota = { success: false, models: [], lastUpdated: Date.now() };
}
quota = await fetchQuotaWithDedup(provider, account.id);
}
const avgQuota = calculateAverageQuota(quota);
@@ -298,20 +329,10 @@ export async function preflightCheck(provider: CLIProxyProvider): Promise<Prefli
return await findAndSwitch(provider, defaultAccount.id, 'Default account on cooldown');
}
// Check quota (with cache)
// Check quota (with cache and deduplication)
let quota = getCachedQuota(provider, defaultAccount.id);
if (!quota) {
try {
quota = await fetchAccountQuota(provider, defaultAccount.id);
setCachedQuota(provider, defaultAccount.id, quota);
} catch {
// API failure: proceed anyway (graceful degradation)
return {
proceed: true,
accountId: defaultAccount.id,
reason: 'Quota check failed, proceeding',
};
}
quota = await fetchQuotaWithDedup(provider, defaultAccount.id);
}
// Calculate average quota
@@ -355,12 +376,7 @@ export async function getQuotaStatus(provider: CLIProxyProvider): Promise<{
accounts.map(async (account) => {
let quota = getCachedQuota(provider, account.id);
if (!quota && provider === 'agy') {
try {
quota = await fetchAccountQuota(provider, account.id);
setCachedQuota(provider, account.id, quota);
} catch {
quota = { success: false, models: [], lastUpdated: Date.now() };
}
quota = await fetchQuotaWithDedup(provider, account.id);
}
const avgQuota = quota ? calculateAverageQuota(quota) : 100;
+15 -1
View File
@@ -32,8 +32,22 @@ export async function startServer(options: ServerOptions): Promise<ServerInstanc
const server = http.createServer(app);
const wss = new WebSocketServer({ server });
// JSON body parsing
// JSON body parsing with error handler for malformed JSON
app.use(express.json());
app.use(
(
err: Error & { status?: number; body?: string },
_req: express.Request,
res: express.Response,
next: express.NextFunction
) => {
if (err instanceof SyntaxError && err.status === 400 && 'body' in err) {
res.status(400).json({ error: 'Invalid JSON in request body' });
return;
}
next(err);
}
);
// REST API routes (modularized)
const { apiRoutes } = await import('./routes/index');
@@ -90,6 +90,7 @@ export function AccountItem({
onRemove,
onPauseToggle,
isRemoving,
isPausingAccount,
privacyMode,
showQuota,
}: AccountItemProps) {
@@ -188,16 +189,19 @@ export function AccountItem({
</DropdownMenuItem>
)}
{onPauseToggle && (
<DropdownMenuItem onClick={() => onPauseToggle(!account.paused)}>
<DropdownMenuItem
onClick={() => onPauseToggle(!account.paused)}
disabled={isPausingAccount}
>
{account.paused ? (
<>
<Play className="w-4 h-4 mr-2" />
Resume account
{isPausingAccount ? 'Resuming...' : 'Resume account'}
</>
) : (
<>
<Pause className="w-4 h-4 mr-2" />
Pause account
{isPausingAccount ? 'Pausing...' : 'Pause account'}
</>
)}
</DropdownMenuItem>
@@ -17,6 +17,8 @@ interface AccountsSectionProps {
onRemoveAccount: (accountId: string) => void;
onPauseToggle?: (accountId: string, paused: boolean) => void;
isRemovingAccount?: boolean;
/** Pause/resume mutation in progress */
isPausingAccount?: boolean;
privacyMode?: boolean;
/** Show quota bars for accounts (only applicable for 'agy' provider) */
showQuota?: boolean;
@@ -34,6 +36,7 @@ export function AccountsSection({
onRemoveAccount,
onPauseToggle,
isRemovingAccount,
isPausingAccount,
privacyMode,
showQuota,
isKiro,
@@ -71,6 +74,7 @@ export function AccountsSection({
onPauseToggle ? (paused) => onPauseToggle(account.id, paused) : undefined
}
isRemoving={isRemovingAccount}
isPausingAccount={isPausingAccount}
privacyMode={privacyMode}
showQuota={showQuota}
/>
@@ -40,6 +40,7 @@ export function ProviderEditor({
onRemoveAccount,
onPauseToggle,
isRemovingAccount,
isPausingAccount,
}: ProviderEditorProps) {
const [customPresetOpen, setCustomPresetOpen] = useState(false);
const { privacyMode } = usePrivacy();
@@ -203,6 +204,7 @@ export function ProviderEditor({
onRemoveAccount={onRemoveAccount}
onPauseToggle={onPauseToggle}
isRemovingAccount={isRemovingAccount}
isPausingAccount={isPausingAccount}
privacyMode={privacyMode}
isRemoteMode={isRemoteMode}
/>
@@ -38,6 +38,8 @@ interface ModelConfigTabProps {
onRemoveAccount: (accountId: string) => void;
onPauseToggle?: (accountId: string, paused: boolean) => void;
isRemovingAccount?: boolean;
/** Pause/resume mutation in progress */
isPausingAccount?: boolean;
privacyMode?: boolean;
/** True if connected to remote CLIProxy (quota not available) */
isRemoteMode?: boolean;
@@ -63,6 +65,7 @@ export function ModelConfigTab({
onRemoveAccount,
onPauseToggle,
isRemovingAccount,
isPausingAccount,
privacyMode,
isRemoteMode,
}: ModelConfigTabProps) {
@@ -138,6 +141,7 @@ export function ModelConfigTab({
onRemoveAccount={onRemoveAccount}
onPauseToggle={onPauseToggle}
isRemovingAccount={isRemovingAccount}
isPausingAccount={isPausingAccount}
privacyMode={privacyMode}
showQuota={provider === 'agy' && !isRemoteMode}
isKiro={isKiro}
@@ -32,6 +32,8 @@ export interface ProviderEditorProps {
onRemoveAccount: (accountId: string) => void;
onPauseToggle?: (accountId: string, paused: boolean) => void;
isRemovingAccount?: boolean;
/** Pause/resume mutation in progress */
isPausingAccount?: boolean;
}
export interface AccountItemProps {
@@ -40,6 +42,8 @@ export interface AccountItemProps {
onRemove: () => void;
onPauseToggle?: (paused: boolean) => void;
isRemoving?: boolean;
/** Pause/resume mutation in progress */
isPausingAccount?: boolean;
privacyMode?: boolean;
/** Show quota bar (only for 'agy' provider) */
showQuota?: boolean;
+4
View File
@@ -221,6 +221,8 @@ export function CliproxyPage() {
};
const handlePauseToggle = (provider: string, accountId: string, paused: boolean) => {
// Prevent rapid clicks while mutation is pending
if (pauseMutation.isPending || resumeMutation.isPending) return;
if (paused) {
pauseMutation.mutate({ provider, accountId });
} else {
@@ -377,6 +379,7 @@ export function CliproxyPage() {
handlePauseToggle(selectedVariantData.provider, accountId, paused)
}
isRemovingAccount={removeMutation.isPending}
isPausingAccount={pauseMutation.isPending || resumeMutation.isPending}
/>
) : selectedStatus ? (
<ProviderEditor
@@ -408,6 +411,7 @@ export function CliproxyPage() {
handlePauseToggle(selectedStatus.provider, accountId, paused)
}
isRemovingAccount={removeMutation.isPending}
isPausingAccount={pauseMutation.isPending || resumeMutation.isPending}
/>
) : (
<EmptyProviderState onSetup={() => setWizardOpen(true)} />