diff --git a/changelog.d/fixes/13338-combo-identical-error-circuit-breaker.md b/changelog.d/fixes/13338-combo-identical-error-circuit-breaker.md new file mode 100644 index 0000000000..8b19e3ee7f --- /dev/null +++ b/changelog.d/fixes/13338-combo-identical-error-circuit-breaker.md @@ -0,0 +1 @@ +- **fix(combo):** stop retrying a malformed-request-shape error across the entire fallback chain — a 400/422 that fails one target for a `kind: "model"` reason (exact status + error message match) will fail identically on every other target too, so a bad payload previously burned the full `MAX_GLOBAL_ATTEMPTS` budget instead of failing fast. Observed live: 41-44 identical combo decisions over 13+ minutes for a single request. Trips only on 3 consecutive identical `kind: "model"` failures, leaving transient and provider-side errors untouched ([#13338](https://github.com/diegosouzapw/OmniRoute/pull/13338)). diff --git a/open-sse/services/combo/comboAttemptLoop.ts b/open-sse/services/combo/comboAttemptLoop.ts index 102c27196f..26ee38f7dd 100644 --- a/open-sse/services/combo/comboAttemptLoop.ts +++ b/open-sse/services/combo/comboAttemptLoop.ts @@ -44,6 +44,8 @@ import { withQuotaExhaustionClassification } from "./quotaExhaustion.ts"; import { COMBO_LOOP_SAFETY_TIMEOUT_MS, COMBO_SAFETY_DRAIN_MS, + IDENTICAL_MODEL_ERROR_STREAK, + hasIdenticalModelErrorStreak, resolveDelayMs, } from "./comboPredicates.ts"; import { evaluateExecuteTargetGates } from "./executeTargetGates.ts"; @@ -190,6 +192,12 @@ export async function dispatchWithCooldownRetry(opts: { }); const runningTasks = new Set>(); let anySuccess = false; + // Flipped once the last IDENTICAL_MODEL_ERROR_STREAK targets have all + // failed with the exact same request-shape error — see comboPredicates.ts. + // Stops this set-try's target loop early AND skips the whole-set retry + // below, since a malformed request fails identically no matter how many + // more times it's replayed against the remaining fallbacks. + let comboRequestMalformed = false; // #10681: steps already recorded as dispatched (so per-target retries do not // duplicate the decision). state.dispatchedTargets = new Set(); @@ -223,7 +231,7 @@ export async function dispatchWithCooldownRetry(opts: { }; for (let i = 0; i < state.orderedTargets.length; i++) { - if (anySuccess || state.comboExpired) break; + if (anySuccess || state.comboExpired || comboRequestMalformed) break; const abortController = new AbortController(); state.abortControllers.set(i, abortController); @@ -296,6 +304,18 @@ export async function dispatchWithCooldownRetry(opts: { `${i + 1}/${state.orderedTargets.length} targets (${state.recordedAttempts} attempted) — stopping` ); } + + if (!anySuccess && !state.comboExpired && hasIdenticalModelErrorStreak(state.comboErrors)) { + comboRequestMalformed = true; + const last = state.comboErrors[state.comboErrors.length - 1]; + deps.log.warn( + "COMBO", + `The last ${IDENTICAL_MODEL_ERROR_STREAK} targets all failed with the identical ` + + `request-shape error (status ${last.status}) after ${i + 1}/${state.orderedTargets.length} ` + + `targets (${state.recordedAttempts} attempted) — stopping instead of retrying the same ` + + `malformed request against remaining fallbacks or set-retries` + ); + } } if (!anySuccess && runningTasks.size > 0) { @@ -386,8 +406,10 @@ export async function dispatchWithCooldownRetry(opts: { }); } - // Retry the entire set if more attempts remain - if (setTry < extra.maxSetRetries) continue; + // Retry the entire set if more attempts remain -- unless the identical- + // error streak already proved the request itself is malformed, in which + // case a fresh set-try would just reproduce the same streak. + if (setTry < extra.maxSetRetries && !comboRequestMalformed) continue; if (!state.lastStatus && state.recordedAttempts === 0 && extra.comboCooldownWaitEnabled) { const circuitOpenWait = resolveCircuitOpenWaitDecision({ diff --git a/open-sse/services/combo/comboPredicates.ts b/open-sse/services/combo/comboPredicates.ts index 435c17458d..847363a70e 100644 --- a/open-sse/services/combo/comboPredicates.ts +++ b/open-sse/services/combo/comboPredicates.ts @@ -24,6 +24,7 @@ import { import { isResourceNotFoundResponse } from "../errorClassifier.ts"; import { getTrustedLocalRateLimitResponse } from "../rateLimitManager/errors.ts"; import type { ResolvedComboTarget } from "./types.ts"; +import type { ComboErrorEntry } from "./comboErrorAggregation.ts"; // Status codes that should mark round-robin target semaphores as cooling down. export const TRANSIENT_FOR_SEMAPHORE = [429, 502, 503, 504]; @@ -111,6 +112,29 @@ export const MAX_GLOBAL_ATTEMPTS = 30; // background-request DoS risk that motivated MAX_COMBO_DEPTH_HARD_CAP. export const MAX_GLOBAL_ATTEMPTS_HARD_CAP = 200; +// A malformed/unsupported request shape (e.g. an incompatible tool-call +// history for a provider's translation layer) fails the SAME way against +// every fallback target, since it's a property of the request, not of any +// one provider. Once this many *consecutive* targets have failed with the +// identical model-shape error (same kind, status, and message), retrying the +// remaining fallbacks — or the whole set again — cannot succeed either; it +// only burns MAX_GLOBAL_ATTEMPTS and wall-clock time. See combo.ts's +// `comboRequestMalformed` handling. +export const IDENTICAL_MODEL_ERROR_STREAK = 3; + +export function hasIdenticalModelErrorStreak( + comboErrors: ReadonlyArray, + streak: number = IDENTICAL_MODEL_ERROR_STREAK +): boolean { + if (comboErrors.length < streak) return false; + const tail = comboErrors.slice(-streak); + const [first, ...rest] = tail; + if (first.kind !== "model") return false; + return rest.every( + (e) => e.kind === first.kind && e.status === first.status && e.error === first.error + ); +} + /** * Clamp an operator-configured combo nesting depth (config.maxComboDepth) to a * safe integer in [1, MAX_COMBO_DEPTH_HARD_CAP]. Anything non-numeric, < 1, or diff --git a/tests/unit/combo-identical-error-streak.test.ts b/tests/unit/combo-identical-error-streak.test.ts new file mode 100644 index 0000000000..688915304b --- /dev/null +++ b/tests/unit/combo-identical-error-streak.test.ts @@ -0,0 +1,83 @@ +// Regression coverage for the identical-request-shape-error circuit breaker +// (combo.ts / comboPredicates.ts): a corrupted conversation history that one +// provider's protocol validation rejects with a 4xx "kind: model" error is +// certain to be rejected identically by every remaining fallback target, +// since the malformed payload itself — not the provider — is at fault. +// Without this breaker, production observed a single malformed request +// burning through all MAX_GLOBAL_ATTEMPTS (30) targets identically, taking +// 13+ minutes before the combo happened to reach a lenient-enough fallback +// or gave up. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const { hasIdenticalModelErrorStreak, IDENTICAL_MODEL_ERROR_STREAK } = await import( + "../../open-sse/services/combo/comboPredicates.ts" +); + +function entry(overrides: Partial<{ status: number; error: string; kind: string }> = {}) { + return { + model: "gemini/gemini-3.1-flash-lite", + status: 400, + error: "[400]: Please ensure that function call turn comes immediately after a user turn or after a function response turn.", + kind: "model", + ...overrides, + } as never; +} + +test("false when fewer than the streak threshold have been attempted", () => { + assert.equal(hasIdenticalModelErrorStreak([]), false); + assert.equal(hasIdenticalModelErrorStreak([entry()]), false); + assert.equal( + hasIdenticalModelErrorStreak(Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK - 1 }, () => entry())), + false + ); +}); + +test("true once the last IDENTICAL_MODEL_ERROR_STREAK targets share status+error+kind", () => { + const errors = Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK }, () => entry()); + assert.equal(hasIdenticalModelErrorStreak(errors), true); +}); + +test("stays true once the streak is reached even with more matching entries after it", () => { + const errors = Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK + 2 }, () => entry()); + assert.equal(hasIdenticalModelErrorStreak(errors), true); +}); + +test("false when the tail is not all the same kind (a transient provider failure mixed in)", () => { + const errors = [ + entry(), + entry({ kind: "provider", status: 503, error: "[503]: upstream unavailable" }), + entry(), + ]; + assert.equal(hasIdenticalModelErrorStreak(errors), false); +}); + +test("false when the tail is not kind 'model' at all (e.g. every target rate-limited)", () => { + const errors = Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK }, () => + entry({ kind: "rate_limit", status: 429, error: "[429]: Provider returned error" }) + ); + assert.equal(hasIdenticalModelErrorStreak(errors), false); +}); + +test("false when the same status/kind repeats but with a genuinely different error message", () => { + const errors = [ + entry({ error: "[400]: missing required field 'model'" }), + entry({ error: "[400]: unknown tool 'foo'" }), + entry({ error: "[400]: Please ensure that function call turn comes immediately after a user turn or after a function response turn." }), + ]; + assert.equal(hasIdenticalModelErrorStreak(errors), false); +}); + +test("true even when an earlier, unrelated failure precedes the matching streak", () => { + const errors = [ + entry({ kind: "auth", status: 401, error: "[401]: invalid api key" }), + ...Array.from({ length: IDENTICAL_MODEL_ERROR_STREAK }, () => entry()), + ]; + assert.equal(hasIdenticalModelErrorStreak(errors), true); +}); + +test("a custom streak length is honored", () => { + const errors = Array.from({ length: 2 }, () => entry()); + assert.equal(hasIdenticalModelErrorStreak(errors, 2), true); + assert.equal(hasIdenticalModelErrorStreak(errors, 3), false); +});