Repository navigation
fix(core): honor Retry-After hints and add a no-hint retry floor - #1233
Conversation
|
@morgan-coded is attempting to deploy a commit to the Sachin Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughRetry timing is centralized through ChangesRetry-After provider flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant createPrediction
participant withProviderRetry
participant ReplicateAPI
participant sleep
Client->>createPrediction: create prediction
createPrediction->>withProviderRetry: execute submission
withProviderRetry->>ReplicateAPI: POST prediction
ReplicateAPI-->>withProviderRetry: 429 with Retry-After
withProviderRetry->>sleep: wait parsed delay
withProviderRetry->>ReplicateAPI: retry submission
ReplicateAPI-->>createPrediction: prediction result
createPrediction-->>Client: return prediction
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/retryAfter.test.ts (1)
39-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo coverage for
httpRateLimiter.ts's rate-limit header handling or theX-RateLimit-*fallback paths.This suite covers
Retry-Afternumeric/date/absent/invalid cases and the Replicate/provider-retry paths well, butparseRetryAfterMs'sX-RateLimit-Reset/X-RateLimit-Remainingbranches andhttpRateLimiter.ts#handleRateLimitResponse's refactor (a complexity-high change per the PR) have no direct test coverage in this new suite.🤖 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 `@test/retryAfter.test.ts` around lines 39 - 64, The retry-after tests do not cover the X-RateLimit fallback logic or the refactored handleRateLimitResponse flow. Extend the Retry-After parsing suite to verify X-RateLimit-Reset and X-RateLimit-Remaining behavior, including relevant missing or invalid headers, and add direct tests for httpRateLimiter.ts#handleRateLimitResponse covering its rate-limit response handling branches.
🤖 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.
Inline comments:
In `@src/lib/utils/retryAfter.ts`:
- Around line 30-34: Update the numeric Retry-After handling in retryAfter so
the seconds-to-milliseconds result is clamped to zero with the same behavior
used by the HTTP-date and X-RateLimit-Reset branches. Preserve the existing
parsing and fallback flow, including the Number.isNaN check.
---
Nitpick comments:
In `@test/retryAfter.test.ts`:
- Around line 39-64: The retry-after tests do not cover the X-RateLimit fallback
logic or the refactored handleRateLimitResponse flow. Extend the Retry-After
parsing suite to verify X-RateLimit-Reset and X-RateLimit-Remaining behavior,
including relevant missing or invalid headers, and add direct tests for
httpRateLimiter.ts#handleRateLimitResponse covering its rate-limit response
handling branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 47ceb20a-9a94-405e-9dec-327645d88d08
📒 Files selected for processing (7)
package.jsonsrc/lib/adapters/replicate/predictionLifecycle.tssrc/lib/mcp/httpRateLimiter.tssrc/lib/utils/errorHandling.tssrc/lib/utils/providerRetry.tssrc/lib/utils/retryAfter.tstest/retryAfter.test.ts
0cbba06 to
69623c4
Compare
|
@morgan-coded can you resolve conflicts? |
|
Resolved the conflict by taking the |
5aa19bf to
e7ec9fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/lib/utils/providerRetry.ts`:
- Around line 146-156: Update the retry delay calculation around getRetryAfterMs
and boundedRetryAfterMs to treat non-finite retryAfterMs values, including NaN
and infinities, as absent hints. Validate finiteness before clamping so the
nullish fallback applies NO_HINT_FLOOR_MS (or the exponential delay) rather than
propagating NaN or an invalid bounded value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e6513b5-15cf-4117-ab40-715f4504ad9d
📒 Files selected for processing (7)
package.jsonsrc/lib/adapters/replicate/predictionLifecycle.tssrc/lib/mcp/httpRateLimiter.tssrc/lib/utils/errorHandling.tssrc/lib/utils/providerRetry.tssrc/lib/utils/retryAfter.tstest/retryAfter.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/lib/utils/errorHandling.ts
- src/lib/utils/retryAfter.ts
- test/retryAfter.test.ts
- package.json
- src/lib/mcp/httpRateLimiter.ts
- src/lib/adapters/replicate/predictionLifecycle.ts
| const retryAfterMs = getRetryAfterMs(error); | ||
| const boundedRetryAfterMs = | ||
| retryAfterMs === undefined | ||
| ? undefined | ||
| : Math.min(MAX_RETRY_AFTER_MS, Math.max(0, retryAfterMs)); | ||
| const delay = | ||
| boundedRetryAfterMs ?? | ||
| Math.max(BASE_RETRY_DELAY_MS * Math.pow(2, attempt), NO_HINT_FLOOR_MS); | ||
|
|
||
| // Record retry event on the OTel span | ||
| span.addEvent("gen_ai.provider.retry", { | ||
| span?.addEvent("gen_ai.provider.retry", { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reject non-finite retry metadata before clamping.
If retryAfterMs is NaN, the clamp also becomes NaN; the default timer then retries immediately instead of applying NO_HINT_FLOOR_MS. Treat non-finite values as absent hints.
Proposed fix
const retryAfterMs = getRetryAfterMs(error);
const boundedRetryAfterMs =
- retryAfterMs === undefined
- ? undefined
+ retryAfterMs !== undefined && Number.isFinite(retryAfterMs)
+ ? Math.min(MAX_RETRY_AFTER_MS, Math.max(0, retryAfterMs))
: undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const retryAfterMs = getRetryAfterMs(error); | |
| const boundedRetryAfterMs = | |
| retryAfterMs === undefined | |
| ? undefined | |
| : Math.min(MAX_RETRY_AFTER_MS, Math.max(0, retryAfterMs)); | |
| const delay = | |
| boundedRetryAfterMs ?? | |
| Math.max(BASE_RETRY_DELAY_MS * Math.pow(2, attempt), NO_HINT_FLOOR_MS); | |
| // Record retry event on the OTel span | |
| span.addEvent("gen_ai.provider.retry", { | |
| span?.addEvent("gen_ai.provider.retry", { | |
| const retryAfterMs = getRetryAfterMs(error); | |
| const boundedRetryAfterMs = | |
| retryAfterMs !== undefined && Number.isFinite(retryAfterMs) | |
| ? Math.min(MAX_RETRY_AFTER_MS, Math.max(0, retryAfterMs)) | |
| : undefined; | |
| const delay = | |
| boundedRetryAfterMs ?? | |
| Math.max(BASE_RETRY_DELAY_MS * Math.pow(2, attempt), NO_HINT_FLOOR_MS); | |
| // Record retry event on the OTel span | |
| span?.addEvent("gen_ai.provider.retry", { |
🤖 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 `@src/lib/utils/providerRetry.ts` around lines 146 - 156, Update the retry
delay calculation around getRetryAfterMs and boundedRetryAfterMs to treat
non-finite retryAfterMs values, including NaN and infinities, as absent hints.
Validate finiteness before clamping so the nullish fallback applies
NO_HINT_FLOOR_MS (or the exponential delay) rather than propagating NaN or an
invalid bounded value.
e7ec9fd to
6cf77e4
Compare
|
🎉 This PR is included in version 10.8.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
withProviderRetrybacked off exponentially at 1s then 2s, rather than the ~1s linear loop described in #1189; AI SDK errors reached it withresponseHeadersintact, while Replicate submissions bypassed it and discarded those headers.This extracts the existing Retry-After parser for numeric seconds, HTTP dates, and rate-limit fallbacks, uses server delays capped at 120s or a 10s no-hint floor in
withProviderRetry, and carries Replicate's header throughNeuroLinkErrorwhile retrying prediction submission, with polling behavior otherwise unchanged.The 10s no-hint floor can delay real model-pool failover while a member retries internally — kept the floor the issue proposes, flagging that tradeoff for your call; the shape follows the existing
ClientRateLimitError.retryAfterandaccountPool.markQuotaExceeded(..., retryAfterMs)handling.I added a focused offline suite with 9 passing tests, then ran format, source lint, validation, builds, CLI help, strict TypeScript, and all 23 unit legs locally; CI runs no test suites, and the only local failures were the five byte-matching baseline failures. Fixes #1189
Summary by CodeRabbit
Retry-Afterand related rate-limit headers, with bounded minimum/maximum delays.429and5xxas retriable and propagating retry timing hints.retryAfterMsfor more precise retry behavior.