mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 12:15:57 +00:00
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:
@@ -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
@@ -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;
|
||||
|
||||
@@ -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)} />
|
||||
|
||||
Reference in New Issue
Block a user