Skip to content

fix: propagate SSE errors as stream terminations instead of silent parts - #11

Open
danielxxomg wants to merge 3 commits into
brent-weatherall:mainfrom
danielxxomg:feat/sse-error-handling
Open

fix: propagate SSE errors as stream terminations instead of silent parts#11
danielxxomg wants to merge 3 commits into
brent-weatherall:mainfrom
danielxxomg:feat/sse-error-handling

Conversation

@danielxxomg

Copy link
Copy Markdown

Summary

Changes SSE error handling so that error events and network read failures terminate the stream via controller.error() instead of being silently enqueued as {type: "error"} stream parts.

Depends on: PR #10 (retry/backoff core) — this PR is stacked on top of it.

Problem

When the Command Code API emits an SSE error event or the network connection drops mid-stream, the original code enqueues the error as a stream part:

// Before (silent — AI SDK never sees the failure)
controller.enqueue({ type: "error", error: event.error ?? ... })

The AI SDK treats {type: "error"} as just another content part and never triggers its own error handling or retry logic. The stream completes "successfully" with an error buried in the content.

Solution

// After (real termination — AI SDK sees the failure)
controller.error(wrapError(event.error ?? ...))

src/stream.ts (188 → 243 lines)

  • wrapError() — normalizes SSE error objects (which are plain objects, not Error instances) into real Error objects with proper stack traces
  • SSE error events → controller.error(wrapError(...)) — terminates the stream
  • Network read failures → controller.error(wrapError(...)) — terminates the stream
  • Removed silent {type: "error"} stream part handling entirely

Tests

  • 6 new + 1 updated tests in tests/unit/stream.test.ts
  • 23 stream tests passing, 88 total tests passing

Why This Matters

Combined with PR #10's retry logic, this creates a clean error propagation chain:

SSE error event
  → controller.error() (this PR)
    → streamWithReconnect catches it (PR #10)
      → shouldRetry() checks retryable + emittedContent (PR #10)
        → reconnect or fail with clear error

Without this change, errors are silently swallowed and the retry engine in PR #10 never gets a chance to handle them.

Test Results

88 pass
0 fail
189 expect() calls
Ran 88 tests across 6 files.

Replace naive exponential backoff with production-grade retry engine:

- Error classification: 18 non-retryable patterns (auth/quota/validation)
  and 24 retryable patterns (network/server/timeout) via extractMessage()
  that handles Error, string, and SSE plain-object shapes
- Fixed backoff schedule [1s, 2.5s, 5s] with ±25% jitter replaces
  unbounded exponential 2^n growth
- fetchWithRetry: retries 5xx/429 with backoff, fails fast on 4xx
- fetchOnce: single fetch for mid-stream reconnects (no double counting)
- streamWithReconnect: emittedContent tracking gates reconnect to prevent
  duplicate content generation; pendingReconnect state machine handles
  clean reconnection flow
- shouldRetry: centralized decision — checks aborted, emittedContent,
  attempt count, and error classification
- buildHttpError: parses JSON error bodies with model ID annotation
- partialOutputError: clear error when reconnect is unsafe
- wrapAsError: normalizes any error shape into Error with ccError/code
SSE error events were silently enqueued as stream parts ({type:"error"})
which the AI SDK treats as ignorable — failures went undetected and the
retry layer never saw them.

Changes:
- Add wrapError() to normalize SSE error objects, strings, and Error
  instances into real Error with ccError and code properties
- toStreamPart("error") now returns null (errors handled at parse level)
- parseStreamEvents: SSE error events call controller.error(wrapError(...))
  instead of controller.enqueue({type:"error"})
- Network read failures in catch block also use controller.error(wrapError(...))
- SSE errors in final buffer chunks also terminate via controller.error()

This ensures the retry layer in model.ts sees real Error objects it can
classify (retryable vs non-retryable) and the AI SDK surfaces failures
to users instead of silently swallowing them.
…nment

Tests that expect 'no key found' behavior were failing when
~/.commandcode/auth.json exists in the test environment.

Mock fs.existsSync to return false for auth file paths
(.commandcode, .pi) while preserving real fs for other paths.
Uses mock.module() with dynamic import so the mock is applied
before auth.ts loads.
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