From 5eed589a5784ff94fd8fe3cb8d3a1f3b76c06e15 Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Thu, 24 Sep 2026 15:52:42 +0200 Subject: [PATCH] fix: stop the blind backoff from growing after timed retries The backoff doubled per attempt and gave up only when the attempt number equaled the blind limit. Once the server had given a time for the first retries, later retries without one waited 16 minutes and more, and never gave up. --- internal/httpclient/httpclient.go | 11 +++++++---- internal/httpclient/httpclient_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/internal/httpclient/httpclient.go b/internal/httpclient/httpclient.go index 022aba6..4d74225 100644 --- a/internal/httpclient/httpclient.go +++ b/internal/httpclient/httpclient.go @@ -74,6 +74,8 @@ func (c *Client) Get(url string, headers map[string]string) ([]byte, error) { // GetWithContext is Get with a context. The request and the waits between // retries stop when ctx is cancelled or reaches its deadline. func (c *Client) GetWithContext(ctx context.Context, url string, headers map[string]string) ([]byte, error) { + blind := 0 + for attempt := 1; ; attempt++ { resp, err := c.do(ctx, url, headers) if err != nil { @@ -87,13 +89,14 @@ func (c *Client) GetWithContext(ctx context.Context, url string, headers map[str _ = resp.Body.Close() - limit := maxAttempts + // The backoff doubles per blind retry, not per attempt: retries + // where the server gave a time must not inflate it. if !told { - limit = maxBlindAttempts - wait = time.Minute << (attempt - 1) + blind++ + wait = time.Minute << (blind - 1) } - if attempt == limit { + if attempt >= maxAttempts || blind >= maxBlindAttempts { return nil, fmt.Errorf("%w after %d attempts: %s", ErrRateLimited, attempt, resp.Status) } diff --git a/internal/httpclient/httpclient_test.go b/internal/httpclient/httpclient_test.go index c229d8c..e11c494 100644 --- a/internal/httpclient/httpclient_test.go +++ b/internal/httpclient/httpclient_test.go @@ -361,3 +361,28 @@ func TestGetWithContextCancelInterruptsBackoff(t *testing.T) { t.Errorf("cancel did not interrupt backoff: elapsed %v", elapsed) } } + +func TestGetCountsBlindRetriesOnTheirOwn(t *testing.T) { + steps := []step{ + {status: http.StatusTooManyRequests, retryAfter: "1"}, + {status: http.StatusTooManyRequests, retryAfter: "1"}, + {status: http.StatusTooManyRequests, retryAfter: "1"}, + {status: http.StatusTooManyRequests, retryAfter: "1"}, + } + for range maxBlindAttempts { + steps = append(steps, step{status: http.StatusTooManyRequests}) + } + c, url, waits := newClient(t, steps...) + + _, err := c.Get(url, nil) + if !errors.Is(err, ErrRateLimited) { + t.Errorf("err = %v, want ErrRateLimited", err) + } + want := []time.Duration{ + time.Second, time.Second, time.Second, time.Second, + time.Minute, 2 * time.Minute, 4 * time.Minute, + } + if !slices.Equal(*waits, want) { + t.Errorf("waits = %v, want %v", *waits, want) + } +}