Skip to content

feat: production-grade retry/backoff with error classification and partial output protection - #10

Open
danielxxomg wants to merge 4 commits into
brent-weatherall:mainfrom
danielxxomg:feat/retry-backoff-core
Open

feat: production-grade retry/backoff with error classification and partial output protection#10
danielxxomg wants to merge 4 commits into
brent-weatherall:mainfrom
danielxxomg:feat/retry-backoff-core

Conversation

@danielxxomg

Copy link
Copy Markdown

Summary

Replaces the naive retry logic in src/model.ts with a production-grade retry engine that handles transient network errors, server failures, and mid-stream disconnects gracefully.

Problems Solved

  1. Thundering herd: Original uses exponential backoff (2s * 2^n) without jitter — all clients retry at the same time
  2. No error classification: Retries on auth/quota errors (insufficient credit, unauthorized) that will never succeed
  3. Duplicate content on reconnect: Mid-stream reconnects can emit text/tool-calls twice if content was already sent downstream
  4. No HTTP status retry: 429 (rate limit) and 5xx (server error) are not retried

Changes

src/model.ts (327 → 532 lines)

  • extractMessage() — handles Error, string, and plain SSE error objects for classification
  • NON_RETRYABLE_PATTERNS (18 patterns) — auth/quota/validation errors, never retry
  • RETRYABLE_PATTERNS (24 patterns) — network/server/transient errors
  • isRetryableStatus() — retries HTTP 429 and 5xx
  • backoffDelay() — fixed schedule [1s, 2.5s, 5s] with ±25% jitter (replaces exponential 2s * 2^n)
  • fetchOnce() — single fetch for mid-stream reconnects (no double retry counting)
  • emittedContent tracking — gates reconnect: if text/reasoning/tool-call already emitted, aborts reconnect to prevent DUPLICATE content
  • shouldRetry() — centralized decision: checks aborted, emittedContent, attempt count, and retryable classification
  • partialOutputError() — clear error message when reconnect would cause duplicates
  • wrapAsError() — normalizes any error shape into a real Error object
  • buildHttpError() — parses JSON error bodies properly

Tests

  • 9 new + 2 updated tests in tests/unit/model.test.ts
  • New mockFetchRetrySequence() helper in tests/helpers/mocks.ts
  • 22 model tests passing, 88 total tests passing

Backoff Schedule Comparison

Attempt Original (exponential) New (fixed + jitter)
1 2s ~1s (0.75–1.25s)
2 4s ~2.5s (1.9–3.1s)
3 8s ~5s (3.75–6.25s)
4 16s ~5s (3.75–6.25s)

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.
fix(stream): terminate stream on SSE errors via controller.error()
@danielxxomg

Copy link
Copy Markdown
Author

Hi @brent-weatherall! Friendly check-in on this PR and repository maintenance status.

Given that upstream has been inactive for a few months and multiple community members are relying on Command Code provider improvements, we've integrated this production-grade retry/backoff engine (along with SSE error propagation and the full 50-model Command Code catalog) into an actively maintained fork: danielxxomg/opencode-commandcode-provider (v0.4.1).

If you'd like to resume maintenance and merge these upstream, we'd be delighted to collaborate. In the meantime, users and contributors can leverage the fork.

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