Skip to content

fix(sse): keep HTTP status and retry hint in post-keepalive /v1/responses error frames - #14632

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
QuangBlue:fix/responses-error-frame-status
Sep 24, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
QuangBlue:fix/responses-error-frame-status

Conversation

@QuangBlue

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #14547

Symptom

/v1/responses streaming requests that go through the early keepalive (#13431) are committed to
HTTP 200 after DEFAULT_THRESHOLD_MS. When the handler then returns an error response (for
example a 403 "model not allowed for this key", a 429 with Retry-After, or a 503), the client
receives a single {"type":"error", code, message, param, sequence_number} frame. The HTTP
status, the Retry-After hint and error.type are gone, so a Responses client cannot tell a
permanent failure from a transient one. In practice clients retried permanent 403s and could not
back off correctly on 429/503.

Root cause

open-sse/utils/earlyStreamKeepalive.ts, non-SSE branch of the slow path: the handler's error
Response is read as text and reshaped by buildResponsesErrorDataLine(text), which only keeps
code, message, param (plus diagnostics). The status and headers of that Response are
never looked at.

Fix

The post-commit Responses error frame now also carries:

  • status_code: the handler response status (always present in this branch).
  • error_type: error.type from the body, when present.
  • retry_after_seconds: Retry-After (delta-seconds or HTTP-date), else
    x-omniroute-retry-after-seconds, clamped to 0..3600; omitted when neither header is usable.

Design notes:

  • error_type, not type: top-level type is the Responses event discriminator.
  • Flat fields, not a nested error object: openai-node throws on an event with a top-level
    error key instead of yielding it, which would undo fix(api): frame post-keepalive /v1/responses stream errors with a type field #13431.
  • Chat Completions and Anthropic frames, and the fast path (response before the threshold), are
    unchanged. Only additive fields; existing keys keep their values.

Follow-up of #13431; related #14377.

Tests

  • New: tests/unit/responses-post-keepalive-error-frame-status.test.ts (red before the fix,
    green after): 429 with Retry-After delta-seconds, HTTP-date Retry-After clamped to 3600,
    the x-omniroute-retry-after-seconds fallback, 403 without a retry hint, a body without
    error.type, and unchanged fast path and chat frames.
  • node --import tsx/esm --test on the new test plus
    issue-13431-responses-post-keepalive-error-frame, early-stream-keepalive,
    earlyStreamKeepalive, responses-route-early-keepalive-wiring, keepalive-threshold:
    54 tests, 52 pass, 2 pre-existing skips, 0 fail.
  • eslint (with the repo suppressions) clean on both files.

@QuangBlue
QuangBlue force-pushed the fix/responses-error-frame-status branch from 03a3194 to df0180f Compare September 23, 2026 11:55
…nses error frames

Once withEarlyStreamKeepalive commits /v1/responses to HTTP 200, a handler error
response is framed as {type:error, code, message, param}. The HTTP status,
Retry-After and error.type were dropped, so clients could not tell a permanent
failure (403 model not allowed) from a transient one (429/503) and retried
permanent errors. The frame now also carries status_code, error_type and
retry_after_seconds (Retry-After or x-omniroute-retry-after-seconds, clamped
to 0..3600). Chat and Anthropic frames and the fast path are unchanged.

Follow-up of diegosouzapw#13431.
@QuangBlue
QuangBlue force-pushed the fix/responses-error-frame-status branch from df0180f to ab0353a Compare September 24, 2026 03:06
@diegosouzapw

Copy link
Copy Markdown
Owner

Nice fix — the design notes on error_type vs type and flat fields (avoiding what would undo
#13431) show real care. Heads up: this PR is now CONFLICTING against the release branch
because #14572 (merged a couple hours ago) touched the exact same return statement in
buildResponsesErrorDataLine to add a real sequence_number. Both changes are compatible (no
logical overlap), so it should be a quick rebase — could you pull latest release/v3.8.51 and
resolve? Happy to re-verify once that's done.

@QuangBlue

Copy link
Copy Markdown
Contributor Author

Thanks! This one is already rebased onto the latest release/v3.8.51 (head ab0353ab9, on top of #14572): the error frame keeps sequence_number: SYNTHETIC_RESPONSES_SEQUENCE_NUMBER from #14572 and adds the status fields after it. GitHub now reports it as mergeable.

@diegosouzapw
diegosouzapw merged commit 966cba0 into diegosouzapw:release/v3.8.51 Sep 24, 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