Skip to content

Commit 05d12b2

Browse files
feat: add retry with exponential backoff for network interactions
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 #518 Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
1 parent 2a55855 commit 05d12b2

2 files changed

Lines changed: 194 additions & 2 deletions

File tree

internal/httputil/client.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@ import (
2020
"github.com/notaryproject/notation/v2/internal/trace"
2121
"github.com/notaryproject/notation/v2/internal/version"
2222
"oras.land/oras-go/v2/registry/remote/auth"
23+
"oras.land/oras-go/v2/registry/remote/retry"
2324
)
2425

2526
var userAgent = "notation/" + version.GetVersion()
2627

27-
// NewAuthClient returns an *auth.Client with debug log and user agent set
28+
// NewAuthClient returns an *auth.Client with retry, debug log and user agent
29+
// set.
2830
func NewAuthClient(ctx context.Context, httpClient *http.Client) *auth.Client {
31+
httpClient = withRetry(httpClient)
2932
httpClient = trace.SetHTTPDebugLog(ctx, httpClient)
3033
client := &auth.Client{
3134
Client: httpClient,
@@ -36,12 +39,27 @@ func NewAuthClient(ctx context.Context, httpClient *http.Client) *auth.Client {
3639
return client
3740
}
3841

39-
// NewClient returns an *http.Client with debug log and user agent set
42+
// NewClient returns an *http.Client with retry, debug log and user agent set.
4043
func NewClient(ctx context.Context, client *http.Client) *http.Client {
44+
client = withRetry(client)
4145
client = trace.SetHTTPDebugLog(ctx, client)
4246
return SetUserAgent(client)
4347
}
4448

49+
// withRetry wraps the client's transport with retry logic using exponential
50+
// backoff. It retries on 5xx errors, 429, 408, and network timeouts.
51+
func withRetry(client *http.Client) *http.Client {
52+
if client == nil {
53+
client = &http.Client{}
54+
}
55+
base := client.Transport
56+
if base == nil {
57+
base = http.DefaultTransport
58+
}
59+
client.Transport = retry.NewTransport(base)
60+
return client
61+
}
62+
4563
type userAgentTransport struct {
4664
base http.RoundTripper
4765
}

internal/httputil/client_test.go

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
// Copyright The Notary Project Authors.
2+
// Licensed under the Apache License, Version 2.0 (the "License");
3+
// you may not use this file except in compliance with the License.
4+
// You may obtain a copy of the License at
5+
//
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
//
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
14+
package httputil
15+
16+
import (
17+
"context"
18+
"net/http"
19+
"net/http/httptest"
20+
"sync/atomic"
21+
"testing"
22+
23+
"oras.land/oras-go/v2/registry/remote/retry"
24+
)
25+
26+
func TestNewAuthClient(t *testing.T) {
27+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
28+
w.WriteHeader(http.StatusOK)
29+
}))
30+
defer ts.Close()
31+
32+
t.Run("nil client", func(t *testing.T) {
33+
client := NewAuthClient(context.Background(), nil)
34+
if client == nil {
35+
t.Fatal("expected non-nil auth client")
36+
}
37+
if client.Cache == nil {
38+
t.Fatal("expected non-nil cache")
39+
}
40+
})
41+
42+
t.Run("custom client", func(t *testing.T) {
43+
httpClient := &http.Client{}
44+
client := NewAuthClient(context.Background(), httpClient)
45+
if client == nil {
46+
t.Fatal("expected non-nil auth client")
47+
}
48+
})
49+
}
50+
51+
func TestNewClient(t *testing.T) {
52+
t.Run("nil client", func(t *testing.T) {
53+
client := NewClient(context.Background(), nil)
54+
if client == nil {
55+
t.Fatal("expected non-nil client")
56+
}
57+
})
58+
59+
t.Run("custom client", func(t *testing.T) {
60+
httpClient := &http.Client{}
61+
client := NewClient(context.Background(), httpClient)
62+
if client == nil {
63+
t.Fatal("expected non-nil client")
64+
}
65+
})
66+
}
67+
68+
func TestWithRetry(t *testing.T) {
69+
t.Run("nil client gets default transport", func(t *testing.T) {
70+
client := withRetry(nil)
71+
if client == nil {
72+
t.Fatal("expected non-nil client")
73+
}
74+
rt, ok := client.Transport.(*retry.Transport)
75+
if !ok {
76+
t.Fatalf("expected *retry.Transport, got %T", client.Transport)
77+
}
78+
if rt.Base != http.DefaultTransport {
79+
t.Fatal("expected base to be http.DefaultTransport")
80+
}
81+
})
82+
83+
t.Run("client with nil transport gets default transport", func(t *testing.T) {
84+
client := withRetry(&http.Client{})
85+
rt, ok := client.Transport.(*retry.Transport)
86+
if !ok {
87+
t.Fatalf("expected *retry.Transport, got %T", client.Transport)
88+
}
89+
if rt.Base != http.DefaultTransport {
90+
t.Fatal("expected base to be http.DefaultTransport")
91+
}
92+
})
93+
94+
t.Run("custom transport is preserved", func(t *testing.T) {
95+
custom := &http.Transport{}
96+
client := withRetry(&http.Client{Transport: custom})
97+
rt, ok := client.Transport.(*retry.Transport)
98+
if !ok {
99+
t.Fatalf("expected *retry.Transport, got %T", client.Transport)
100+
}
101+
if rt.Base != custom {
102+
t.Fatal("expected base to be the custom transport")
103+
}
104+
})
105+
}
106+
107+
func TestRetryOnServerError(t *testing.T) {
108+
var attempts atomic.Int32
109+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
110+
if attempts.Add(1) == 1 {
111+
w.WriteHeader(http.StatusServiceUnavailable)
112+
return
113+
}
114+
w.WriteHeader(http.StatusOK)
115+
}))
116+
defer ts.Close()
117+
118+
client := NewClient(context.Background(), nil)
119+
resp, err := client.Get(ts.URL)
120+
if err != nil {
121+
t.Fatalf("unexpected error: %v", err)
122+
}
123+
defer resp.Body.Close()
124+
if resp.StatusCode != http.StatusOK {
125+
t.Fatalf("expected status 200, got %d", resp.StatusCode)
126+
}
127+
if got := attempts.Load(); got != 2 {
128+
t.Fatalf("expected 2 attempts, got %d", got)
129+
}
130+
}
131+
132+
func TestNoRetryOnClientError(t *testing.T) {
133+
var attempts atomic.Int32
134+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
135+
attempts.Add(1)
136+
w.WriteHeader(http.StatusBadRequest)
137+
}))
138+
defer ts.Close()
139+
140+
client := NewClient(context.Background(), nil)
141+
resp, err := client.Get(ts.URL)
142+
if err != nil {
143+
t.Fatalf("unexpected error: %v", err)
144+
}
145+
defer resp.Body.Close()
146+
if resp.StatusCode != http.StatusBadRequest {
147+
t.Fatalf("expected status 400, got %d", resp.StatusCode)
148+
}
149+
if got := attempts.Load(); got != 1 {
150+
t.Fatalf("expected 1 attempt (no retry), got %d", got)
151+
}
152+
}
153+
154+
func TestSetUserAgent(t *testing.T) {
155+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
156+
ua := r.Header.Get("User-Agent")
157+
if ua != userAgent {
158+
w.WriteHeader(http.StatusBadRequest)
159+
return
160+
}
161+
w.WriteHeader(http.StatusOK)
162+
}))
163+
defer ts.Close()
164+
165+
client := NewClient(context.Background(), nil)
166+
resp, err := client.Get(ts.URL)
167+
if err != nil {
168+
t.Fatalf("unexpected error: %v", err)
169+
}
170+
defer resp.Body.Close()
171+
if resp.StatusCode != http.StatusOK {
172+
t.Fatalf("expected status 200, got %d", resp.StatusCode)
173+
}
174+
}

0 commit comments

Comments
 (0)