diff --git a/CHANGELOG.md b/CHANGELOG.md index e626903711..f436ac87e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ - **cline (empty/false-502 non-streaming responses):** the Cline gateway can wrap OpenAI-compatible chat completions in a `{ success, data: { choices, usage, … } }` envelope. The non-streaming path checked the top-level body for empty content before unwrapping, so a valid Cline response was treated as malformed. The body is now unwrapped via `unwrapClineNonStreamingEnvelope()` right after the provider-envelope unwrap and before the empty-content check, closing the remaining gap from #5956/#5924. Regression guard: `tests/unit/cline-response-envelope.test.ts`. ([#5956](https://github.com/diegosouzapw/OmniRoute/issues/5956) — thanks @KooshaPari) +- **combo (per-model-quota providers exhausted on a single model 500):** for per-model-quota providers (gemini, github, passthrough, compatible) that multiplex many models behind one connection, a model-level `500` (e.g. Gemini "Internal error encountered") wrongly marked the whole connection exhausted, so sibling models on the same connection were skipped and the combo could 502 instead of falling back. `markConnectionLevelExhaustion` now leaves the connection eligible on a `500` for per-model-quota providers (other connection-level statuses — 408/502/503/504/524 — still exhaust correctly), and the retry loop early-returns when a model is already in lockout. Regression guard: `tests/unit/combo/combo-target-exhaustion.test.ts`. ([#5976](https://github.com/diegosouzapw/OmniRoute/pull/5976) — thanks @hartmark) + ### 📝 Maintenance - **test (deflake `setup-claude`):** `tests/unit/cli/setup-claude.test.ts` failed ~50% of runs with `Unable to deserialize cloned data due to invalid or unsupported version` at file teardown (all subtests passed), randomly reddening `Unit Tests fast-path (2/2)` / `Fast Quality Gates` across the PR→release queue. Root cause: `node --test` streams each file's report to the parent as V8-serialized frames on fd 1 (stdout), and the CLI helper under test (`syncClaudeProfilesFromModels`) prints progress via `console.log` — that stdout output interleaved with the serialized frames and corrupted the stream. The test now silences the stdout-writing `console` methods for the file's duration (no assertion inspects stdout), making it deterministic (15/15 green locally). ([#5959](https://github.com/diegosouzapw/OmniRoute/issues/5959)) diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 6518a3413d..4ce9712934 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -1920,6 +1920,15 @@ export async function handleComboChat({ !isTokenLimitBreach && [408, 429, 500, 502, 503, 504].includes(result.status); if (retry < maxRetries && isTransient && !providerExhausted) { + if ( + provider && + rawModel && + isModelLocked(provider, targetWithConnection.connectionId || "", rawModel) + ) { + log.info("COMBO", `Skipping retry for ${modelStr} — model lockout active`); + if (i > 0) fallbackCount++; + return null; + } // Record model lockout immediately on the first transient failure — // once the model is cooling down, retrying it would waste an upstream // call and extend the cooldown via exponential backoff. diff --git a/open-sse/services/combo/__tests__/targetExhaustion.test.ts b/open-sse/services/combo/__tests__/targetExhaustion.test.ts deleted file mode 100644 index f422aafc6a..0000000000 --- a/open-sse/services/combo/__tests__/targetExhaustion.test.ts +++ /dev/null @@ -1,311 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { applyComboTargetExhaustion, type ComboExhaustionSets } from "../targetExhaustion.ts"; -import type { ResolvedComboTarget, ComboLogger } from "../types.ts"; - -function makeTarget(overrides: Partial = {}): ResolvedComboTarget { - return { - kind: "model", - stepId: "step-1", - executionKey: "key-1", - modelStr: "gpt-4", - provider: "openai", - providerId: "p1", - connectionId: "c1", - allowedConnectionIds: null, - weight: 1, - label: null, - failoverBeforeRetry: undefined, - ...overrides, - }; -} - -function makeLogger(): ComboLogger { - const msgs: string[] = []; - return { - info: (...args: unknown[]) => { msgs.push(args.join(" ")); }, - warn: (...args: unknown[]) => { msgs.push(args.join(" ")); }, - error: (...args: unknown[]) => { msgs.push(args.join(" ")); }, - debug: (...args: unknown[]) => { msgs.push(args.join(" ")); }, - _msgs: msgs, - } as ComboLogger & { _msgs: string[] }; -} - -function makeSets(): ComboExhaustionSets { - return { - exhaustedProviders: new Set(), - exhaustedConnections: new Set(), - transientRateLimitedProviders: new Set(), - }; -} - -describe("applyComboTargetExhaustion", () => { - it("marks provider exhausted when isProviderExhaustedReason is true (quota)", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 429 }, - // isProviderExhaustedReason reads `reason`/`creditsExhausted`/`dailyQuotaExhausted` - // (NOT `error.code`), so signal full-account exhaustion via creditsExhausted. - fallbackResult: { creditsExhausted: true }, - errorText: "", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(true); - expect(sets.exhaustedProviders.has("openai")).toBe(true); - expect(sets.exhaustedProviders.size).toBe(1); - expect(sets.transientRateLimitedProviders.has("openai")).toBe(false); - }); - - it("marks provider exhausted when classifyErrorText returns QUOTA_EXHAUSTED", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 429 }, - fallbackResult: {} as any, - // classifyErrorText flags "quota exceeded" as QUOTA_EXHAUSTED. - errorText: "Quota exceeded — please retry later.", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(true); - expect(sets.exhaustedProviders.has("openai")).toBe(true); - }); - - it("marks provider exhausted when allAccountsRateLimited is true", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 503 }, - fallbackResult: {} as any, - errorText: "Service temporarily unavailable", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: true, - sets, - log, - tag: "COMBO-RR", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(true); - expect(sets.exhaustedProviders.has("openai")).toBe(true); - }); - - it("does NOT mark provider exhausted for per-model-quota providers (different model)", () => { - const sets = makeSets(); - const log = makeLogger(); - // gemini has per-model quotas (hasPerModelQuota === true): a model-scoped quota - // 429 must NOT mark the whole provider exhausted — other models may still work. - const target = makeTarget({ provider: "gemini" }); - const exhausted = applyComboTargetExhaustion(target, { - result: { status: 429 }, - fallbackResult: { reason: "quota_exhausted" } as any, - errorText: "quota exceeded for model gpt-4", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - expect(sets.exhaustedProviders.has("gemini")).toBe(false); - expect(sets.transientRateLimitedProviders.has("gemini")).toBe(true); - }); - - it("does NOT mark provider exhausted for unknown providers", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget({ provider: "unknown" }), { - result: { status: 503 }, - fallbackResult: { error: { code: "quota_exhausted" } }, - errorText: "quota exhausted", - rawModel: "unknown-model", - isTokenLimitBreach: false, - allAccountsRateLimited: true, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - }); - - it("does NOT mark provider exhausted for empty provider strings", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget({ provider: "" }), { - result: { status: 503 }, - fallbackResult: { error: { code: "quota_exhausted" } }, - errorText: "quota exhausted", - rawModel: "model", - isTokenLimitBreach: false, - allAccountsRateLimited: true, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - }); - - it("marks transientRateLimited on 429 when NOT token-limit breach and NOT provider-exhausted", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 429 }, - fallbackResult: {} as any, - errorText: "Rate limited", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - expect(sets.transientRateLimitedProviders.has("openai")).toBe(true); - expect(sets.exhaustedProviders.has("openai")).toBe(false); - }); - - it("does NOT mark transientRateLimited on 429 when isTokenLimitBreach is true", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 429 }, - fallbackResult: {} as any, - errorText: "Token limit exceeded", - rawModel: "gpt-4", - isTokenLimitBreach: true, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - expect(sets.transientRateLimitedProviders.has("openai")).toBe(false); - expect(sets.exhaustedProviders.has("openai")).toBe(false); - }); - - it("marks exhaustedConnections on connection-level error status (502) with connectionId", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion( - makeTarget({ provider: "openai", connectionId: "conn-1" }), - { - result: { status: 502 }, - fallbackResult: {} as any, - errorText: "Bad Gateway", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - } - ); - expect(exhausted).toBe(false); - expect(sets.exhaustedConnections.has("openai:conn-1")).toBe(true); - expect(sets.exhaustedProviders.has("openai")).toBe(false); - }); - - it("marks exhaustedProviders on connection-level error when NO connectionId", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion( - makeTarget({ provider: "openai", connectionId: null }), - { - result: { status: 502 }, - fallbackResult: {} as any, - errorText: "Bad Gateway", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - } - ); - expect(exhausted).toBe(false); - expect(sets.exhaustedProviders.has("openai")).toBe(true); - expect(sets.exhaustedConnections.size).toBe(0); - }); - - it("does NOT mark anything for circuit-open (X-OmniRoute-Provider-Breaker header)", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 503, headers: new Map([["x-omniroute-provider-breaker", "open"]]) as any }, - fallbackResult: {} as any, - errorText: "", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - expect(sets.exhaustedProviders.has("openai")).toBe(false); - expect(sets.exhaustedConnections.has("openai:c1")).toBe(false); - expect(sets.transientRateLimitedProviders.has("openai")).toBe(false); - }); - - it("does NOT mark exhaustion for non-connection-level status codes (400)", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 400 }, - fallbackResult: {} as any, - errorText: "Bad Request", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - expect(sets.exhaustedConnections.size).toBe(0); - expect(sets.exhaustedProviders.size).toBe(0); - expect(sets.transientRateLimitedProviders.size).toBe(0); - }); - - it("does NOT mark anything for 200 (success)", () => { - const sets = makeSets(); - const log = makeLogger(); - const exhausted = applyComboTargetExhaustion(makeTarget(), { - result: { status: 200 }, - fallbackResult: {} as any, - errorText: "", - rawModel: "gpt-4", - isTokenLimitBreach: false, - allAccountsRateLimited: false, - sets, - log, - tag: "COMBO", - exhaustedLogLevel: "info", - }); - expect(exhausted).toBe(false); - expect(sets.exhaustedProviders.size).toBe(0); - expect(sets.exhaustedConnections.size).toBe(0); - expect(sets.transientRateLimitedProviders.size).toBe(0); - }); -}); diff --git a/open-sse/services/combo/targetExhaustion.ts b/open-sse/services/combo/targetExhaustion.ts index 01f73d58db..7091b483ef 100644 --- a/open-sse/services/combo/targetExhaustion.ts +++ b/open-sse/services/combo/targetExhaustion.ts @@ -104,7 +104,7 @@ export function applyComboTargetExhaustion( if (result.status === 429 && !isTokenLimitBreach && provider && provider !== "unknown") { transientRateLimitedProviders.add(provider); } - markConnectionLevelExhaustion(target, { result, errorText, sets, log, tag }); + markConnectionLevelExhaustion(target, { result, errorText, sets, log, tag, rawModel }); } return providerExhausted; @@ -118,9 +118,12 @@ export function applyComboTargetExhaustion( */ function markConnectionLevelExhaustion( target: ResolvedComboTarget, - opts: Pick + opts: Pick< + ApplyComboTargetExhaustionOptions, + "result" | "errorText" | "sets" | "log" | "tag" | "rawModel" + > ): void { - const { result, errorText, sets, log, tag } = opts; + const { result, errorText, sets, log, tag, rawModel } = opts; const provider = target.provider; if ( !provider || @@ -130,7 +133,13 @@ function markConnectionLevelExhaustion( // #5085: empty-content 502 is a healthy connection returning no body — model-level, not // connection-level. Don't exhaust the provider; let the remaining legs (incl. same-provider) // be tried in-request. - isEmptyContentFailure(result.status, errorText) + isEmptyContentFailure(result.status, errorText) || + // Per-model-quota providers (gemini, github, passthrough, compatible) multiplex models + // behind one connection. A model-level 500 (e.g. Gemini "Internal error encountered") + // must NOT exhaust the connection — other models on the same connection may still succeed. + // Other connection-level statuses (408/502/503/504/524) indicate the connection itself is + // bad, so they correctly exhaust even for per-model-quota providers. + (result.status === 500 && hasPerModelQuota(provider, rawModel)) ) { return; } diff --git a/tests/unit/combo/combo-target-exhaustion.test.ts b/tests/unit/combo/combo-target-exhaustion.test.ts index 5d28703aa0..c6b0369187 100644 --- a/tests/unit/combo/combo-target-exhaustion.test.ts +++ b/tests/unit/combo/combo-target-exhaustion.test.ts @@ -166,3 +166,200 @@ test("a 200/benign status with no exhaustion mutates nothing and returns false", 0 ); }); + +test("does NOT mark provider exhausted for per-model-quota providers (different model)", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion(target({ provider: "gemini" }), { + ...baseOpts, + result: { status: 429 }, + fallbackResult: { reason: "quota_exhausted" }, + errorText: "quota exceeded for model gpt-4", + sets: s, + }); + assert.equal(exhausted, false); + assert.equal(s.exhaustedProviders.has("gemini"), false); + assert.ok(s.transientRateLimitedProviders.has("gemini")); +}); + +test("does NOT mark provider exhausted for empty provider strings", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion(target({ provider: "" }), { + ...baseOpts, + result: { status: 503 }, + fallbackResult: { error: { code: "quota_exhausted" } }, + errorText: "quota exhausted", + allAccountsRateLimited: true, + sets: s, + }); + assert.equal(exhausted, false); +}); + +test("does NOT mark transientRateLimited on 429 when isTokenLimitBreach is true", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion(target(), { + ...baseOpts, + result: { status: 429 }, + fallbackResult: {}, + errorText: "Token limit exceeded", + isTokenLimitBreach: true, + sets: s, + }); + assert.equal(exhausted, false); + assert.equal(s.transientRateLimitedProviders.has("test-dedup-provider"), false); + assert.equal(s.exhaustedProviders.has("test-dedup-provider"), false); +}); + +test("does NOT mark anything for circuit-open (X-OmniRoute-Provider-Breaker header)", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion(target(), { + ...baseOpts, + result: { status: 503, headers: new Map([["x-omniroute-provider-breaker", "open"]]) }, + fallbackResult: {}, + errorText: "", + sets: s, + }); + assert.equal(exhausted, false); + assert.equal(s.exhaustedProviders.has("test-dedup-provider"), false); + assert.equal(s.exhaustedConnections.has("test-dedup-provider:conn-1"), false); + assert.equal(s.transientRateLimitedProviders.has("test-dedup-provider"), false); +}); + +test("does NOT mark exhaustion for non-connection-level status codes (400)", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion(target(), { + ...baseOpts, + result: { status: 400 }, + fallbackResult: {}, + errorText: "Bad Request", + sets: s, + }); + assert.equal(exhausted, false); + assert.equal(s.exhaustedConnections.size, 0); + assert.equal(s.exhaustedProviders.size, 0); + assert.equal(s.transientRateLimitedProviders.size, 0); +}); + +test("does NOT mark connection exhausted for per-model-quota provider on 500 (gemini model-level error)", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion( + target({ provider: "gemini", connectionId: "gemini-conn-1" }), + { + ...baseOpts, + result: { status: 500 }, + fallbackResult: {}, + errorText: "Internal error encountered.", + rawModel: "gemma-4-31b-it", + sets: s, + } + ); + assert.equal(exhausted, false); + assert.equal(s.exhaustedProviders.has("gemini"), false); + assert.equal(s.exhaustedConnections.has("gemini:gemini-conn-1"), false); + assert.equal(s.transientRateLimitedProviders.has("gemini"), false); +}); + +// Sanitized Gemini 500 response — model-level "Internal error encountered" should NOT exhaust +// the connection, allowing sibling models on the same provider to be tried. +test("gemini 500 INTERNAL (sanitized real response) does NOT exhaust connection — sibling retry", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion( + target({ provider: "gemini", connectionId: "gemini-key-abc" }), + { + ...baseOpts, + result: { status: 500 }, + fallbackResult: {}, + errorText: "Internal error encountered.", + rawModel: "gemma-4-31b-it", + structuredError: { code: 500, status: "INTERNAL", message: "Internal error encountered." }, + sets: s, + } + ); + assert.equal(exhausted, false, "providerExhausted must be false"); + assert.equal(s.exhaustedProviders.has("gemini"), false, "must not exhaust provider"); + assert.equal( + s.exhaustedConnections.has("gemini:gemini-key-abc"), + false, + "must not exhaust connection — sibling model may succeed" + ); + assert.equal(s.transientRateLimitedProviders.has("gemini"), false); +}); + +// Non-500 connection-level errors MUST exhaust the connection even for per-model-quota providers. +// A 503 (Service Unavailable) means the upstream is down — retrying sibling models wastes calls. +test("gemini 503 DOES exhaust connection (upstream down, not model-level)", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion( + target({ provider: "gemini", connectionId: "gemini-key-abc" }), + { + ...baseOpts, + result: { status: 503 }, + fallbackResult: {}, + errorText: "The service is currently unavailable.", + rawModel: "gemma-4-31b-it", + sets: s, + } + ); + assert.equal(exhausted, false, "providerExhausted is false (not quota)"); + assert.equal( + s.exhaustedConnections.has("gemini:gemini-key-abc"), + true, + "503 must exhaust connection — upstream is down" + ); + assert.equal(s.exhaustedProviders.size, 0); +}); + +test("gemini 502 DOES exhaust connection (bad gateway)", () => { + const s = sets(); + const exhausted = applyComboTargetExhaustion( + target({ provider: "gemini", connectionId: "gemini-key-abc" }), + { + ...baseOpts, + result: { status: 502 }, + fallbackResult: {}, + errorText: "Bad Gateway", + rawModel: "gemma-4-31b-it", + sets: s, + } + ); + assert.equal(exhausted, false); + assert.equal(s.exhaustedConnections.has("gemini:gemini-key-abc"), true); +}); + +test("gemini 504 DOES exhaust connection (gateway timeout)", () => { + const s = sets(); + applyComboTargetExhaustion(target({ provider: "gemini", connectionId: "gemini-key-abc" }), { + ...baseOpts, + result: { status: 504 }, + fallbackResult: {}, + errorText: "Gateway Timeout", + rawModel: "gemini-2.0-flash", + sets: s, + }); + assert.equal(s.exhaustedConnections.has("gemini:gemini-key-abc"), true); +}); + +test("gemini 408 DOES exhaust connection (request timeout)", () => { + const s = sets(); + applyComboTargetExhaustion(target({ provider: "gemini", connectionId: "gemini-key-abc" }), { + ...baseOpts, + result: { status: 408 }, + fallbackResult: {}, + errorText: "Request Timeout", + rawModel: "gemini-2.0-flash", + sets: s, + }); + assert.equal(s.exhaustedConnections.has("gemini:gemini-key-abc"), true); +}); + +test("gemini 524 DOES exhaust connection (cloudflare timeout)", () => { + const s = sets(); + applyComboTargetExhaustion(target({ provider: "gemini", connectionId: "gemini-key-abc" }), { + ...baseOpts, + result: { status: 524 }, + fallbackResult: {}, + errorText: "A Timeout Occurred", + rawModel: "gemini-2.0-flash", + sets: s, + }); + assert.equal(s.exhaustedConnections.has("gemini:gemini-key-abc"), true); +});