fix: cap honored Retry-After waits at 60 seconds

Sleeps between retry attempts sit outside the per-attempt timeout,
so honoring an arbitrary Retry-After could park a call for hours.
A mandated wait beyond 60s now surfaces the 429 immediately, with
the full wait still available on the error's RateLimit.RetryAfter.

Author: Zingzy

client.go

@@ -129,7 +129,10 @@ func (c *Client) request(ctx context.Context, method, path string, query url.Val
 }
 
 // send builds and performs one HTTP call, retrying transient failures
-// with exponential backoff and jitter, honoring Retry-After. What
+// with exponential backoff and jitter, honoring Retry-After up to a
+// 60-second cap: a longer mandated wait surfaces the response
+// immediately instead of parking the goroutine (the full wait still
+// reaches the caller via the error's RateLimit.RetryAfter). What
 // counts as transient depends on the method: see
 // transport.RetryableStatus. Callers own the response body.
 func (c *Client) send(ctx context.Context, method, path string, query url.Values, body any, creds Credentials, extra http.Header) (*http.Response, error) {
@@ -153,7 +156,8 @@ func (c *Client) send(ctx context.Context, method, path string, query url.Values
 			if !transport.IdempotentMethod(method) || attempt >= c.maxRetries || ctx.Err() != nil {
 				return nil, err
 			}
-		} else if !transport.RetryableStatus(method, resp.StatusCode) || attempt >= c.maxRetries {
+		} else if !transport.RetryableStatus(method, resp.StatusCode) || attempt >= c.maxRetries ||
+			transport.RetryAfterExceedsCap(resp.Header.Get("Retry-After")) {
 			return resp, nil
 		}
 		var retryAfter string

internal/transport/transport.go

@@ -19,6 +19,14 @@ const (
 	RetryBaseDelay = 500 * time.Millisecond
 	// retryMaxDelay caps a single computed backoff.
 	retryMaxDelay = 8 * time.Second
+	// maxRetryAfter is the longest server-mandated Retry-After the
+	// retry loop will honor. Sleeps between attempts fall outside the
+	// per-attempt timeout, so honoring an arbitrary Retry-After would
+	// park the caller for unbounded wall-clock time per call. Beyond
+	// this cap the response surfaces immediately instead; the full
+	// mandated wait still reaches the caller through the error's
+	// RateLimit.RetryAfter.
+	maxRetryAfter = 60 * time.Second
 )
 
 // IdempotentMethod reports whether an HTTP method is safe to replay
@@ -63,6 +71,14 @@ func RetryDelay(base time.Duration, attempt int, retryAfter string) time.Duratio
 	return d/2 + rand.N(d/2+1) //nolint:gosec // math/rand jitter; no security material
 }
 
+// RetryAfterExceedsCap reports whether a parsable Retry-After header
+// mandates a wait longer than maxRetryAfter. Callers give up on
+// retrying and surface the response instead when it does.
+func RetryAfterExceedsCap(v string) bool {
+	d, ok := ParseRetryAfter(v, time.Now())
+	return ok && d > maxRetryAfter
+}
+
 // ParseRetryAfter reads a Retry-After header value: delay seconds or an
 // HTTP date (RFC 9110 §10.2.3). ok is false when absent or malformed.
 func ParseRetryAfter(v string, now time.Time) (_ time.Duration, ok bool) {

internal/transport/transport_test.go

@@ -1,6 +1,34 @@
 package transport
 
-import "testing"
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+// Only a parsable Retry-After longer than maxRetryAfter trips the cap;
+// exactly 60s is still honored, and absent or malformed headers fall
+// through to computed backoff.
+func TestRetryAfterExceedsCap(t *testing.T) {
+	tests := []struct {
+		v    string
+		want bool
+	}{
+		{"", false},
+		{"garbage", false},
+		{"0", false},
+		{"60", false},
+		{"61", true},
+		{"3600", true},
+		{time.Now().Add(2 * time.Hour).UTC().Format(http.TimeFormat), true},
+		{time.Now().Add(5 * time.Second).UTC().Format(http.TimeFormat), false},
+	}
+	for _, tt := range tests {
+		if got := RetryAfterExceedsCap(tt.v); got != tt.want {
+			t.Errorf("RetryAfterExceedsCap(%q) = %v, want %v", tt.v, got, tt.want)
+		}
+	}
+}
 
 // The filename is server-supplied and consumers hand it straight to
 // os.Create, so path-shaped suggestions must never survive: relative

retry_test.go

@@ -129,6 +129,68 @@ func TestRetryHonorsRetryAfter(t *testing.T) {
 	}
 }
 
+// A Retry-After within the 60s cap stays authoritative: the retry
+// waits out the mandated second before the next attempt.
+func TestRetryAfterWithinCapHonored(t *testing.T) {
+	var attempts atomic.Int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		if attempts.Add(1) == 1 {
+			w.Header().Set("Retry-After", "1")
+			w.WriteHeader(http.StatusTooManyRequests)
+			w.Write([]byte(`{"error":"slow down","code":"rate_limit_exceeded"}`))
+			return
+		}
+		w.Write([]byte(`{"user":{"id":"1"}}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient(option.WithBaseURL(srv.URL))
+	fastRetries(c)
+	start := time.Now()
+	if _, err := c.Me(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+	if elapsed := time.Since(start); elapsed < time.Second {
+		t.Fatalf("retry took %v, the mandated 1s wait was not honored", elapsed)
+	}
+	if attempts.Load() != 2 {
+		t.Fatalf("attempts = %d, want 2", attempts.Load())
+	}
+}
+
+// A Retry-After beyond the cap would park the goroutine for the whole
+// mandated wait (sleeps between attempts sit outside the per-attempt
+// timeout), so the 429 must surface immediately, carrying the full
+// wait on the error for the caller to schedule around.
+func TestRetryAfterBeyondCapSurfacesImmediately(t *testing.T) {
+	var attempts atomic.Int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		attempts.Add(1)
+		w.Header().Set("Retry-After", "3600")
+		w.WriteHeader(http.StatusTooManyRequests)
+		w.Write([]byte(`{"error":"slow down","code":"rate_limit_exceeded"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient(option.WithBaseURL(srv.URL))
+	fastRetries(c)
+	start := time.Now()
+	_, err := c.Me(context.Background())
+	var apiErr *Error
+	if !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusTooManyRequests {
+		t.Fatalf("err = %v, want *Error with 429", err)
+	}
+	if apiErr.RateLimit.RetryAfter != 3600*time.Second {
+		t.Fatalf("RetryAfter = %v, want 1h (the full mandated wait)", apiErr.RateLimit.RetryAfter)
+	}
+	if elapsed := time.Since(start); elapsed > 2*time.Second {
+		t.Fatalf("took %v, the hour-long Retry-After was slept on", elapsed)
+	}
+	if attempts.Load() != 1 {
+		t.Fatalf("attempts = %d, want 1 (a wait past the cap must not retry)", attempts.Load())
+	}
+}
+
 // failOnceTransport drops the first request at the transport level to
 // simulate a connection error, then delegates.
 type failOnceTransport struct {