Skip to content

fix(sse): label HTTP 499 disconnects as client_disconnected - #8552

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.49from
DinonowDev:fix/8535-client-disconnect-error-label
Jul 26, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.49from
DinonowDev:fix/8535-client-disconnect-error-label

Conversation

@DinonowDev

@DinonowDev DinonowDev commented Jul 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Map HTTP 499 to client_disconnected in ERROR_TYPES / default messages so disconnects are not logged as invalid_request_error with an empty code.
  • Let buildErrorBody() keep an optional caller-supplied type/code instead of always re-deriving from the status table.
  • Pass stream-failure classification through from createStreamFailureFinalizers so abort handling reaches the request log correctly.

Fixes #8535

Test plan

  • node --import tsx/esm --test tests/unit/error-config.test.ts tests/unit/stream-failure-499-classification.test.ts tests/unit/error-message-sanitization.test.ts (42/42 pass)
  • CI unit / lint on the PR

Made with Cursor

Preserve caller-supplied error type/code in buildErrorBody so stream
abort classification is not overwritten by the status-code table.
@DinonowDev
DinonowDev requested a review from diegosouzapw as a code owner July 25, 2026 09:34
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks for the fix — this closes a real gap: getErrorInfo(499) had no entry in ERROR_TYPES, so it fell through to {type: "invalid_request_error", code: ""} even though the internal errorCode on the same failure event was already client_disconnected. Your change makes the JSON body match the log.

I re-ran the included tests plus the neighboring 499/combo/chatCore suites on the PR head and everything passes (114 tests total), and typecheck:core is clean. I also checked that nothing in the codebase keys off the old 499 shape (invalid_request_error + empty code), so this doesn't look like it breaks any existing consumer.

One tiny, non-blocking nit for a future pass: docs/security/ERROR_SANITIZATION.md documents buildErrorBody's optional 3rd argument (upstreamDetails) but not the new optional 4th argument (classification) — worth a one-line addition whenever convenient, doesn't need to hold up this PR.

@DinonowDev
DinonowDev force-pushed the fix/8535-client-disconnect-error-label branch from 8ad17ed to 6498f12 Compare July 25, 2026 14:55
@DinonowDev
DinonowDev force-pushed the fix/8535-client-disconnect-error-label branch from 6498f12 to 0061d42 Compare July 25, 2026 14:55
@DinonowDev

Copy link
Copy Markdown
Contributor Author

Thanks for the fix — this closes a real gap: getErrorInfo(499) had no entry in ERROR_TYPES, so it fell through to {type: "invalid_request_error", code: ""} even though the internal errorCode on the same failure event was already client_disconnected. Your change makes the JSON body match the log.

I re-ran the included tests plus the neighboring 499/combo/chatCore suites on the PR head and everything passes (114 tests total), and typecheck:core is clean. I also checked that nothing in the codebase keys off the old 499 shape (invalid_request_error + empty code), so this doesn't look like it breaks any existing consumer.

One tiny, non-blocking nit for a future pass: docs/security/ERROR_SANITIZATION.md documents buildErrorBody's optional 3rd argument (upstreamDetails) but not the new optional 4th argument (classification) — worth a one-line addition whenever convenient, doesn't need to hold up this PR.

Thanks for the thorough review — really appreciate you checking the tests, typecheck, and that nothing keyed off the old 499 shape.

Addressed the docs nit in 0061d42: docs/security/ERROR_SANITIZATION.md now documents the optional 4th classification argument on buildErrorBody (caller-supplied type/code wins over the status-derived defaults, e.g. HTTP 499 → client_disconnected).

@diegosouzapw
diegosouzapw merged commit 553a073 into cheaper-inference:release/v3.8.49 Jul 26, 2026
14 of 15 checks passed
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…inference#8552)

* fix(sse): label HTTP 499 disconnects as client_disconnected

Preserve caller-supplied error type/code in buildErrorBody so stream
abort classification is not overwritten by the status-code table.

* docs(security): document buildErrorBody classification arg

---------

Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…inference#8552)

* fix(sse): label HTTP 499 disconnects as client_disconnected

Preserve caller-supplied error type/code in buildErrorBody so stream
abort classification is not overwritten by the status-code table.

* docs(security): document buildErrorBody classification arg

---------

Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
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.

fix(sse): 499 client disconnects labelled invalid_request_error — buildErrorBody discards caller's classification

2 participants