Repository navigation
fix(proxy): harden Anthropic request and retry handling - #1165
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughClaude proxy routing now preserves Claude Code OAuth request structure, coordinates transient 429 retries and cooldown admissions, classifies construction rejections as terminal invalid requests, gates fallback, and standardizes nested transport-error reporting. ChangesClaude proxy reliability
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RoutedRequestLoop
participant TransientCooldownAdmission
participant AnthropicUpstream
Client->>RoutedRequestLoop: submit Claude-compatible request
RoutedRequestLoop->>TransientCooldownAdmission: check account availability
TransientCooldownAdmission-->>RoutedRequestLoop: paced retry admission
RoutedRequestLoop->>AnthropicUpstream: send normalized request
AnthropicUpstream-->>RoutedRequestLoop: response or classified terminal error
RoutedRequestLoop-->>Client: stream or finalized proxy error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. 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 |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Pull request overview
This PR hardens the Anthropic/Claude proxy’s request transformation and retry/cooldown behavior, focusing on preserving genuine Claude Code OAuth request shapes, correctly classifying “construction” 429s vs real rate limits, and reducing retry amplification under concurrent load.
Changes:
- Preserve genuine Claude Code
systemblock ordering/content for OAuth, while continuing to relocate non-Claude client system instructions into messages. - Introduce shared transient 429 retry budgets and paced admission during short transient cooldowns to avoid concurrent retry herds.
- Add a dedicated terminal error path for construction rejections, improve stream transport error diagnostics, and skip provider fallback for deterministic invalid requests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/proxyReliabilityHardening.test.ts | Adds coverage for OAuth request-shape preservation, 429 classification, shared transient retry budgets, transient cooldown pacing, fallback skipping, and stream transport error descriptions. |
| test/continuous-test-suite-bugfixes.ts | Updates invariant string checks to reflect let effectiveAccounts change. |
| src/lib/types/proxy.ts | Extends proxy types to support shared transient retry budget state and a fetch-layer terminal error handoff. |
| src/lib/server/routes/claudeProxyRoutes.ts | Implements request-shape preservation tweaks, terminal construction rejection finalization, shared transient retry/cooldown pacing, improved stream diagnostics, and invalid-request fallback suppression. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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/proxyReliabilityHardening.test.ts (1)
885-891: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert terminal telemetry behavior instead of source text.
Checking for one function name can still pass when another path emits duplicate terminal telemetry. Drive a failed stream and assert the final logger, tracer, and usage statistics each settle once.
🤖 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/proxyReliabilityHardening.test.ts` around lines 885 - 891, Replace the source-text assertion in the “uses only canonical terminal telemetry for stream failures” test with an end-to-end failed-stream scenario. Assert that the final logger, tracer, and usage-statistics handlers each settle exactly once, covering all terminal telemetry paths rather than checking only for the absence of logStreamError.
🤖 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/server/routes/claudeProxyRoutes.ts`:
- Around line 5081-5105: The post-refresh path in handleAnthropicAuthRetry must
reuse the existing construction_rejection terminal mapping from the anti-abuse
429 branch in src/lib/server/routes/claudeProxyRoutes.ts:5081-5105, returning a
non-retryable 400 with the same error body, headers, logging, tracing, and loop
termination behavior. Add an auth-retry or route-level regression in
test/proxyReliabilityHardening.test.ts:157-230 proving that an anti-abuse 429
after OAuth refresh is finalized as a 400 rather than retried.
---
Nitpick comments:
In `@test/proxyReliabilityHardening.test.ts`:
- Around line 885-891: Replace the source-text assertion in the “uses only
canonical terminal telemetry for stream failures” test with an end-to-end
failed-stream scenario. Assert that the final logger, tracer, and
usage-statistics handlers each settle exactly once, covering all terminal
telemetry paths rather than checking only for the absence of logStreamError.
🪄 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
Run ID: b8456673-40ad-4263-b269-c68ee66a7851
📒 Files selected for processing (4)
src/lib/server/routes/claudeProxyRoutes.tssrc/lib/types/proxy.tstest/continuous-test-suite-bugfixes.tstest/proxyReliabilityHardening.test.ts
11d666b to
ac62eb4
Compare
|
Addressed the CodeRabbit review-body test nit in ac62eb4 as well: the prior source-text assertion was replaced with a behavioral failed-stream test that consumes the stream and verifies exactly one final request log, one tracer error/end, one upstream span end, and one request/error stats increment. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
🎉 This PR is included in version 9.87.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Reviewed the 4 changed files in this already-merged PR. No new blocking issues were identified beyond the points already raised and resolved by prior reviewers.
Files reviewed
src/lib/server/routes/claudeProxyRoutes.tssrc/lib/types/proxy.tstest/continuous-test-suite-bugfixes.tstest/proxyReliabilityHardening.test.ts
Existing review points (already handled)
- Billing-block ordering for synthesized Claude Code identity blocks — addressed by author.
- Post-OAuth-refresh construction-rejection terminal 400 path — addressed by author with matching regression test.
Assessment
- The construction-rejection classification now correctly bypasses rate-limit accounting and returns a non-retryable 400 in both the initial-fetch and auth-retry paths.
- Shared transient retry budgets and paced cooldown admission are bounded and reset-aware.
- Stream transport errors are described once and terminal telemetry settles exactly once.
- Provider fallback is correctly gated on
invalidRequestFailure === null. - Types and test coverage are aligned with the runtime changes.
New issues raised this review: 0 critical, 0 major, 0 minor.
Approving.
Summary
Evidence
A direct A/B replay used the same captured request, account, endpoint, and headers:
A later incident reproduced the same result with a different captured request.
Validation
The live proxy was not modified or restarted during development or verification.
Summary by CodeRabbit