fix(combos): shorten request-rate cooldowns and send Retry-After on combo 503 - #3294
Conversation
…ombo 503 Request-rate 429s without Retry-After no longer inherit the 60s quota cooldown. Combo unavailable responses advertise the soonest remaining target cooldown.
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughChangesThe PR adds transient request-rate cooldown handling for combo failover. It propagates failure metadata, computes remaining cooldown time, and includes validated Combo cooldown handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to The proxy now shortens selected request-rate cooldowns and reports the soonest combo cooldown through Retry-After on 503 responses. It is mergeable with owner awareness that removed targets or concurrent cooldown updates could produce an inaccurate retry delay, causing either premature retries or unnecessary client waiting. Sequence Diagram(s)sequenceDiagram
participant Request as Request handling
participant Resolve as advanceComboAfterFailure
participant Failover as coolComboTarget
participant Response as comboUnavailableResponse
Request->>Resolve: pass status, code, and message
Resolve->>Failover: forward failure metadata
Failover->>Failover: select 5-second or default cooldown
Request->>Response: build unavailable response
Response->>Failover: query remaining cooldown
Failover-->>Response: return Retry-After seconds
Response-->>Request: return 503 with Retry-After
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
리뷰 · 우선순위 58 / 80이 PR은 콤보(여러 타깃을 묶어서 쓰는 길)에서 잠깐 막히는 요청 속도 제한과, 타깃이 전부 식고 있을 때 클라이언트가 어떻게 기다리게 할지 고칩니다. 지금 지금 라인 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5de7d9227b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| targetCooldowns.delete(key); | ||
| continue; | ||
| } | ||
| if (soonest === undefined || remaining < soonest) soonest = remaining; |
There was a problem hiding this comment.
remainingComboCooldownMs takes the minimum across every stored cooldown for the combo, even though handleComboResponses may exclude targets through payloadEligible and pickComboTarget also excludes disabled providers. For example, if an encrypted worker request can use only a native target cooled for 60 seconds while an ineligible non-native target is cooled for 5 seconds, the resulting 503 advertises Retry-After: 5; retrying then produces another local 503 because the only usable target remains cooled. Pass the current eligibility predicate/configured target set into this calculation so the header reflects the earliest target that could actually serve the request.
Useful? React with 👍 / 👎.
| ?? (isTransientRequestRateLimit({ | ||
| status: options?.status, | ||
| code: options?.code, | ||
| message: options?.message, | ||
| }) ? COMBO_REQUEST_RATE_COOLDOWN_MS : DEFAULT_COOLDOWN_MS); |
There was a problem hiding this comment.
This branch changes request-rate failures from the documented 60-second default to 5 seconds, and the response path now exposes a Retry-After header on cooldown-backed combo_unavailable responses, but docs-site/src/content/docs/guides/combos.md still states that hopped targets use a 60-second default and documents only the 503 body; translated guides repeat that contract. Update the English guide/reference and synchronize the localized pages so users do not rely on obsolete cooldown and response-header behavior.
AGENTS.md reference: AGENTS.md:L343-L344
Useful? React with 👍 / 👎.
2 participants
Summary
When every combo target is in cooldown, the proxy returns HTTP 503
combo_unavailablewith noRetry-After. Clients that retry immediately then spend the cooldown window on local failures instead of waiting for a target to thaw.Request-rate 429s that omit
Retry-Aftercurrently inherit the same 60s target cooldown as usage-window exhaustion. That is longer than the recovery time for a short request-rate limit.This change:
1302,1305) and theRate limit reached for requestsmessage, when the upstream response has noRetry-After.1308and related) and generic 429s on the existing 60s default.Retry-AfterorcooldownMs.Retry-Afteron 503combo_unavailableto the soonest remaining target cooldown.Quota 429s, auth failures, and explicit backoff headers are unchanged. This is not the first-byte stall hop in #3266.
Test plan
bun test tests/combos.test.tsReview readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Retry-Aftervalue when a cooldown remains.Bug Fixes