Repository navigation
fix(resilience): a request-translation failure no longer cools the account or trips the provider breaker (#14815) - #14830
Merged
diegosouzapw merged 3 commits intoSep 25, 2026
Conversation
…count or trips the provider breaker (#14815) Translation runs locally on the client's body before any upstream call. When it threw, chatCore returned a bare 500 that chat.ts handled like an upstream failure: markAccountUnavailable cooled the account (a single-account install then answered every request with "all accounts rate limited") and the 500 counted toward the provider-wide breaker. The same body fails identically on every account, so rotating is pointless as well. createTranslationFailureResult() now labels the result with the internal errorCode request_translation_failed (the client body is unchanged); shouldSkipConnDisable() and shouldTripProviderBreakerForResult() skip it, so the error goes straight back to the client. Refs #14815
…ation-failure-no-account-cooldown
fouadSalkini
added a commit
to fouadSalkini/OmniRoute
that referenced
this pull request
Sep 26, 2026
Slice 1/3 copied src/shared/utils/apiKeyPolicy.ts, its test file and stryker.conf.json from an older snapshot, silently undoing base work that landed after that snapshot. Re-apply the base versions and keep only this PR's own additions (self-service settings on apiKeyInfo, the env-key forwarding default, two new policy tests, one stryker test entry). Restored: - apiKeyPolicy.ts: formatResetDurationSuffix, the "Resets in Xh Ym." message suffixes and retryAfter on the budget, token-limit and request-limit 429 responses (cheaper-inference#14188) - tests/unit/api-key-policy.test.ts: readErrorBody, the reset-timing assertions and the "returns the token-limit reset instant" test (cheaper-inference#14188) - stryker.conf.json tap.testFiles: quota-reset-timing and combo-skipped-reset-timing (cheaper-inference#14188), translation-failure-skips-account-cooldown-14815 (cheaper-inference#14830), sudo-password-never-reaches-command-stdin (cheaper-inference#14836)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #14815
Problem
In #14815 a single large image made request translation throw
Maximum call stack size exceeded. chatCore turned that into a bare 500, andchat.tshandled it like an upstream failure:markAccountUnavailable()put the account into cooldown. On a single-account install, every later request, including ones that would translate fine, gotall 1 active accounts rate limited (reset after 5s).Translation runs locally on the client's own body, before any upstream call. The failure says nothing about the account, and every account fails the same body the same way, so rotating accounts does not help either.
Fix
createTranslationFailureResult()sets the internal labelerrorCode: "request_translation_failed"(TRANSLATION_FAILURE_CODE) on the result object. The body the client receives is unchanged.shouldSkipConnDisable()(account cooldown) andshouldTripProviderBreakerForResult()(provider breaker) skip that label, the same way they already skipplugin_blockandclient_disconnected. The error goes straight back to the client.Not in this PR
The stack overflow itself did not reproduce. I tested on the current tip and on
release/v3.8.50, callingtranslateRequestdirectly and going throughhandleChatCorewith a claude OAuth connection, using random-noise images up to 9 MB base64, with stream on and off and compression on and off. The reporter's settings must reach a code path these runs did not hit. This PR removes the availability impact; finding the root cause needs the stack trace from #14815.Validation
tests/unit/translation-failure-skips-account-cooldown-14815.test.ts(5 cases): the result carries the label and keeps the client body unchanged, the cooldown gate and the provider-breaker gate skip it, and an ordinary upstream 500 still cools the account and still trips the breaker. With the two predicates restored from the base, the gate cases fail (red→green shown locally).typecheck:core0 errors,check:open-sse-typecheckOK,check:cyclesOK. The neighbouring breaker/cooldown suites pass:combo-skip-conn-disable-plugin-block-7806,breaker-network-error-guard,circuit-breaker-local-execution,circuit-breaker-abort-provider-trip-7907andrate-limit-local-error-classification.check:mutation-test-coverageentries it still reports (flagship-0day-discovery, metered-budget…, expiry-first…, claude-passthrough-empty-response) come from the tip, not from this PR.