fix(http): avoid retrying keyless writes (#103)#256
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Walkthrough
ChangesTransport retry policy
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 timed out. The project may have too many dependencies for the sandbox. 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 |
zeshi-du
left a comment
There was a problem hiding this comment.
Correct and minimal: transport-level retry now requires GET/HEAD or an Idempotency-Key header (case-insensitive), gated at both retry sites, with the 429/409 semantic retry path untouched. All standard write commands already attach idempotency keys, so the practical effect is scoped to keyless writes like cancelRun — which loses silent transport retry as an accepted behavior change (we'll consider giving cancel its own idempotency key as a follow-up). Thanks @naufalfx805-source!
What does this PR do?
Closes #103.
This gates transport-level retries so non-idempotent writes without an
Idempotency-Keyare not blindly replayed after a network error or bare edge 408/502/504 response.Changes:
GET/HEADrequests.Idempotency-Keyheader is present, case-insensitively.Verification
npm test -- src/lib/http.test.ts-> 45 passednpm run lintnpm run typechecknpm run buildgit diff --checkSummary by CodeRabbit