hashicorp / hashicorp/go-retryablehttp

RateLimitLinearJitterBackoff ignores RetryWaitMax for Retry-After (same as DefaultBackoff)

Open Beginner friendly
#295 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
2.3k
Forks
298
PR merge metrics
No merged PRs in 30d

Description

## Summary

`DefaultBackoff` is known not to cap `Retry-After` by `RetryWaitMax` (#247, open PR #283). The same unbounded `Retry-After` path exists in `RateLimitLinearJitterBackoff` (and any caller using that helper):

```go
func RateLimitLinearJitterBackoff(min, max time.Duration, attemptNum int, resp *http.Response) time.Duration {
if resp != nil {
if resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode == http.StatusServiceUnavailable {
if sleep, ok := parseRetryAfterHeader(resp.Header["Retry-After"]); ok {
return sleep // max is ignored
}
}
}
return LinearJitterBackoff(min, max, attemptNum, resp)
}
```

`DefaultBackoff` has the same early return. Exponential/jitter branches do enforce `max`; the `Retry-After` branch does not.

## Impact

A 429/503 with `Retry-After: 3600` (or a far-future HTTP-date) makes the client sleep for that full duration even when `Client.RetryWaitMax` is a few seconds. That can stall workers and block shutdown longer than operators expect from `RetryWaitMax`.

## Expected behaviour

Either:

1. Cap `Retry-After` sleeps with `max` (`min(parsed, max)`), or
2. Document that `RetryWaitMax` does not apply when `Retry-After` is present, and offer a separate knob.

Option 1 matches how people usually read `RetryWaitMax` (hard ceiling on wait).

## Suggested fix

Share one helper used by both `DefaultBackoff` and `RateLimitLinearJitterBackoff`:

```go
if sleep, ok := parseRetryAfterHeader(resp.Header["Retry-After"]); ok {
if sleep > max {
return max
}
return sleep
}
```

## Related

- #247 (`DefaultBackoff` only)
- #283 (open PR for `DefaultBackoff`; this issue asks for the same policy on `RateLimitLinearJitterBackoff` so the two helpers stay consistent)

## Minimal repro sketch

```go
// Server returns 429 with Retry-After: 3600
// Client: RetryWaitMax = 2 * time.Second, Backoff = RateLimitLinearJitterBackoff
// Observe sleep ~3600s instead of <= 2s
```

Happy to send a small PR that caps both helpers and adds table tests for `Retry-After` vs `max`.

Contributor guide

No contributing guide indexed for this repository

Research direction

Locate RateLimitLinearJitterBackoff, DefaultBackoff, LinearJitterBackoff, and parseRetryAfterHeader, then trace how Retry-After is handled before the jitter fallback. Add table coverage for Retry-After values against max in both helpers, and confirm the retry wait never exceeds max while existing fallback behavior remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
networking
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.