hashicorp / hashicorp/go-retryablehttp

Backoff wait sleep should check that it wouldn't exceed the context Deadline

Open
#272 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
2.3k
Forks
298
PR merge metrics
No merged PRs in 30d

Description

For example, if the backoff is such that the next attempt is in 10 minutes, and the context's deadline is in 5 minutes, then the code will wait for 5 minutes for no reason.

Refer to this code:

https://github.com/hashicorp/go-retryablehttp/blob/867c4f597d1322e1642f39fd366f9df7a5c2cdc2/client.go#L771-L791

Before the `select` block, the future retry time should be checked to see if it is beyond the current context's deadline, and return early error in that case.

Also side note: the use of `time.Timer` and `.Stop()` is unnecessary and can just be `time.After(...)` in modern go, and even if backwards compatibility is an issue then `defer timer.Stop()` should be preferred...

So, how about this?
```
nextTime := time.Now().Add(wait)
if cd, ok := ctx.Deadline(); ok && nextTime.After(cd) {
return context.DeadlineExceeded // Its the same error we would have returned, we're just returning it earlier...
}
select {
case <-req.Context().Done():
c.HTTPClient.CloseIdleConnections()
return nil, req.Context().Err()
case <-time.After(time.Until(nextTime)):
}
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.