Repository navigation
fix(core): forward retryAfterMs across provider session errors - #57589
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Forwarding works. I ran RequestExecutor.httpFailure → toSessionError on both checkouts. On the base, retryAfterMs is dropped for rate-limit errors. With this PR, retry-after: 45 gives 45000 and retry-after-ms: 1500.5 gives 1501. core's session-error.test.ts and the full ai suite pass, and typecheck is clean in both packages.
One regression to fix before merging: the stricter schema type means a very large Retry-After value now throws inside httpFailure instead of producing a provider error (see inline).
Public API: SessionError.retryAfterMs was already NonNegativeInt, so the server, SDK and OpenAPI shapes don't change. Session errors now include retryAfterMs for provider.rate-limit, provider.quota and provider.internal too, which is additive. In @opencode/ai, retryAfterMs on RateLimitError, QuotaExceededError, ProviderInternalError and HttpRateLimitDetails narrows from any number to a non-negative safe integer. That is enforced when the error is built, so any caller passing a fraction or an unsafe integer will now get an exception.
Related: #33955
|
|
||
| const seconds = Number(value) | ||
| if (Number.isFinite(seconds)) return Math.max(0, seconds * 1000) | ||
| if (Number.isFinite(seconds)) return Math.max(0, Math.ceil(seconds * 1000)) |
There was a problem hiding this comment.
NonNegativeInt is built on Schema.Int, which only accepts safe integers, and these error classes check their fields when constructed. So a large header value now makes httpFailure throw instead of returning a provider error. For example, retry-after: 1e13 (1e16 ms, above 2**53) or a huge retry-after-ms value causes new RateLimitError(...) to throw Schema validation failed. On the base this was a normal provider.rate-limit error. Could retryAfterMs cap the result here, e.g. Math.min(Number.MAX_SAFE_INTEGER, …) or a sane upper bound, so a buggy or hostile gateway header can't turn a 429 into an exception?
- Tool output: upstream's new `save` (bytes a tool cannot attach inline) sits beside the fork's `truncateError`. The fork's private text saver is now `saveText`: its error path must catch a failed save, which upstream's `save` turns into a defect. - Model requests: the `model.request` hook still sees `dryRun`; `baseURL` is passed as is, since upstream typed it as a string. - Tests: the skill-instructions expectation for a changed description is upstream's new delta text, with the fork's list-of-rulesets `load`; the MCP test keeps both sides' imports. - The OpenAPI documents (protocol and www) are regenerated: upstream's commit for `retryAfterMs` on session errors (anomalyco#57589) left them without it.
Problem
SessionError.ErrordefinesretryAfterMs?: NonNegativeInt, and@opencode/aialready parsesretryAfterMsonRateLimitError,QuotaExceededError, andProviderInternalError. However,toSessionErroronly copiedretryAfterMsinside theFreeUsageLimitErrorbranch, dropping it onprovider.rate-limit,provider.quota, andprovider.internalerrors and re-checkingNumber.isFinite/>= 0/Math.ceildownstream instead of typingretryAfterMsasNonNegativeIntat the schema and parser boundary.Change
retryAfterMsasNonNegativeIntonHttpRateLimitDetails,RateLimitError,QuotaExceededError, andProviderInternalError, and round up inRequestExecutor.retryAfterMsat the HTTP header parsing boundary.retryAfterMsdirectly inproviderError()so every provider error reason that carriesretryAfterMspopulatesSessionError.Error.retryAfterMs.Added by the opencode bot (AI-generated).
Related: #33955