Skip to content

fix(sse): do not replay a malformed 200 on the same account - #14955

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
shipsfromrio:fix/malformed-translated-response-no-paid-retry
Sep 28, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
shipsfromrio:fix/malformed-translated-response-no-paid-retry

Conversation

@shipsfromrio

Copy link
Copy Markdown
Contributor

Summary

  • chatCore turns a non-streaming upstream HTTP 200 into a 502 (malformed_translated_response) when the translated body has no usable output. Its error.type is upstream_response_error, and the code is upstream_empty_response, upstream_response_failed or upstream_fake_success.
  • The single-model same-account transport retry (fix(resilience): retry transient Codex transport failures on the same account before fallback #9708) treated every 502 as a transport failure that happened before any output. So it waited 2-3 s and sent the request to the same account again, even though the upstream had already answered that call with a body (and most likely billed it).
  • diagnostics.ts now exports this type as UPSTREAM_RESPONDED_ERROR_TYPE (same value), and the transport retry lists it as non-retryable. These still get their one same-account retry:
    • real transport failures before any output (502/503/504/507)
    • 502s sent by an upstream without the local marker
  • The client already sees the marker in error.type, so the response shape does not change.

Related Issues

Validation

  • Change type: routing
  • Focused test: node --import tsx/esm --test tests/unit/malformed-translated-no-same-account-retry.test.ts. 6/6 pass with the fix. With the previous sameAccountTransportRetry.ts, the 3 malformed-200 cases fail.
  • eslint and prettier on the touched files
  • Based on the current release/v3.8.51
  • Production-code change includes a new automated test

Tests Added Or Updated

  • tests/unit/malformed-translated-no-same-account-retry.test.ts (new)

Coverage Notes

  • For each of the three malformed codes, the test builds the exact result chatCore returns, using describeMalformedNonStream and createErrorResult. It checks that shouldRetrySameAccountTransport returns false. Control cases pin the retry behavior that does not change.

Reviewer Notes

  • Behavior change: a malformed 200 now goes straight to the normal account-fallback path (markAccountUnavailable, transient 502 cooldown, rotation to the next account if one exists). Before, it went there only after the same-account retry also failed. So the account is rotated one attempt earlier, and the same-account replay goes away.
  • chat.ts and the combo files are not touched. Both are frozen by check:file-size.
  • Not covered here: in combo mode, upstream_fake_success is not request-scoped, so executeTargetAttempt still retries the same target once when model lockout is off (the default). upstream_empty_response and upstream_response_failed are already request-scoped there and are not retried on the same target. A follow-up can add the same marker check to isTransient.

chatCore turns a non-streaming upstream HTTP 200 whose translated body
carries no usable output into a 502 (malformed_translated_response) with
error.type "upstream_response_error". The single-model pre-output
transport retry (diegosouzapw#9708) treated every 502 as a transport blip and
replayed the request once on the same account, so a call the upstream
had already answered (and likely billed) was paid for twice for the same
verdict.

Export the marker type as UPSTREAM_RESPONDED_ERROR_TYPE from
diagnostics.ts and list it in the transport retry's non-retryable types.
Genuine pre-output 502/503/504/507 failures, and upstream-sent 502s
without the local marker, keep their single same-account retry.
@diegosouzapw
diegosouzapw merged commit 4236117 into diegosouzapw:release/v3.8.51 Sep 28, 2026
3 checks passed
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.

2 participants