fix(sse): re-export PROVIDER_BREAKER_FAILURE_STATUSES for the orphaned all-rate-limited breaker path (#8390)

Root cause: #8013 extracted shouldTripProviderBreakerForResult() from
src/sse/handlers/chat.ts into the new src/sse/handlers/chatPredicates.ts,
taking the (non-exported) const PROVIDER_BREAKER_FAILURE_STATUSES with it.
A second, independent use of that const survived in chat.ts's
handleSingleModelChat(), in the "all credentials rate-limited" block
(~line 1340) — that reference was left orphaned by the extraction.

Production impact: any request where every credential for a
provider+model is simultaneously rate-limited throws
`ReferenceError: PROVIDER_BREAKER_FAILURE_STATUSES is not defined` at
runtime in that code path. Concretely this meant:
- breaker._onFailure() was unreachable on the all-rate-limited path, so
  the provider circuit breaker could not trip from it
- the ReferenceError propagated up and got mapped to a generic 502,
  masking the real 503 upstream-unavailable status in combo responses
- the issue-agent route surfaced a generic 400 instead of the actual
  429 provider-rate-limited response

Fix: export PROVIDER_BREAKER_FAILURE_STATUSES from chatPredicates.ts and
add it to chat.ts's existing import block from that module. No behavior
change — the classification set ([408, 500, 502, 503, 504]) is
unchanged, this only repairs the broken reference.

Also re-points tests/unit/nvidia-quota-phase1.test.ts's regex-based
declaration check at chatPredicates.ts, where the const now actually
lives (it previously read chat.ts via fs+regex and silently failed to
find the declaration). The regex and the classification assertions
themselves are unchanged — this test still proves 429 is excluded from
the whole-provider breaker.

Refs #8013
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-24 09:56:51 -03:00
committed by GitHub
parent 3504050fcf
commit 3b4f4afc9d
3 changed files with 8 additions and 4 deletions

View File

@@ -75,6 +75,7 @@ import {
} from "./chatHelpers";
import {
isAntigravityMissingProjectError,
PROVIDER_BREAKER_FAILURE_STATUSES,
shouldTripProviderBreakerForResult,
} from "./chatPredicates";
import { connectionHasExtraKeys } from "@omniroute/open-sse/services/apiKeyRotator.ts";

View File

@@ -1,7 +1,7 @@
import { isLocalStreamLifecycleError } from "../../shared/utils/circuitBreaker";
import { isRequestScopedUpstreamFailure } from "./comboFailureLogging";
const PROVIDER_BREAKER_FAILURE_STATUSES = new Set([408, 500, 502, 503, 504]);
export const PROVIDER_BREAKER_FAILURE_STATUSES = new Set([408, 500, 502, 503, 504]);
// #7907/#7908: single-model breaker trip bypasses the `isFailure` option (only applies
// inside `breaker.execute()`), so it needs its own `isLocalStreamLifecycleError` guard —

View File

@@ -214,16 +214,19 @@ test("semaphore.getStats reflects nvidia's per-connection gate key", async () =>
// ── Failure-mode separation regression guard ────────────────────────────────
test("nvidia 429 does not trip the provider circuit breaker (unchanged 408/500/502/503/504-only classification)", () => {
// This PR does not touch src/sse/handlers/chat.ts or the circuit breaker — this
// is a documentation-alignment guard proving Phase 1 didn't accidentally widen
// This PR does not touch the circuit breaker classification — this is a
// documentation-alignment guard proving Phase 1 didn't accidentally widen
// PROVIDER_BREAKER_FAILURE_STATUSES to include 429 (which would collapse the
// per-model lockout this PR adds into a whole-connection/provider outage).
// #8013 extracted the const from src/sse/handlers/chat.ts into chatPredicates.ts
// (still consumed by chat.ts's breaker paths) — read the declaration from its
// current home.
const chatHandlerPath = path.join(
process.cwd(),
"src",
"sse",
"handlers",
"chat.ts"
"chatPredicates.ts"
);
const source = fs.readFileSync(chatHandlerPath, "utf8");
const match = source.match(/PROVIDER_BREAKER_FAILURE_STATUSES\s*=\s*new Set\(\[([^\]]+)\]\)/);