Skip to content

Commit 89f7c13

Browse files
Refactor and export range retry transport (#2012)
This transport had some weirdness about it in terms of using an HTTP client internally instead of the "usual" transport layering. This fixes that and also avoids storing a context unnecessarily. That makes the transport itself stateless and statically usable for clients as well. Fixes a couple of other nits around the transport as well.
1 parent 968521f commit 89f7c13

File tree

4 files changed

+20
-31
lines changed

4 files changed

+20
-31
lines changed

pkg/apk/apk/implementation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1384,7 +1384,7 @@ func (a *APK) FetchPackage(ctx context.Context, pkg FetchablePackage) (io.ReadCl
13841384
}
13851385

13861386
// This will return a body that retries requests using Range requests if Read() hits an error.
1387-
rrt := newRangeRetryTransport(ctx, client)
1387+
rrt := NewRangeRetryTransport(client.Transport)
13881388
res, err := rrt.RoundTrip(req)
13891389
if err != nil {
13901390
return nil, fmt.Errorf("unable to get package apk at %s: %w", u, err)

pkg/apk/apk/index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ func fetchRepositoryIndex(ctx context.Context, u string, etag string, opts *inde
321321
}
322322

323323
// This will return a body that retries requests using Range requests if Read() hits an error.
324-
rrt := newRangeRetryTransport(ctx, client)
324+
rrt := NewRangeRetryTransport(client.Transport)
325325
res, err := rrt.RoundTrip(req)
326326
if err != nil {
327327
return nil, err

pkg/apk/apk/transport.go

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,45 +15,40 @@
1515
package apk
1616

1717
import (
18-
"context"
1918
"errors"
2019
"fmt"
2120
"io"
2221
"net/http"
2322
)
2423

2524
type rangeRetryTransport struct {
26-
client *http.Client
27-
ctx context.Context
25+
base http.RoundTripper
2826
}
2927

30-
func newRangeRetryTransport(ctx context.Context, client *http.Client) *rangeRetryTransport {
31-
return &rangeRetryTransport{
32-
client: client,
33-
ctx: ctx,
28+
// NewRangeRetryTransport returns a transport that retries failed reads using HTTP Range requests.
29+
func NewRangeRetryTransport(base http.RoundTripper) http.RoundTripper {
30+
if base == nil {
31+
base = http.DefaultTransport
3432
}
33+
return &rangeRetryTransport{base: base}
3534
}
3635

3736
func (t *rangeRetryTransport) RoundTrip(req *http.Request) (*http.Response, error) {
38-
r := rangeRetryReader{
39-
client: t.client,
40-
ctx: t.ctx,
41-
req: req,
37+
r := &rangeRetryReader{
38+
base: t.base,
39+
req: req,
4240
}
4341

4442
return r.reset(nil)
4543
}
4644

4745
type rangeRetryReader struct {
48-
client *http.Client
49-
ctx context.Context
50-
51-
req *http.Request
46+
base http.RoundTripper
47+
req *http.Request
5248

5349
body io.ReadCloser
5450

5551
progress int64
56-
total int64
5752
}
5853

5954
func (r *rangeRetryReader) reset(oerr error) (*http.Response, error) {
@@ -62,14 +57,13 @@ func (r *rangeRetryReader) reset(oerr error) (*http.Response, error) {
6257
_ = r.body.Close()
6358
}
6459

65-
req := r.req.WithContext(r.ctx)
60+
req := r.req.WithContext(r.req.Context())
6661

67-
rangeHeader := fmt.Sprintf("bytes=%d-", r.progress)
6862
if r.progress != 0 {
69-
req.Header.Set("Range", rangeHeader)
63+
req.Header.Set("Range", fmt.Sprintf("bytes=%d-", r.progress))
7064
}
7165

72-
resp, err := r.client.Do(req)
66+
resp, err := r.base.RoundTrip(req)
7367
if err != nil {
7468
return resp, errors.Join(oerr, err)
7569
}
@@ -78,10 +72,6 @@ func (r *rangeRetryReader) reset(oerr error) (*http.Response, error) {
7872
return resp, nil
7973
}
8074

81-
if r.total == 0 {
82-
r.total = resp.ContentLength
83-
}
84-
8575
if resp.StatusCode == http.StatusOK {
8676
// If the upstream doesn't support Range requests for some reason and only returns 200,
8777
// we need to discard anything we've already Read().
@@ -91,11 +81,11 @@ func (r *rangeRetryReader) reset(oerr error) (*http.Response, error) {
9181
}
9282
}
9383
} else if resp.StatusCode != http.StatusPartialContent {
94-
if oerr != nil {
95-
return resp, fmt.Errorf("retrying %w: %s %s (Range: %s): unexpected status code: %d", oerr, req.Method, req.URL.String(), rangeHeader, resp.StatusCode)
84+
if r.progress != 0 {
85+
return resp, fmt.Errorf("retrying %w: %s %s (Range: %s): unexpected status code: %d", oerr, req.Method, req.URL.String(), req.Header.Get("Range"), resp.StatusCode)
9686
}
9787

98-
return resp, fmt.Errorf("%s %s (Range: %s): unexpected status code: %d", req.Method, req.URL.String(), rangeHeader, resp.StatusCode)
88+
return resp, fmt.Errorf("%s %s: unexpected status code: %d", req.Method, req.URL.String(), resp.StatusCode)
9989
}
10090

10191
r.body = resp.Body

pkg/apk/apk/transport_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ package apk
1616

1717
import (
1818
"bytes"
19-
"context"
2019
"fmt"
2120
"io"
2221
"net/http"
@@ -166,7 +165,7 @@ func TestTransport(t *testing.T) {
166165
ranges: tc.ranges,
167166
}
168167

169-
rt := newRangeRetryTransport(context.Background(), &http.Client{Transport: tt})
168+
rt := NewRangeRetryTransport(tt)
170169

171170
req := &http.Request{
172171
URL: &url.URL{},

0 commit comments

Comments
 (0)