From 002a5ce7135b15897325e5f29e73bef906972dd6 Mon Sep 17 00:00:00 2001 From: Adam Stokes Date: Thu, 8 Oct 2026 13:45:06 -0400 Subject: [PATCH 1/2] source/http: retry transient request failures The HTTP source issues every request with a bare client.Do and returns the error unchanged, so one dropped connection fails the whole solve. A solve can hold thousands of independent HTTP sources, each fetched over its own connection and with little reuse between them, so this is likely rather than rare on a lossy path. A 2000-URL build reproduces it within seconds, consistently: failed to load cache key: Get "http://...": EOF Registry fetches already retry this class of error through retryhandler. Use the same helper here. Only the request is retried. A failure part way through reading the response body is not covered, and status-code handling is unchanged. Signed-off-by: Adam Stokes --- source/http/retry_test.go | 72 +++++++++++++++++++++++++++++++++++++++ source/http/source.go | 25 ++++++++++++-- 2 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 source/http/retry_test.go diff --git a/source/http/retry_test.go b/source/http/retry_test.go new file mode 100644 index 000000000..fba0c4c79 --- /dev/null +++ b/source/http/retry_test.go @@ -0,0 +1,72 @@ +package http + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestDoRequestRetriesDroppedConnection covers a connection dropped before any +// response is written, which reaches the client as EOF. A solve with many HTTP +// sources hits this regularly on a lossy network, and before the retry a +// single occurrence failed the whole build. +func TestDoRequestRetriesDroppedConnection(t *testing.T) { + t.Parallel() + + var attempts atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if attempts.Add(1) <= 2 { + hj, ok := w.(http.Hijacker) + if !ok { + return + } + conn, _, err := hj.Hijack() + if err != nil { + return + } + conn.Close() + return + } + w.Write([]byte("content")) + })) + defer server.Close() + + ctx := t.Context() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL, nil) + require.NoError(t, err) + + resp, err := doRequest(ctx, server.Client(), req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, int32(3), attempts.Load()) +} + +// TestDoRequestDoesNotRetryStatusErrors asserts the retry does not change how +// HTTP status codes are handled: the response is returned to the caller on the +// first attempt, exactly as before. +func TestDoRequestDoesNotRetryStatusErrors(t *testing.T) { + t.Parallel() + + var attempts atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts.Add(1) + w.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + + ctx := t.Context() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, server.URL, nil) + require.NoError(t, err) + + resp, err := doRequest(ctx, server.Client(), req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusNotFound, resp.StatusCode) + require.Equal(t, int32(1), attempts.Load()) +} diff --git a/source/http/source.go b/source/http/source.go index 858805358..f9a9e38fc 100644 --- a/source/http/source.go +++ b/source/http/source.go @@ -31,6 +31,7 @@ import ( "github.com/moby/buildkit/util/bklog" "github.com/moby/buildkit/util/cachedigest" "github.com/moby/buildkit/util/pgpsign" + "github.com/moby/buildkit/util/resolver/retryhandler" "github.com/moby/buildkit/util/tracing" "github.com/moby/buildkit/util/urlutil" "github.com/moby/buildkit/version" @@ -215,6 +216,24 @@ func (hs *httpSourceHandler) client(g session.Group) *http.Client { return &http.Client{Transport: newTransport(hs.transport, hs.sm, g)} } +// doRequest performs req, retrying transient network failures with +// exponential backoff. +// +// A solve can contain thousands of independent HTTP sources, each fetched over +// its own connection and with no connection reuse between them. Without a +// retry, one dropped connection anywhere fails the entire solve. Registry +// fetches already retry this class of error through retryhandler; HTTP sources +// did not. +// +// Only the request is retried. A failure part way through reading the response +// body is not covered here, and neither is a non-2xx response: those are left +// to the caller so that status handling is unchanged. +func doRequest(ctx context.Context, client *http.Client, req *http.Request) (*http.Response, error) { + return retryhandler.WithRetry(ctx, nil, func(ctx context.Context) (*http.Response, error) { + return client.Do(req.Clone(ctx)) + }) +} + // urlHash is internal hash the etag is stored by that doesn't leak outside // this package. func (hs *httpSourceHandler) urlHash() (digest.Digest, error) { @@ -408,7 +427,7 @@ func (hs *httpSourceHandler) resolveMetadataRef(ctx context.Context, jobCtx solv // we need to add accept-encoding header manually because stdlib only adds it to GET requests // some servers will return different etags if Accept-Encoding header is different req.Header.Set("Accept-Encoding", "gzip") - resp, err := client.Do(req) + resp, err := doRequest(ctx, client, req) if err == nil { if resp.StatusCode == http.StatusOK || resp.StatusCode == http.StatusNotModified { respETag := etagValue(resp.Header.Get("ETag")) @@ -454,7 +473,7 @@ func (hs *httpSourceHandler) resolveMetadataRef(ctx context.Context, jobCtx solv req.Header.Del("Accept-Encoding") } - resp, err := client.Do(req) + resp, err := doRequest(ctx, client, req) if err != nil { return nil, err } @@ -889,7 +908,7 @@ func (hs *httpSourceHandler) Snapshot(ctx context.Context, jobCtx solver.JobCont client := hs.client(g) - resp, err := client.Do(req) + resp, err := doRequest(ctx, client, req) if err != nil { return nil, err } From ec5a999673b2e18f546976b4f61e36d1e7156284 Mon Sep 17 00:00:00 2001 From: Adam Stokes Date: Tue, 3 Nov 2026 15:19:10 -0500 Subject: [PATCH 2/2] source/http: do not retry the best-effort HEAD request The HEAD request in resolveMetadataRef is an optimization that servers are not required to support. Its error is already ignored and the code falls through to the GET, which is retried. Retrying the HEAD adds nothing, since the GET covers the same failures, and costs up to ~7s of backoff before the GET even starts. Signed-off-by: Adam Stokes --- source/http/source.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/http/source.go b/source/http/source.go index f9a9e38fc..2813b69bc 100644 --- a/source/http/source.go +++ b/source/http/source.go @@ -427,7 +427,8 @@ func (hs *httpSourceHandler) resolveMetadataRef(ctx context.Context, jobCtx solv // we need to add accept-encoding header manually because stdlib only adds it to GET requests // some servers will return different etags if Accept-Encoding header is different req.Header.Set("Accept-Encoding", "gzip") - resp, err := doRequest(ctx, client, req) + // Best-effort: on any error we fall through to the GET below, which is retried. + resp, err := client.Do(req) if err == nil { if resp.StatusCode == http.StatusOK || resp.StatusCode == http.StatusNotModified { respETag := etagValue(resp.Header.Get("ETag"))