refactor(opencode): make classifyRetry read providerFailure.kind - #1113
Conversation
Slice ④ of #1105. Make the retry consumer read the canonical providerFailure.kind (landed in slice ②) instead of re-deriving the retry/stop decision from the provider SDK's isRetryable flag. - classifyRetry's APIError gate now 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: a terminal kind never retries and a transient kind always does, regardless of what the SDK reported. Out of scope (option A, chosen with the maintainer): retry-notice copy is unchanged — the provider's descriptive message is still shown. 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; the non-APIError plain-text fallbacks are kept for errors that carry no providerFailure. Tests: bun test src/session/retry.test.ts test/session/retry.test.ts test/session/message-v2.test.ts test/session/retry-decision.test.ts test/session/processor-rate-limit.test.ts; bun run typecheck. Part of #1105.
Astro-Han
added
P2
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
More reviews will be available in 28 minutes and 7 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)
✨ 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]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Astro-Han
added
the
task
1 participant
What
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'sisRetryableflag.classifyRetry'sAPIErrorgate now keys offproviderFailure.kind:auth,invalid_request,quota_exhausted) never retryrate_limit,server_overload,transport_disconnect,decompression) always retryunknownkinds and rows that predateproviderFailurefall back to the legacyisRetryable+ 5xx signal. That signal agrees with the kind classification for every classified case today, so behavior is unchanged for real inputs.isRetryableflag: a terminal kind never retries and a transient kind always does, regardless of what the SDK reported.Why
#1105unifies provider-failure classification behind one serializable discriminant read by every consumer. Slice ② populatedproviderFailure; this slice makes the retry path consume it, collapsing the second (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)
kind. Retry notices are transient and the raw provider message carries the most actionable info (request IDs, specific guidance).free_quota_exhaustedstays a retry-time concept (depends on retry-after headers + wall-clock reset) and is still detected from the opencodeFreeUsageLimitErrormarker.APIErrorplain-text fallbacks are kept for errors that carry noproviderFailure.Verification
bun test src/session/retry.test.ts— 23 pass (7 new: terminal kinds never retry even withisRetryable: true; transient kinds retry even withisRetryable: false/non-5xx;unknown+ absent fall back to legacy;free_quota_exhaustedstill wins; retry-notice copy unchanged).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 (existing behavior unchanged).bun run typecheck(tsgo --noEmit) — clean.Part of #1105.