feat(opencode): add serializable providerFailure classification vocabulary - #1108
Conversation
…ulary
Introduce one canonical, serializable provider-failure discriminant so retry,
UI, and observability can read a single field instead of re-sniffing error
strings.
- Add ProviderFailureKind zod enum (auth, rate_limit, quota_exhausted,
server_overload, invalid_request, transport_disconnect, decompression,
unknown). free_quota_exhausted stays a retry-time concept and is
intentionally excluded; context overflow keeps ContextOverflowError.
- Classify the kind once at parse time: parseStreamError carries kind+code for
the codes it already handles; parseAPICallError derives kind from status code
and body error code via apiCallErrorKind.
- Carry providerFailure { kind, code } on APIError.data and populate it in the
transport_disconnect, decompression, APICallError, and stream-error branches
of fromError.
- Schema field is optional for back-compat with rows persisted before it
existed; consumers fall back to message sniffing when absent.
No consumer reads providerFailure yet — classifyRetry and other decision
functions keep their existing behavior; that consolidation lands in a later
slice. This slice only establishes and populates the vocabulary.
Part of #1105.
Tests: bun test test/session/message-v2.test.ts test/session/retry.test.ts;
bun run typecheck.
|
Warning Review limit reached
More reviews will be available in 13 minutes and 52 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a unified provider failure classification system that maps diverse error signals—HTTP status codes, provider-specific error codes, transport disconnects, and decompression failures—to a canonical ChangesProvider Failure Classification
Sequence DiagramsequenceDiagram
participant Transport
participant Decompression
participant APICallError
participant StreamParse
participant FromError
participant Metadata["APIError.metadata"]
Transport->>FromError: transport.code
FromError->>Metadata: providerFailure.kind = transport_disconnect
Decompression->>FromError: FetchDecompressionError.code
FromError->>Metadata: providerFailure.kind = decompression
APICallError->>FromError: parsed.kind + parsed.code
FromError->>Metadata: providerFailure = {kind, code}
StreamParse->>FromError: parsed.kind + parsed.code
FromError->>Metadata: providerFailure = {kind, code}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
harness
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Astro-Han
added
the
tech-debt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a canonical, serializable classification for provider and API failures (ProviderFailureKind) using Zod, updating error parsing utilities and the APIError schema to carry this classification for improved observability and retry handling. Feedback suggests mapping HTTP status codes 400 and 422 to the invalid_request failure kind to improve classification accuracy, along with adding corresponding test cases to verify this behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/test/session/message-v2.test.ts (1)
1700-1724: ⚡ Quick winConsider adding test coverage for
server_is_overloadedcode.The PR objectives mention "server_is_overloaded / server_error → server_overload", but only
server_erroris explicitly tested. Adding a test case for theserver_is_overloadedcode would provide explicit coverage of this mapping, which is part of the stated PR scope.You could add it to the parameterized test at lines 1659-1698 or create a similar dedicated test case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/test/session/message-v2.test.ts` around lines 1700 - 1724, Add a test that mirrors the existing "serializes OpenAI response server_error stream chunks as retryable APIError" case but uses an error body with error.code === "server_is_overloaded" (and type "error"/"server_error" as appropriate) and assert that MessageV2.fromError(...) produces providerFailure: { kind: "server_overload", code: "server_is_overloaded" } (and the same isRetryable/responseBody/providerID/message expectations); place it alongside the existing test (or add to the parameterized block) and use the same callsite, MessageV2.fromError, and expectation shape so the mapping from "server_is_overloaded" → "server_overload" is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/opencode/test/session/message-v2.test.ts`:
- Around line 1700-1724: Add a test that mirrors the existing "serializes OpenAI
response server_error stream chunks as retryable APIError" case but uses an
error body with error.code === "server_is_overloaded" (and type
"error"/"server_error" as appropriate) and assert that MessageV2.fromError(...)
produces providerFailure: { kind: "server_overload", code:
"server_is_overloaded" } (and the same
isRetryable/responseBody/providerID/message expectations); place it alongside
the existing test (or add to the parameterized block) and use the same callsite,
MessageV2.fromError, and expectation shape so the mapping from
"server_is_overloaded" → "server_overload" is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1cbca373-36b9-42b2-90c5-41b3665c4876
📒 Files selected for processing (3)
packages/opencode/src/provider/error.tspackages/opencode/src/session/message-v2.tspackages/opencode/test/session/message-v2.test.ts
400 (Bad Request) and 422 (Unprocessable Entity) are client-side request rejections. Overflow 4xx is already routed to context_overflow before apiCallErrorKind runs, so what reaches here is a genuine invalid request rather than an over-long prompt. Map them to invalid_request instead of unknown to improve classification coverage for errors that carry no specific error code. Addresses review feedback on #1108.
Slice ④ of #1105. Make the retry consumer read the canonical providerFailure.kind (landed in slice ② via #1108) instead of re-deriving the retry/stop decision from the provider SDK's isRetryable flag. What - classifyRetry's APIError gate keys off providerFailure.kind: terminal kinds (auth, invalid_request, quota_exhausted) never retry; transient kinds (rate_limit, server_overload, transport_disconnect, decompression) always do. - `unknown` kinds and rows that predate providerFailure fall back to the legacy isRetryable + 5xx signal, which agrees with the kind classification for every classified case today, so behavior is unchanged for real inputs. - Reading the kind makes the decision robust against a mis-set isRetryable flag. Why - #1105 unifies provider-failure classification behind one serializable discriminant read by every consumer. Slice ② populated providerFailure; this slice makes the retry path consume it, collapsing the retry-time string-sniffing classification onto the parse-time one and removing the drift risk between them. Scope boundary (option A, chosen with the maintainer) - Retry-notice copy is unchanged: the provider's descriptive message is still shown during retries. Standardized per-kind copy and actionable affordances belong to the design-gated UI slice ⑥ where the UI reads kind. - free_quota_exhausted stays a retry-time concept and is still detected from the opencode FreeUsageLimitError marker; non-APIError plain-text fallbacks are kept for errors that carry no providerFailure. Verification - bun test src/session/retry.test.ts (23 pass, 7 new). - bun test test/session/retry.test.ts test/session/message-v2.test.ts test/session/retry-decision.test.ts test/session/processor-rate-limit.test.ts (all pass, behavior unchanged). - bun run typecheck (tsgo --noEmit) clean. - codex review: no blocking findings. Part of #1105.
1 participant
What
Slice ② of #1105. Introduce one canonical, serializable provider-failure discriminant (
providerFailure) so retry, UI, and observability can read a single field instead of re-sniffing error strings.ProviderFailureKindzod enum:auth,rate_limit,quota_exhausted,server_overload,invalid_request,transport_disconnect,decompression,unknown.free_quota_exhaustedis intentionally excluded — it is a retry-time concept that depends on retry-after headers and wall-clockresetAt, not a parse-time property. Context overflow keeps its ownContextOverflowErrorname.parseStreamErrorcarrieskind+codefor the codes it already handles (insufficient_quota/usage_not_included→quota_exhausted,invalid_prompt→invalid_request,server_is_overloaded/server_error→server_overload). Unknown codes still returnundefined(unchanged).parseAPICallErrorderives the kind from status code + body error code viaapiCallErrorKind.providerFailure { kind, code }onAPIError.dataand populate it in thetransport_disconnect,decompression,APICallError, and stream-error branches offromError.Why
#1105tracks unifying provider-failure classification behind one serializable discriminant. Today every consumer (retry, UI, observability) re-sniffsmessage/statusCode/codeindependently, which drifts. This slice establishes and populates the vocabulary so later slices can consolidate the decision logic onto a single field.Scope boundary
No consumer reads
providerFailureyet.classifyRetryand other decision functions keep their existing behavior verbatim (the retry-notice copy is user-facing and stays unchanged). Consolidating consumers onto the new field lands in a later slice. This PR is purely additive vocabulary + population + persistence schema.Verification
bun test test/session/message-v2.test.ts— 63 pass (6 new: transport/decompression/status-code population, schema round-trip + back-compat + unknown-kind rejection).bun test test/session/retry.test.ts— 35 pass (unchanged behavior confirmed).bun run typecheck(tsgo --noEmit) — clean.Part of #1105.
Summary by CodeRabbit
Improvements
Tests