Skip to content

Default backoff doesn't honour RetryWaitMax when "Retry-After" header is sent in the response #247

@Hemanthk1099

Description

@Hemanthk1099

go-retryablehttp/client.go

Lines 551 to 566 in 9dfd949

func DefaultBackoff(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
}
}
}
mult := math.Pow(2, float64(attemptNum)) * float64(min)
sleep := time.Duration(mult)
if float64(sleep) != mult || sleep > max {
sleep = max
}
return sleep
}

The DefaultBackoff in function has a logical flaw when handling the Retry-After header. If the server provides an unreasonably high value in the Retry-After header, the function respects it without enforcing the RetryWaitMax limit set for the httpClient. This can lead to indefinite wait times or blocking behaviour.

Steps to Reproduce

  1. Simulate a server response with an HTTP 429 Too Many Requests status.
  2. Set a high value in the Retry-After header (e.g., Retry-After: 3600 for 1 hour).
  3. Observe that the backoff duration exceeds the configured RetryWaitMax limit.

Expected Behaviour

I believe the backoff duration should always be bounded by the configured RetryWaitMax value, regardless of the Retry-After header value.

Suggested Fix

Honour "Retry-After" header value only if its less than or equal to RetryWaitMax in default retry strategy

if sleep <= max {
    return max // Enforce max backoff limit
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions