Skip to content

fix(api-gateway): classify statusless upstream timeouts and aborts as 504 - #22081

Open
cherry-coding[bot] wants to merge 1 commit into
mainfrom
fix/22075-gateway-idle-timeout-504
Open

cherry-coding[bot] wants to merge 1 commit into
mainfrom
fix/22075-gateway-idle-timeout-504

Conversation

@cherry-coding

@cherry-coding cherry-coding Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Before this PR: an Agent turn that died after the upstream stalled ended with
ClaudeCodeResultError: API Error: 500 Internal server error …, and the main process logged
ApiGatewayErrors code=UNKNOWN with the real cause next to it. The gateway's provider-error
mapping read only a numeric status; any error without one became
statusCode = status ?? 500 plus safeMessage's production fallback Internal server error, so a
recognizable upstream idle timeout / mid-stream abort reached the client as a bare 500. The 504
classification the gateway already had (streamInterruptedError) only covers the pause path.

After this PR: the three generation transformers share one normalization step. An existing numeric
status keeps its precedence and its provider message (a real 401/429/500/503 is untouched). Without a
status, a TimeoutError, or an exact message of Upstream idle timeout exceeded /
Stream idle timeout exceeded / Idle timeout exceeded, returns 504 “Upstream request timed out”;
AbortError, ResponseAborted or string code: 'ABORT_ERR' returns 504 “Upstream stream aborted”;
anything else stays the generic production 500. The new causes are fixed strings, so no statusless
provider text, stack, url or request/response body reaches the client. Streaming responses that
already committed HTTP 200 keep their status and emit the classified reason in the terminal dialect
error frame instead.

before / after: gateway timeout and abort classification

Related to #22075

Why we need it and why it was done in this way

The report is “the app knows the cause, the user gets a bare 500”. The classification belongs at the
boundary that already owns provider-error normalization (errors.ts), because that is the single
place both the pending HTTP response and the committed SSE frame pass through — fixing it per
transport would duplicate the policy three times.

The discriminator set is deliberately narrow, and this is the tradeoff worth reviewing:

  • Statusless errors are matched on the identity that actually survives to this boundary (name,
    string code) or on an exact whole-string message. Substring/regex matching (/timeout/i),
    network categories, ETIMEDOUT/ECONNRESET and numeric DOMException codes are all
    deliberately rejected, so database timeout, MCP timeout, timeout reading /path/to/secret and
    a prefixed or case-changed idle phrase all stay generic 500.
  • The exact-message rule admits a semantic collision: an unrelated internal error carrying the same
    phrase is indistinguishable here. It is included because the reported evidence is text-only and the
    emitted message is fixed, so the collision cannot leak anything. Zero-collision classification
    would need upstream provenance — a different change.
  • Where the class is attached was considered and rejected: putting statusCode: 504 on the timer
    error instead also changes the renderer error block, the persisted execution failure and the
    channel adapters for ordinary Chat/Agent turns, and it does not reach upstream-produced errors at
    all. The local idle timer normally takes the already-correct pause path, which was measured before
    writing any code.
  • Alternatives considered: reusing classifyErrorCategory (its network bucket also contains plain
    connectivity failures and has no status policy), importing the AI SDK's isAbortError (its
    instanceof check fails on the serialized plain objects this boundary receives).

Agent Note / Spec

Agent Note: N/A — automation-authored PR (cherry-coding[bot]); the tradeoff record is the section above
Spec PR: N/A — local bug fix in one existing module
Acceptance criteria: a statusless timeout/abort error from the gateway mapping returns 504 with a fixed classified reason instead of 500 “Internal server error”; an error carrying a real provider status keeps that status and message; an unmatched statusless internal error stays a generic 500 in production.

Verification

  • pnpm exec vitest run --project main src/main/features/apiGateway/__tests__/errors.http.test.ts src/main/features/apiGateway/__tests__/proxyStream.stream.test.ts — 914 tests pass. The new file drives a real Elysia app on a real @elysia/node socket with real fetch, real serializeError, real withIdleTimeout/pipeStreamLoop, the installed OpenAI-compatible SDK against local HTTP fixtures, and the real buildStreamErrorFrame; every case runs over all four wire formats (anthropic / openai chat / openai responses / gemini) in three states (pending JSON, before commitment, after commitment = HTTP 200 + terminal frame). 822 measured wire observations are written to an evidence JSON.
  • Fix-before comparison: the same two test files (hashes recorded as identical) run against the baseline errors.ts restored byte-for-byte from 6550a6c9 fail with 409 of 914 — the 360 classification observations assert 504 / fixed cause and observe 500 / Internal server error, and the four new consumer cases fail the same way. Controls (provider-401/429/500/503, database timeout, MCP timeout, prefixed phrases, ETIMEDOUT, sanitized-primitive and non-object input, REST/MCP vocabulary) pass on both sides.
  • pnpm exec tsc --noEmit -p tsconfig.node.json --composite false, pnpm exec oxlint --deny-warnings, pnpm exec eslint, pnpm exec oxfmt --check on the changed files, pnpm docs:check, git diff --check — all clean.
  • The full --project main suite was also run before opening this PR.

What I did not reproduce: the original Windows 2.1.4 event. The report is a redacted diagnostic package; the exact error object that reached the gateway there is not known, and the reported string does not exist anywhere in this repository (it comes from the upstream provider path). I measured the shapes this boundary can receive (real SDK SSE error object, real fetch timeout, real abort, real provider HTTP errors, and the serializer's lossy projections) rather than claiming the reported incident. An Electron/Work run was not made, and whether the embedded Claude CLI keeps the new reason (or still appends its loopback-gateway sentence) is not proven by this change — that text lives in the CLI binary, not here. The 20-minute production idle budget is not exercised; a real upstream stall of that length takes the pause path, which already returned 504 and is unchanged.

Breaking changes

No API-shape change. Statusless provider failures that used to answer 500 now answer 504 with a fixed
message; client code that treated any 500 as “gateway broken” will now see a timeout class.

Special notes for your reviewer

  • proxyStream.ts, AiStreamManager's retry budget, REST/MCP handlers and safeMessage are
    untouched on purpose — the diff is one helper, three call sites, tests and the gateway doc.
  • The exact-message list is the only heuristic in the change; if you would rather ship identity-only
    classification, the three-string list is one if to remove (it is also the only part that covers
    the reported text-only evidence).
  • docs/references/api-gateway/README.md now describes status-first normalization with the bounded
    statusless exception; no translated pair exists for that reference domain.
Agent turns that fail after an upstream stall now end with a classified timeout/abort reason (504) instead of a bare 500 "Internal server error".

Provider/runtime errors that carry no HTTP status used to collapse to
500 "Internal server error", so a recognizable upstream idle timeout or
mid-stream abort reached the client as a bare 500 with no cause.

Add one private normalization step shared by the anthropic, openai and
google transformers: an existing numeric status keeps its precedence and
its provider message; otherwise TimeoutError, or an exact message of
"Upstream idle timeout exceeded" / "Stream idle timeout exceeded" /
"Idle timeout exceeded", maps to 504 "Upstream request timed out";
otherwise AbortError, ResponseAborted or string code ABORT_ERR maps to
504 "Upstream stream aborted"; anything else stays the generic 500.
The new causes are fixed strings, so no statusless provider text or
AI-SDK diagnostic extra reaches the client. REST, MCP, the pause-504
path in proxyStream and the zero-retry budget are unchanged.

Verification: real Elysia/@elysia/node socket suite plus the existing
processMessage commitment harness, both green; the same tests run against
the baseline mapper fail as expected (409 of 914). Docs updated.

Signed-off-by: cherry-coding[bot] <342372629+cherry-coding[bot]@users.noreply.github.com>

@cherry-pr-review cherry-pr-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technical review supports approval at 67598bb46b68e394fdc21d749f4e291094380606. No confirmed defects: numeric provider statuses retain precedence, recognized statusless interruptions receive fixed 504 reasons, and JSON/SSE delivery follows the stated contract.

Focused local verification passed 917 tests across three files. Recorded head CI succeeded for basic, general, render, main-shard and macOS/Windows checks; renderer-shard and package checks were skipped. No synthetic-merge check runs were recorded; legacy pending status has zero contexts.

The original Windows incident, embedded Claude CLI presentation and full production idle interval were not reproduced. No Electron or full-suite run was performed.

Cherry Review · Passed

no findings

This branch has not been deployed

No deployments
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.

0 participants