fix(client): retry transient API failures - #34
Conversation
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe client adds bounded retries for idempotent requests, including ChangesHTTP retry engine
Signing-key retry integration
Pagination validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RetryingRequestDoer
participant HTTPServer
Client->>RetryingRequestDoer: Send idempotent GET or HEAD
RetryingRequestDoer->>HTTPServer: Execute request
HTTPServer-->>RetryingRequestDoer: Return transient failure
RetryingRequestDoer->>RetryingRequestDoer: Apply Retry-After or backoff
RetryingRequestDoer->>HTTPServer: Retry request
HTTPServer-->>Client: Return final response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@cmd/nvfleetint/node_test.go`:
- Around line 327-340: Strengthen the merged-output assertions in the test
around the decoded got.Items value: unmarshal each item’s nodeUUID and verify
the returned identities are node-1 followed by node-2. Keep the existing
item-count, pagination, and page-call assertions, but ensure the test rejects
duplicate first-page nodes.
In `@nvfleetint/client.go`:
- Around line 388-389: Update the Retry-After parsing logic around the seconds
conversion to validate that the parsed value does not exceed the maximum
representable time.Duration in seconds before multiplying by time.Second; reject
or clamp oversized values so the resulting duration never wraps negative. Add
boundary coverage for the largest valid value and an overflowing value,
including the resulting waitForRetry behavior.
🪄 Autofix
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: Enterprise
Run ID: fed230a7-3229-4c71-8398-83ef883a54b8
📒 Files selected for processing (5)
cmd/nvfleetint/node_test.gointernal/clihelpers/pagination.gonvfleetint/client.gonvfleetint/client_test.gonvfleetint/verify_test.go
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
Description
Checklist
git commit -s) per the Contributing Guidelines.Summary by CodeRabbit
New Features
Bug Fixes