diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index caac4f0127..02d6abfbd9 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -61,6 +61,16 @@ const COMBO_BAD_REQUEST_FALLBACK_PATTERNS = [ /third-party apps/i, ]; +// Patterns that signal all accounts for a provider are rate-limited / exhausted. +// Used to detect 503 responses from handleNoCredentials so combo can fallback. +const ALL_ACCOUNTS_RATE_LIMITED_PATTERNS = [/unavailable/i, /service temporarily unavailable/i]; + +function isAllAccountsRateLimitedResponse(status: number, contentType: string | null, errorText: string): boolean { + if (status !== 503) return false; + if (!contentType?.includes("application/json")) return false; + return ALL_ACCOUNTS_RATE_LIMITED_PATTERNS.some((p) => p.test(errorText)); +} + const MAX_COMBO_DEPTH = 3; const MAX_FALLBACK_WAIT_MS = 5000; @@ -1625,6 +1635,12 @@ export async function handleComboChat({ } } + const isAllAccountsRateLimited = isAllAccountsRateLimitedResponse( + result.status, + result.headers?.get("content-type") ?? null, + errorText + ); + const { shouldFallback, cooldownMs } = checkFallbackError( result.status, errorText, @@ -1641,7 +1657,9 @@ export async function handleComboChat({ breaker._onFailure(); } - if (!shouldFallback && !comboBadRequestFallback) { + if (isAllAccountsRateLimited) { + log.info("COMBO", `All accounts rate-limited for ${modelStr}, falling back to next model`); + } else if (!shouldFallback && !comboBadRequestFallback) { log.warn("COMBO", `Model ${modelStr} failed (no fallback)`, { status: result.status }); recordComboRequest(combo.name, modelStr, { success: false, @@ -1966,6 +1984,12 @@ async function handleRoundRobinCombo({ ); const comboBadRequestFallback = shouldFallbackComboBadRequest(result.status, errorText); + const isAllAccountsRateLimited = isAllAccountsRateLimitedResponse( + result.status, + result.headers?.get("content-type") ?? null, + errorText + ); + // Transient errors → mark in semaphore AND record circuit breaker failure if (TRANSIENT_FOR_BREAKER.includes(result.status) && cooldownMs > 0) { semaphore.markRateLimited(semaphoreKey, cooldownMs); @@ -1976,7 +2000,9 @@ async function handleRoundRobinCombo({ ); } - if (!shouldFallback && !comboBadRequestFallback) { + if (isAllAccountsRateLimited) { + log.info("COMBO", `All accounts rate-limited for ${modelStr}, falling back to next model`); + } else if (!shouldFallback && !comboBadRequestFallback) { log.warn("COMBO-RR", `${modelStr} failed (no fallback)`, { status: result.status }); recordComboRequest(combo.name, modelStr, { success: false, diff --git a/tests/unit/combo-routing-engine.test.ts b/tests/unit/combo-routing-engine.test.ts index 6acd94f0e0..38deb7ef7a 100644 --- a/tests/unit/combo-routing-engine.test.ts +++ b/tests/unit/combo-routing-engine.test.ts @@ -1765,3 +1765,112 @@ test("handleComboChat round-robin recovers from provider-scoped 400s when a late assert.equal(result.ok, true); assert.deepEqual(calls, ["model-a", "model-b"]); }); + +test("handleComboChat falls back to next model when first model returns all-accounts-rate-limited 503", async () => { + const calls = []; + + const result = await handleComboChat({ + body: {}, + combo: { + name: "all-accounts-rate-limited-fallback", + strategy: "priority", + models: ["model-a", "model-b"], + config: { maxRetries: 0 }, + }, + handleSingleModel: async (_body, modelStr) => { + calls.push(modelStr); + // Simulate handleNoCredentials returning a 503 with "unavailable" message + // This is the signal emitted when getProviderCredentialsWithQuotaPreflight exhausts all accounts + return new Response( + JSON.stringify({ error: { message: `[provider/model] Service temporarily unavailable` } }), + { + status: 503, + headers: { "content-type": "application/json" }, + } + ); + }, + isModelAvailable: async () => true, + log: createLog(), + settings: null, + allCombos: null, + }); + + const payload = await result.json(); + // First model returns 503 with "unavailable" → combo should try model-b next + // If the fix is not applied, combo would abort here and return 503 immediately + assert.equal(result.ok, true); + assert.deepEqual(calls, ["model-a", "model-b"]); + assert.equal(payload.choices[0].message.content, "ok"); +}); + +test("handleComboChat round-robin falls back when all-accounts-rate-limited 503 is returned", async () => { + const calls = []; + + const result = await handleComboChat({ + body: {}, + combo: { + name: "rr-all-accounts-rate-limited", + strategy: "round-robin", + models: ["model-a", "model-b"], + config: { maxRetries: 0, retryDelayMs: 1, concurrencyPerModel: 1, queueTimeoutMs: 5 }, + }, + handleSingleModel: async (_body, modelStr) => { + calls.push(modelStr); + // Simulate all accounts rate-limited — handleNoCredentials signal + return new Response( + JSON.stringify({ error: { message: `[provider/model] Service temporarily unavailable` } }), + { + status: 503, + headers: { "content-type": "application/json" }, + } + ); + }, + isModelAvailable: async () => true, + log: createLog(), + settings: null, + allCombos: null, + }); + + const payload = await result.json(); + assert.equal(result.ok, true); + assert.deepEqual(calls, ["model-a", "model-b"]); + assert.equal(payload.choices[0].message.content, "ok"); +}); + +test("handleComboChat aborts combo when 503 response does NOT contain the unavailable signal", async () => { + const calls = []; + + const result = await handleComboChat({ + body: {}, + combo: { + name: "503-no-signal-abort", + strategy: "priority", + models: ["model-a", "model-b"], + config: { maxRetries: 0 }, + }, + handleSingleModel: async (_body, modelStr) => { + calls.push(modelStr); + // A generic 503 that is NOT an all-accounts-rate-limited signal + // (missing "unavailable" in message or wrong content-type) + return new Response( + JSON.stringify({ error: { message: "Server error" } }), + { + status: 503, + headers: { "content-type": "text/html" }, + } + ); + }, + isModelAvailable: async () => true, + log: createLog(), + settings: null, + allCombos: null, + }); + + const payload = await result.json(); + // Without the fix, combo would abort (still 503). With the fix, it's still 503 because + // the signal check filters out non-JSON or non-"unavailable" responses. + assert.equal(result.status, 503); + // Model-a was tried, model-b was NOT tried (combo aborted) + assert.deepEqual(calls, ["model-a"]); + assert.ok(payload.error?.message?.includes("Server error") || payload.error?.message?.includes("unavailable") || result.status === 503); +});