Skip to content

fix(openai): stop retrying streamed APIError events (fixes NV-9270) - #25

Merged
ChmaraX merged 1 commit into
mainfrom
nv-9270-thalamus-stop-retrying-openai-streamed-error-events-that
Nov 2, 2026
Merged

ChmaraX merged 1 commit into
mainfrom
nv-9270-thalamus-stop-retrying-openai-streamed-error-events-that

Conversation

@ChmaraX

@ChmaraX ChmaraX commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Why

openai 7.6 and newer throw a named SSE event: error frame as an APIError with no HTTP status, before Thalamus's parser sees it (openai/openai-node#2409). isTransientStreamError only stopped on a 4xx status, so a rate-limit or invalid-key error reported in the stream was treated as a dropped connection. After response.created, the provider resumed the failed response through GET /responses/{id} up to 3 times before it failed.

What changed

  • isTransientStreamError treats an APIError with no status as terminal. APIConnectionError and APIConnectionTimeoutError, which also have no status, are checked first and still reconnect. 5xx responses also still reconnect.
  • classifyOpenAIFailure is now the one code/message → ThalamusError classifier. The parser's case "error" and the provider's mapError both use it.
  • mapError passes a ThalamusError through unchanged and returns ThalamusError. The two inline instanceof ThalamusError ? err : mapError(...) copies in the provider now call mapError directly.
  • The vi.mock("openai") factories spread the real module ({ ...actual, default: MockOpenAI }), so new SDK imports in the provider don't need test edits.

Scope

  • Thalamus only. No public types or config change. mapOpenAIError now returns the narrower ThalamusError type.
  • The parser's case "error" path stays. The edge observer and older SDKs still reach it, and it is still covered in openai.errors.test.ts.
  • The openai dev dependency pin to ^7.44.0 already landed in refactor(openai): sign Bedrock with the SDK provider fixes NV-9271 #24, so this PR doesn't touch package.json or the lockfile.

Blast radius

On openai 7.6 and newer, an in-stream OpenAI error now fails the run immediately with the mapped error, such as ProviderRateLimitError or ProviderAuthError, and makes no reconnect requests. A mid-stream server_error frame now also fails the run instead of resuming. That matches how the parser already handled it on openai 6.x. Novu doesn't construct this provider today.

Verification

  • pnpm typecheck, pnpm test (261 tests), and pnpm build pass. Biome on tracked files finds only the existing warnings.
  • New openai.streamed-error.test.ts runs a real openai client against a fake fetch. Rate-limit and invalid-key event: error frames fail with the mapped error and make no GET /responses/{id} calls. A dropped socket after response.created still resumes.
  • New cases in openai.reconnect.test.ts make the first resume request fail with APIConnectionError, APIConnectionTimeoutError, or a 503, and the send still completes. With the APIConnectionError check removed, the two connection-error cases fail.

RetriggerView in GreptileConfidence Score: 5/5 Tier: plus

The PR appears safe to merge; no actionable issues were found.

Summary

Stops reconnecting after streamed provider errors while keeping retries for connection failures and HTTP 5xx errors.

  • OpenAI stream errors stop retries while network drops still reconnect.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["Stream throws"] --> B{"Connection error?"}
  B -->|Yes| C["Keep reconnect eligible"]
  B -->|No| D{"Statusless APIError or HTTP 4xx?"}
  D -->|Yes| E["Stop reconnecting"]
  D -->|No| C
  E --> F["Map error and fail send"]
Loading

Reviews (1) · Last reviewed commit: "fix(openai): stop retrying streamed APIE..." · Reviewed by Greptile

openai 7.6 and newer throw a named SSE error as APIError with no HTTP status. Treat that as a provider failure, and classify it with the same helper the parser uses. Connection errors, timeouts and 5xx responses still reconnect.

mapError now passes ThalamusError through, so the provider maps errors in one place. The openai test mocks spread the real module instead of listing exports.
@linear-code

linear-code Bot commented Nov 2, 2026

Copy link
Copy Markdown

NV-9270

@ChmaraX
ChmaraX merged commit a036bf6 into main Nov 2, 2026
5 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.

1 participant