feat: add retry with exponential backoff for network interactions#1342
feat: add retry with exponential backoff for network interactions#1342ogulcanaydogan wants to merge 1 commit intonotaryproject:mainfrom
Conversation
ogulcanaydogan
requested review from
NiazFK,
gokarnm,
priteshbandi,
rgnote,
shizhMSFT,
toddysm,
vaninrao10 and
yizha1
as code owners
Wrap HTTP transports with retry logic from oras-go/v2/registry/remote/retry in both NewAuthClient and NewClient. The retry transport uses exponential backoff with jitter (200ms-3s, up to 5 retries) and retries on 5xx errors, 429 Too Many Requests, 408 Request Timeout, and network dial timeouts. - Add withRetry helper to wrap client transports with retry.Transport - Add unit tests for retry behavior, transport wrapping, and user agent Fixes notaryproject#518 Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
ogulcanaydogan
force-pushed
the
feat/518-retry-network-interactions
branch
from
76006e7 to
05d12b2
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds automatic retry with exponential backoff for all HTTP network interactions in notation by wrapping HTTP client transports with oras-go/v2/registry/remote/retry.Transport. Since oras-go/v2 is already a dependency, no new dependencies are introduced. This addresses issue #518.
Changes:
- Added
withRetry()helper ininternal/httputil/client.gothat wraps the HTTP client transport withretry.NewTransport, and integrated it into bothNewAuthClient()andNewClient() - Added comprehensive unit tests in
internal/httputil/client_test.gocovering nil/custom client handling, retry on server errors, no retry on client errors, and user-agent propagation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/httputil/client.go |
Added withRetry() function and integrated it into NewAuthClient() and NewClient(), updated doc comments |
internal/httputil/client_test.go |
New test file with tests for NewAuthClient, NewClient, withRetry, retry behavior on server/client errors, and user-agent header propagation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer ts.Close() |
There was a problem hiding this comment.
The httptest.NewServer created on line 27 is never used—none of the subtests make requests to ts.URL. This is dead code that allocates a goroutine and listener unnecessarily. Remove the unused test server.
Summary
Add automatic retry with exponential backoff for all HTTP network interactions in notation. This wraps HTTP client transports with
oras-go/v2/registry/remote/retry.Transport, which provides:oras.land/oras-go/v2which is already ingo.modChanges
internal/httputil/client.go: AddwithRetry()helper that wraps the HTTP client transport withretry.NewTransport. Applied in bothNewAuthClient()andNewClient().internal/httputil/client_test.go: Add unit tests covering:Transport stack
Test plan
go test ./internal/httputil/ -v— all new tests passgo build ./cmd/notation/— builds successfullygo test ./...— full test suite passesgo vet ./internal/httputil/— no issuesFixes #518