hashicorp / hashicorp/go-retryablehttp
RateLimitLinearJitterBackoff ignores RetryWaitMax for Retry-After (same as DefaultBackoff)
- 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