fix(design): honor Retry-After header in variants 429 handler#1337
Closed
stedfn wants to merge 1 commit intogarrytan:mainfrom
Closed
fix(design): honor Retry-After header in variants 429 handler#1337stedfn wants to merge 1 commit intogarrytan:mainfrom
stedfn wants to merge 1 commit intogarrytan:mainfrom
Conversation
Closes garrytan#1244. The 429 handler in `generateVariant` discarded the `Retry-After` response header and fell straight through to a local exponential schedule (2s/4s/8s). In image-generation batches, that burns retry attempts inside the provider's cooldown window and the request never recovers. Now we parse `Retry-After` per RFC 7231 — both delta-seconds (`Retry-After: 5`) and HTTP-date (`Retry-After: Fri, 31 Dec 1999 23:59:59 GMT`). Honored waits are capped at 60s to bound stalls from hostile or buggy headers. Delta-seconds are validated as digits-only (rejects `2abc`). When `Retry-After` is honored (including 0 / past-date "retry now"), the next iteration's leading exponential sleep is skipped so we don't double-wait. Invalid or missing headers fall through to the existing exponential schedule unchanged. Behavior matrix: | Header | Behavior | |---------------------------------|-------------------------------------------| | Retry-After: 5 | wait 5s, skip leading on next attempt | | Retry-After: 999999 | capped to 60s, skip leading | | Retry-After: 2abc | invalid, fall through to exponential | | Retry-After: 0 | wait 0, skip leading (retry immediately) | | Retry-After: <past HTTP-date> | wait 0, skip leading | | Retry-After: <future date> | wait diff capped at 60s, skip leading | | no header | fall through to existing exponential | `generateVariant` now accepts an optional `fetchFn` parameter (defaults to `globalThis.fetch`) so tests can inject a stub. Production call sites are unchanged. Tests cover the five behavior buckets above, asserting both the 1st-to-2nd call timing gap and call counts. All five pass in ~8s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
7 tasks
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1244.
Problem
design/src/variants.ts's 429 handler ingenerateVariantdiscards theRetry-Afterresponse header and falls through to a local exponential schedule (2s/4s/8s). On image-generation batches, that can burn all retry attempts inside the provider's cooldown window and the request never recovers.Fix
Parse
Retry-Afterper RFC 7231 — both delta-seconds and HTTP-date. Honored waits capped at 60s to bound stalls from hostile or buggy headers. Delta-seconds validated as digits-only (rejects2abc). When honored (including0and past dates — "retry now"), skip the next iteration's leading exponential sleep so we don't double-wait. Invalid or missing headers fall through to the existing exponential schedule unchanged.Behavior matrix
Retry-After: 5Retry-After: 999999Retry-After: 2abcRetry-After: 0Retry-After: <past HTTP-date>Retry-After: <future date>Test plan
design/test/variants-retry-after.test.ts— 5 cases covering delta-seconds, HTTP-date, invalid, no-header, andRetry-After: 0. Each asserts both the call count and the 1st-to-2nd call timing gap so the fall-through and the no-double-wait properties are both verified.generateVariantnow accepts an optionalfetchFnparameter (defaultglobalThis.fetch) so the tests can inject a stub. Production call sites unchanged.bun test design/test/variants-retry-after.test.ts— all 5 pass in ~8s locally.Notes
Math.min(..., MAX_RETRY_AFTER_MS)); not asserted at runtime to keep the test under 10s.Date.now()is ms).