From 3b4f4afc9d2c75aa71b49b06eae95bc64b0165d6 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 24 Jul 2026 09:56:51 -0300 Subject: [PATCH] fix(sse): re-export PROVIDER_BREAKER_FAILURE_STATUSES for the orphaned all-rate-limited breaker path (#8390) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/sse/handlers/chat.ts | 1 + src/sse/handlers/chatPredicates.ts | 2 +- tests/unit/nvidia-quota-phase1.test.ts | 9 ++++++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 161321117e..20e96b2684 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -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"; diff --git a/src/sse/handlers/chatPredicates.ts b/src/sse/handlers/chatPredicates.ts index a45dbc4d5a..fd14015c8d 100644 --- a/src/sse/handlers/chatPredicates.ts +++ b/src/sse/handlers/chatPredicates.ts @@ -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 — diff --git a/tests/unit/nvidia-quota-phase1.test.ts b/tests/unit/nvidia-quota-phase1.test.ts index ef09091d01..346868be07 100644 --- a/tests/unit/nvidia-quota-phase1.test.ts +++ b/tests/unit/nvidia-quota-phase1.test.ts @@ -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\(\[([^\]]+)\]\)/);