Skip to content

fix(chatcore): report string-reason client aborts as 499, not 502 (#7907) - #8011

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
Long-Feeds:fix/7907-chatcore-499-status
Jul 22, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
Long-Feeds:fix/7907-chatcore-499-status

Conversation

@Long-Feeds

Copy link
Copy Markdown
Contributor

Summary

Focused status-normalization follow-up to #7908 for #7907, as offered in the #7934 closing thread.

#7908 fixed the resilience side: client aborts no longer trip the provider circuit breaker or cool down healthy connections. What it intentionally left untouched is chatCore's error surface — when abort(reason) rejects the upstream fetch with a raw string reason ("request_signal_aborted" from streamHandler's default, or "Client disconnected: ..."), the string has no name/status, so the catch block's error.name === "AbortError" check misses it and the abort falls through to the 502 provider-failure default. The client's own disconnect is then recorded as FAILED 502 / Bad Gateway in the response body, request logs, and usage records.

Change

  • open-sse/handlers/chatCore.ts: classify the caught error once with the existing isLocalStreamLifecycleError helper (which fix(resilience): don't cool down accounts or trip the breaker on client aborts #7908 expanded to cover AbortError plus the known abort reason strings) and use that for the 499 status/message mapping and the early-return path. No new classifier is introduced.
  • tests/unit/chatcore-translation-paths.test.ts: new integration test chatCore maps raw string abort reasons to 499, not 502 (#7907) — throws a bare "request_signal_aborted" string from the upstream fetch and asserts status: 499, error: "Request aborted".
  • changelog.d/fixes/7907-chatcore-499-string-abort.md.

Verification

  • RED first: on release/v3.8.49 (124557c, post-fix(resilience): don't cool down accounts or trip the breaker on client aborts #7908) the new test fails with actual: 502, expected: 499.
  • GREEN after: tests/unit/chatcore-translation-paths.test.ts — 62/62 pass.
  • Neighbors: circuit-breaker-abort-provider-trip-7907, circuit-breaker-client-abort, circuit-breaker-failure-kind, combo-499-abort, chatcore-compression-combo-predicates, proxyfetch-caller-abort-log-suppression — 30/30 pass.
  • check:changelog-integrity OK; eslint on both touched files reports the same pre-existing count as base (no new findings).

Refs #7907. Follow-up to #7908; supersedes the telemetry half of the closed #7934.

…eaper-inference#7907)

abort(reason) rejects the upstream fetch with the raw reason, which is
often a bare string ("request_signal_aborted", "Client disconnected: ...")
carrying no `name` or `status`. The chatCore catch block only recognized
`error.name === "AbortError"`, so those aborts fell through to the 502
provider-failure default and were surfaced as `FAILED 502 / Bad Gateway`
in the client response, request logs, and usage records.

Classify the caught error with the existing isLocalStreamLifecycleError
helper (expanded by cheaper-inference#7908 to cover AbortError plus the known abort reason
strings) so every client-abort shape maps to `499 Request aborted`.

Status-normalization follow-up to cheaper-inference#7908, which already excluded these
aborts from provider circuit-breaker and cooldown accounting.
@Long-Feeds
Long-Feeds requested a review from diegosouzapw as a code owner July 21, 2026 15:45
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Verified this against the current release tip: chatCore's failure-status mapping (lines ~3113-3157) still only checks error.name === "AbortError", so a raw-string abort reason does fall through to the 502 default exactly as described, and #7908 (already merged) never touched this file — it fixed the separate cooldown/breaker-accounting layer. Reusing isLocalStreamLifecycleError() instead of adding new classification logic is the right call since #7908 already extended it to cover these string shapes. Confirmed #7934 is closed and #8011 correctly covers the remaining, non-overlapping scope. Test lands in the right runner/file and reproduces the exact bug. No concerns — merge-ready.

@Long-Feeds

Copy link
Copy Markdown
Contributor Author

Thanks for the review! One small note: the three PR workflows (Quality Gates, semgrep, DAST smoke) are still sitting in action_required — first-time-contributor runs need a maintainer to click "Approve and run" before they execute. Everything was pre-verified locally (62/62 on the touched test file, 30/30 on the breaker/abort neighbors, changelog integrity OK), so they should go green once approved.

@diegosouzapw
diegosouzapw merged commit 1699933 into cheaper-inference:release/v3.8.49 Jul 22, 2026
5 checks passed
@diegosouzapw diegosouzapw mentioned this pull request Jul 23, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…eaper-inference#7907) (cheaper-inference#8011)

abort(reason) rejects the upstream fetch with the raw reason, which is
often a bare string ("request_signal_aborted", "Client disconnected: ...")
carrying no `name` or `status`. The chatCore catch block only recognized
`error.name === "AbortError"`, so those aborts fell through to the 502
provider-failure default and were surfaced as `FAILED 502 / Bad Gateway`
in the client response, request logs, and usage records.

Classify the caught error with the existing isLocalStreamLifecycleError
helper (expanded by cheaper-inference#7908 to cover AbortError plus the known abort reason
strings) so every client-abort shape maps to `499 Request aborted`.

Status-normalization follow-up to cheaper-inference#7908, which already excluded these
aborts from provider circuit-breaker and cooldown accounting.

Co-authored-by: xiaolong.835 <xiaolong.835@bytedance.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…eaper-inference#7907) (cheaper-inference#8011)

abort(reason) rejects the upstream fetch with the raw reason, which is
often a bare string ("request_signal_aborted", "Client disconnected: ...")
carrying no `name` or `status`. The chatCore catch block only recognized
`error.name === "AbortError"`, so those aborts fell through to the 502
provider-failure default and were surfaced as `FAILED 502 / Bad Gateway`
in the client response, request logs, and usage records.

Classify the caught error with the existing isLocalStreamLifecycleError
helper (expanded by cheaper-inference#7908 to cover AbortError plus the known abort reason
strings) so every client-abort shape maps to `499 Request aborted`.

Status-normalization follow-up to cheaper-inference#7908, which already excluded these
aborts from provider circuit-breaker and cooldown accounting.

Co-authored-by: xiaolong.835 <xiaolong.835@bytedance.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.

2 participants