Skip to content

fix(sse): repair orphaned PROVIDER_BREAKER_FAILURE_STATUSES reference on the all-rate-limited path - #8390

Merged
diegosouzapw merged 1 commit into
release/v3.8.49from
fix/basered-breaker-statuses-orphan
Jul 24, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.49from
fix/basered-breaker-statuses-orphan

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

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 = new Set([408, 500, 502, 503, 504]) with it.

A second, independent use of that const survived in chat.ts's handleSingleModelChat(), in the "all credentials rate-limited" block (~line 1340):

if (!forceLiveComboTest && credentials?.allRateLimited && PROVIDER_BREAKER_FAILURE_STATUSES.has(breakerFailureStatus)) {
  breaker._onFailure();
}

The extraction removed the local declaration but left this second reference behind, orphaned.

Production impact

Any request where every credential for a provider+model is simultaneously rate-limited hits ReferenceError: PROVIDER_BREAKER_FAILURE_STATUSES is not defined at runtime in this code path. Concretely:

  • breaker._onFailure() was unreachable on the all-rate-limited path — the provider circuit breaker could never trip from it.
  • The uncaught 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.

This sits in the central chat resilience path (handleSingleModelChat), so it fires for any provider whenever all of its credentials are simultaneously cooling down — not an edge case.

Fix

Two-line production fix, no behavior change:

  • src/sse/handlers/chatPredicates.ts: export PROVIDER_BREAKER_FAILURE_STATUSES.
  • src/sse/handlers/chat.ts: add it to the existing import block from ./chatPredicates.

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 — a stale test left behind by the same #8013 extraction). The regex and the classification assertions are unchanged; the test still proves 429 is excluded from the whole-provider breaker.

Validation (TDD — red before fix, green after)

All 6 affected files run sequentially with env -u OMNIROUTE_API_KEY:

Test file Before After
tests/unit/chat-rate-limit-body-lock.test.ts 0/2 (2 ReferenceError) 2/2
tests/unit/chat-cooldown-aware-retry.test.ts 4/6 (2 ReferenceError) 6/6
tests/unit/chat-combo-live-test.test.ts 4/5 (1 ReferenceError) 5/5
tests/unit/chat-route-coverage.test.ts 12/15 (503→502 masked + 2 ReferenceError) 15/15
tests/unit/issue-agent-route-execution.test.ts 1/2 (400 instead of 429) 2/2
tests/unit/nvidia-quota-phase1.test.ts 12/13 (regex found no declaration) 13/13
Total 33/43 43/43

Also verified:

  • npm run typecheck:core — clean.
  • npx eslint on the 3 changed files — clean.
  • Adjacent tests/unit/combo-breaker-429.test.ts (uses a deliberately independent, locally-declared const of the same name in open-sse/services/combo/comboPredicates.ts, to avoid an open-sse → src/sse import cycle) — unaffected, still 9/9 green. Confirms this fix is contained to the orphaned single-model path and does not touch the separate combo-layer classification.

Refs #8013

…d all-rate-limited breaker path

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
@diegosouzapw
diegosouzapw merged commit 3b4f4af into release/v3.8.49 Jul 24, 2026
10 checks passed
@diegosouzapw
diegosouzapw deleted the fix/basered-breaker-statuses-orphan branch July 24, 2026 13:03
MumuTW added a commit to MumuTW/OmniRoute that referenced this pull request Jul 25, 2026
… import

Rebased onto release/v3.8.49 and reduced to the test file only — the
production fix landed in diegosouzapw#8390. Asserts chatPredicates exports the Set
with the documented statuses and that chat.ts imports the symbol cleanly.
MumuTW added a commit to MumuTW/OmniRoute that referenced this pull request Jul 26, 2026
… import

Rebased onto release/v3.8.49 and reduced to the test file only — the
production fix landed in diegosouzapw#8390. Asserts chatPredicates exports the Set
with the documented statuses and that chat.ts imports the symbol cleanly.
diegosouzapw pushed a commit that referenced this pull request Jul 27, 2026
…8611)

The `compat-build-26` job in nightly-compat.yml is the only place in the CI
matrix that runs `npm run build` on Node 26 (ci.yml pins CI_NODE_VERSION=24).
It failed every nightly with the runner-reclaimed signature ("The runner has
received a shutdown signal" / "The operation was canceled", no exit code),
always at the same Turbopack compile phase — the classic OOM-kill pattern on
the memory-constrained ubuntu-latest runner.

Root cause: Turbopack's native (Rust, off-V8-heap) allocation is not bounded by
--max-old-space-size and peaks far higher than webpack on OmniRoute's large
module graph (#6409), heavier still under Node 26. Raising the heap does not
help — the codebase's own documented escape hatch for RAM-constrained
environments is the webpack fallback (OMNIROUTE_USE_TURBOPACK=0; see
docs/reference/ENVIRONMENT.md and scripts/build/build-next-isolated.mjs).

Wire that fallback into the Node 26 compat build: it still validates the app
builds on Node 26 (the point of the job) at a much lower memory peak.
Turbopack-on-Node-24 stays covered by ci.yml's build job.

Adds a regression guard (tests/unit/nightly-compat-node26-webpack-8090.test.ts)
asserting the job keeps the webpack fallback so it cannot silently regress.

Class 1 of the triage (shard test failures) was already resolved by #8390,
#8386, #8381, #8383.

Closes #8090
Refs #6949 #6409
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…d all-rate-limited breaker path (diegosouzapw#8390)

Root cause: diegosouzapw#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 diegosouzapw#8013
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…iegosouzapw#8611)

The `compat-build-26` job in nightly-compat.yml is the only place in the CI
matrix that runs `npm run build` on Node 26 (ci.yml pins CI_NODE_VERSION=24).
It failed every nightly with the runner-reclaimed signature ("The runner has
received a shutdown signal" / "The operation was canceled", no exit code),
always at the same Turbopack compile phase — the classic OOM-kill pattern on
the memory-constrained ubuntu-latest runner.

Root cause: Turbopack's native (Rust, off-V8-heap) allocation is not bounded by
--max-old-space-size and peaks far higher than webpack on OmniRoute's large
module graph (diegosouzapw#6409), heavier still under Node 26. Raising the heap does not
help — the codebase's own documented escape hatch for RAM-constrained
environments is the webpack fallback (OMNIROUTE_USE_TURBOPACK=0; see
docs/reference/ENVIRONMENT.md and scripts/build/build-next-isolated.mjs).

Wire that fallback into the Node 26 compat build: it still validates the app
builds on Node 26 (the point of the job) at a much lower memory peak.
Turbopack-on-Node-24 stays covered by ci.yml's build job.

Adds a regression guard (tests/unit/nightly-compat-node26-webpack-8090.test.ts)
asserting the job keeps the webpack fallback so it cannot silently regress.

Class 1 of the triage (shard test failures) was already resolved by diegosouzapw#8390,
diegosouzapw#8386, diegosouzapw#8381, diegosouzapw#8383.

Closes diegosouzapw#8090
Refs diegosouzapw#6949 diegosouzapw#6409
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…d all-rate-limited breaker path (diegosouzapw#8390)

Root cause: diegosouzapw#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 diegosouzapw#8013
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#8611)

The `compat-build-26` job in nightly-compat.yml is the only place in the CI
matrix that runs `npm run build` on Node 26 (ci.yml pins CI_NODE_VERSION=24).
It failed every nightly with the runner-reclaimed signature ("The runner has
received a shutdown signal" / "The operation was canceled", no exit code),
always at the same Turbopack compile phase — the classic OOM-kill pattern on
the memory-constrained ubuntu-latest runner.

Root cause: Turbopack's native (Rust, off-V8-heap) allocation is not bounded by
--max-old-space-size and peaks far higher than webpack on OmniRoute's large
module graph (diegosouzapw#6409), heavier still under Node 26. Raising the heap does not
help — the codebase's own documented escape hatch for RAM-constrained
environments is the webpack fallback (OMNIROUTE_USE_TURBOPACK=0; see
docs/reference/ENVIRONMENT.md and scripts/build/build-next-isolated.mjs).

Wire that fallback into the Node 26 compat build: it still validates the app
builds on Node 26 (the point of the job) at a much lower memory peak.
Turbopack-on-Node-24 stays covered by ci.yml's build job.

Adds a regression guard (tests/unit/nightly-compat-node26-webpack-8090.test.ts)
asserting the job keeps the webpack fallback so it cannot silently regress.

Class 1 of the triage (shard test failures) was already resolved by diegosouzapw#8390,
diegosouzapw#8386, diegosouzapw#8381, diegosouzapw#8383.

Closes diegosouzapw#8090
Refs diegosouzapw#6949 diegosouzapw#6409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant