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

[Hemanthk1099]

#247

Currently 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.

This change will make sure that "Retry-After" header value is honoured only if its less than or equal to RetryWaitMax in default retry strategy.

[hashicorp-cla-app]

All committers have signed the CLA.

[julianmaze]

Can we get a PR review on this? This is a much-needed addition by @Hemanthk1099. I too am experiencing seemingly stuck HTTP calls when a 429 Retry-After value is higher than RetryWaitMax.

[davidcollom]

I too have this issue with calls to the Docker Container Registry...

INFO[0036] [DEBUG] GET https://registry.hub.docker.com/v2/repositories/XXXXX/XXXXX/tags?page_size=100 (status: 429): retrying in 484289h20m8s (10 left)  client=docker component=clientz

🙈

I did make a work around by implementing the following:

func HTTPBackOff(min, max time.Duration, attemptNum int, resp *http.Response) time.Duration {
	sleep := retryablehttp.DefaultBackoff(min, max, attemptNum, resp)
	if sleep <= max {
		return sleep
	}

	return 0
}

// Then Setup the client..
retryclient := retryablehttp.NewClient()
retryclient.HTTPClient.Timeout = 10 * time.Second
retryclient.RetryMax = 10
retryclient.RetryWaitMax = 10 * time.Minute
retryclient.RetryWaitMin = 1 * time.Second
retryclient.Backoff = HTTPBackOff