From cd3f6600ca6661d6fffaae3ac0bc60f05e3fb233 Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Mon, 30 Mar 2026 13:04:51 +0800 Subject: [PATCH 1/4] fix(utils): honor Retry-After for 429 retries --- pkg/utils/http_retry.go | 29 +++++++++++++- pkg/utils/http_retry_test.go | 75 ++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/pkg/utils/http_retry.go b/pkg/utils/http_retry.go index 135ea0ef5..812271290 100644 --- a/pkg/utils/http_retry.go +++ b/pkg/utils/http_retry.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "strconv" "time" ) @@ -36,7 +37,7 @@ func DoRequestWithRetry(client *http.Client, req *http.Request) (*http.Response, } if i < maxRetries-1 { - if err = sleepWithCtx(req.Context(), retryDelayUnit*time.Duration(i+1)); err != nil { + if err = sleepWithCtx(req.Context(), retryDelayForAttempt(resp, i)); err != nil { if resp != nil { resp.Body.Close() } @@ -47,6 +48,32 @@ func DoRequestWithRetry(client *http.Client, req *http.Request) (*http.Response, return resp, err } +func retryDelayForAttempt(resp *http.Response, attempt int) time.Duration { + fallback := retryDelayUnit * time.Duration(attempt+1) + if resp == nil || resp.StatusCode != http.StatusTooManyRequests { + return fallback + } + + retryAfter := resp.Header.Get("Retry-After") + if retryAfter == "" { + return fallback + } + + if seconds, err := strconv.Atoi(retryAfter); err == nil && seconds >= 0 { + return time.Duration(seconds) * time.Second + } + + if when, err := http.ParseTime(retryAfter); err == nil { + delay := time.Until(when) + if delay < 0 { + return 0 + } + return delay + } + + return fallback +} + func sleepWithCtx(ctx context.Context, d time.Duration) error { timer := time.NewTimer(d) defer timer.Stop() diff --git a/pkg/utils/http_retry_test.go b/pkg/utils/http_retry_test.go index d64cd5eda..918245b57 100644 --- a/pkg/utils/http_retry_test.go +++ b/pkg/utils/http_retry_test.go @@ -80,6 +80,81 @@ func TestDoRequestWithRetry(t *testing.T) { } } +func TestDoRequestWithRetry_RetryAfter429Honored(t *testing.T) { + retryDelayUnit = 10 * time.Millisecond + t.Cleanup(func() { retryDelayUnit = time.Second }) + + attempts := 0 + var firstAttemptAt time.Time + var secondAttemptAt time.Time + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + if attempts == 1 { + firstAttemptAt = time.Now() + w.Header().Set("Retry-After", "1") + w.WriteHeader(http.StatusTooManyRequests) + return + } + if attempts == 2 { + secondAttemptAt = time.Now() + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client := &http.Client{Timeout: 5 * time.Second} + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) + + resp, err := DoRequestWithRetry(client, req) + require.NoError(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() + require.Equal(t, 2, attempts) + + assert.GreaterOrEqual(t, secondAttemptAt.Sub(firstAttemptAt), 900*time.Millisecond) +} + +func TestDoRequestWithRetry_RetryAfter429InvalidFallsBack(t *testing.T) { + retryDelayUnit = 50 * time.Millisecond + t.Cleanup(func() { retryDelayUnit = time.Second }) + + attempts := 0 + var firstAttemptAt time.Time + var secondAttemptAt time.Time + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + if attempts == 1 { + firstAttemptAt = time.Now() + w.Header().Set("Retry-After", "invalid") + w.WriteHeader(http.StatusTooManyRequests) + return + } + if attempts == 2 { + secondAttemptAt = time.Now() + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client := &http.Client{Timeout: 5 * time.Second} + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) + + resp, err := DoRequestWithRetry(client, req) + require.NoError(t, err) + require.NotNil(t, resp) + assert.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() + require.Equal(t, 2, attempts) + + assert.GreaterOrEqual(t, secondAttemptAt.Sub(firstAttemptAt), 45*time.Millisecond) + assert.Less(t, secondAttemptAt.Sub(firstAttemptAt), 500*time.Millisecond) +} + func TestDoRequestWithRetry_ContextCancel(t *testing.T) { // Use a long retry delay so cancellation always hits during sleepWithCtx. retryDelayUnit = 10 * time.Second From 9440bebca6f8daa683c8b5519809840ebb65daa1 Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Mon, 30 Mar 2026 21:58:12 +0800 Subject: [PATCH 2/4] utils: anchor date retry-after to response date and cap delay --- pkg/utils/http_retry.go | 24 +++++++++++++++---- pkg/utils/http_retry_test.go | 46 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/pkg/utils/http_retry.go b/pkg/utils/http_retry.go index 812271290..fcd8f50f5 100644 --- a/pkg/utils/http_retry.go +++ b/pkg/utils/http_retry.go @@ -11,6 +11,7 @@ import ( const maxRetries = 3 var retryDelayUnit = time.Second +var maxRetrySleepDuration = 1 * time.Minute func shouldRetry(statusCode int) bool { return statusCode == http.StatusTooManyRequests || @@ -51,27 +52,40 @@ func DoRequestWithRetry(client *http.Client, req *http.Request) (*http.Response, func retryDelayForAttempt(resp *http.Response, attempt int) time.Duration { fallback := retryDelayUnit * time.Duration(attempt+1) if resp == nil || resp.StatusCode != http.StatusTooManyRequests { - return fallback + return clampRetryDelay(fallback) } retryAfter := resp.Header.Get("Retry-After") if retryAfter == "" { - return fallback + return clampRetryDelay(fallback) } if seconds, err := strconv.Atoi(retryAfter); err == nil && seconds >= 0 { - return time.Duration(seconds) * time.Second + return clampRetryDelay(time.Duration(seconds) * time.Second) } if when, err := http.ParseTime(retryAfter); err == nil { delay := time.Until(when) + if serverDate, err := http.ParseTime(resp.Header.Get("Date")); err == nil { + delay = when.Sub(serverDate) + } if delay < 0 { return 0 } - return delay + return clampRetryDelay(delay) } - return fallback + return clampRetryDelay(fallback) +} + +func clampRetryDelay(delay time.Duration) time.Duration { + if delay <= 0 { + return 0 + } + if delay > maxRetrySleepDuration { + return maxRetrySleepDuration + } + return delay } func sleepWithCtx(ctx context.Context, d time.Duration) error { diff --git a/pkg/utils/http_retry_test.go b/pkg/utils/http_retry_test.go index 918245b57..1e5ac4064 100644 --- a/pkg/utils/http_retry_test.go +++ b/pkg/utils/http_retry_test.go @@ -279,3 +279,49 @@ func TestDoRequestWithRetry_Delay(t *testing.T) { assert.GreaterOrEqual(t, delays[2], time.Millisecond) } + +func TestRetryDelayForAttempt_DateRetryAfterUsesResponseDateHeader(t *testing.T) { + maxRetrySleepDuration = time.Minute + t.Cleanup(func() { maxRetrySleepDuration = time.Minute }) + + serverDate := time.Date(2000, 1, 2, 15, 4, 5, 0, time.UTC) + retryAfterAt := serverDate.Add(10 * time.Second) + resp := &http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: http.Header{ + "Retry-After": []string{retryAfterAt.Format(http.TimeFormat)}, + "Date": []string{serverDate.Format(http.TimeFormat)}, + }, + } + + assert.Equal(t, 10*time.Second, retryDelayForAttempt(resp, 0)) +} + +func TestRetryDelayForAttempt_DateRetryAfterInvalidDateFallsBackSafely(t *testing.T) { + maxRetrySleepDuration = time.Minute + t.Cleanup(func() { maxRetrySleepDuration = time.Minute }) + + resp := &http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: http.Header{ + "Retry-After": []string{time.Date(2000, 1, 2, 15, 4, 5, 0, time.UTC).Format(http.TimeFormat)}, + "Date": []string{"invalid-date"}, + }, + } + + assert.Equal(t, time.Duration(0), retryDelayForAttempt(resp, 0)) +} + +func TestRetryDelayForAttempt_RetryAfterIsCapped(t *testing.T) { + maxRetrySleepDuration = 2 * time.Second + t.Cleanup(func() { maxRetrySleepDuration = time.Minute }) + + resp := &http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: http.Header{ + "Retry-After": []string{"999999"}, + }, + } + + assert.Equal(t, 2*time.Second, retryDelayForAttempt(resp, 0)) +} From 345d4fddc9b2a1d96b366b0ebd3fe6421eb09d6c Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Mon, 30 Mar 2026 22:02:16 +0800 Subject: [PATCH 3/4] utils: make retry-after numeric clamp overflow-safe --- pkg/utils/http_retry.go | 16 +++++++++-- pkg/utils/http_retry_test.go | 54 ++++++++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/pkg/utils/http_retry.go b/pkg/utils/http_retry.go index fcd8f50f5..a808759ae 100644 --- a/pkg/utils/http_retry.go +++ b/pkg/utils/http_retry.go @@ -60,8 +60,8 @@ func retryDelayForAttempt(resp *http.Response, attempt int) time.Duration { return clampRetryDelay(fallback) } - if seconds, err := strconv.Atoi(retryAfter); err == nil && seconds >= 0 { - return clampRetryDelay(time.Duration(seconds) * time.Second) + if delay, ok := numericRetryAfterDelay(retryAfter); ok { + return delay } if when, err := http.ParseTime(retryAfter); err == nil { @@ -78,6 +78,18 @@ func retryDelayForAttempt(resp *http.Response, attempt int) time.Duration { return clampRetryDelay(fallback) } +func numericRetryAfterDelay(retryAfter string) (time.Duration, bool) { + seconds, err := strconv.ParseInt(retryAfter, 10, 64) + if err != nil || seconds < 0 { + return 0, false + } + maxSeconds := int64(maxRetrySleepDuration / time.Second) + if seconds > maxSeconds { + return maxRetrySleepDuration, true + } + return clampRetryDelay(time.Duration(seconds) * time.Second), true +} + func clampRetryDelay(delay time.Duration) time.Duration { if delay <= 0 { return 0 diff --git a/pkg/utils/http_retry_test.go b/pkg/utils/http_retry_test.go index 1e5ac4064..4d6021ff7 100644 --- a/pkg/utils/http_retry_test.go +++ b/pkg/utils/http_retry_test.go @@ -297,19 +297,43 @@ func TestRetryDelayForAttempt_DateRetryAfterUsesResponseDateHeader(t *testing.T) assert.Equal(t, 10*time.Second, retryDelayForAttempt(resp, 0)) } -func TestRetryDelayForAttempt_DateRetryAfterInvalidDateFallsBackSafely(t *testing.T) { - maxRetrySleepDuration = time.Minute +func TestRetryDelayForAttempt_DateRetryAfterInvalidOrMissingDateFallsBackSafely(t *testing.T) { + maxRetrySleepDuration = 30 * time.Second t.Cleanup(func() { maxRetrySleepDuration = time.Minute }) - resp := &http.Response{ - StatusCode: http.StatusTooManyRequests, - Header: http.Header{ - "Retry-After": []string{time.Date(2000, 1, 2, 15, 4, 5, 0, time.UTC).Format(http.TimeFormat)}, - "Date": []string{"invalid-date"}, + retryAfterAt := time.Now().UTC().Add(3 * time.Second).Format(http.TimeFormat) + testcases := []struct { + name string + header http.Header + }{ + { + name: "invalid-date-header", + header: http.Header{ + "Retry-After": []string{retryAfterAt}, + "Date": []string{"invalid-date"}, + }, + }, + { + name: "missing-date-header", + header: http.Header{ + "Retry-After": []string{retryAfterAt}, + }, }, } - assert.Equal(t, time.Duration(0), retryDelayForAttempt(resp, 0)) + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + resp := &http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: tc.header, + } + + delay := retryDelayForAttempt(resp, 0) + assert.Greater(t, delay, time.Duration(0)) + assert.GreaterOrEqual(t, delay, 1500*time.Millisecond) + assert.LessOrEqual(t, delay, 5*time.Second) + }) + } } func TestRetryDelayForAttempt_RetryAfterIsCapped(t *testing.T) { @@ -325,3 +349,17 @@ func TestRetryDelayForAttempt_RetryAfterIsCapped(t *testing.T) { assert.Equal(t, 2*time.Second, retryDelayForAttempt(resp, 0)) } + +func TestRetryDelayForAttempt_RetryAfterNumericOverflowStillCaps(t *testing.T) { + maxRetrySleepDuration = 2 * time.Second + t.Cleanup(func() { maxRetrySleepDuration = time.Minute }) + + resp := &http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: http.Header{ + "Retry-After": []string{"9223372036854775807"}, + }, + } + + assert.Equal(t, 2*time.Second, retryDelayForAttempt(resp, 0)) +} From 711685192c6e63e0a007114c68fe3dcf4585532f Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Mon, 30 Mar 2026 22:08:21 +0800 Subject: [PATCH 4/4] utils: gofumpt http retry formatting --- pkg/utils/http_retry.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/utils/http_retry.go b/pkg/utils/http_retry.go index a808759ae..514f9781b 100644 --- a/pkg/utils/http_retry.go +++ b/pkg/utils/http_retry.go @@ -10,8 +10,10 @@ import ( const maxRetries = 3 -var retryDelayUnit = time.Second -var maxRetrySleepDuration = 1 * time.Minute +var ( + retryDelayUnit = time.Second + maxRetrySleepDuration = 1 * time.Minute +) func shouldRetry(statusCode int) bool { return statusCode == http.StatusTooManyRequests ||