hashicorp / hashicorp/go-retryablehttp
Preserve url.Error/http.httpError Timeout
- Dominant language
- Go
- Stars
- 2.3k
- Forks
- 298
- PR merge metrics
- No merged PRs in 30d
Description
Similar to https://github.com/hashicorp/go-retryablehttp/issues/129 but instead of context.Deadline it is for url.Error/http.httpError Timeout().
When a timeout occurs, expecting the error returned to have `url.Error` with a `Timeout() == true`. Because https://github.com/hashicorp/go-retryablehttp/blob/master/client.go#L693 wraps the error from `client.Do` and then Go std then puts this error into a `url.Error` type (https://cs.opensource.google/go/go/+/refs/tags/go1.17.1:src/net/http/client.go;l=621-636) the top level `url.Error`'s `Timeout() == false`. This means you need to unwrap the top level error, then call `errors.As` to get a `url.Error` with a `Timeout() == true`.
To reproduce:
```go
package main
import (
"context"
"errors"
"net/http"
"net/http/httptest"
"net/url"
"time"
"github.com/hashicorp/go-retryablehttp"
)
func main() {
var (
ctx = context.Background()
timeout = time.Millisecond
retryableClient = retryablehttp.NewClient()
ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// always exceed deadline
time.Sleep(timeout * 2)
}))
)
defer ts.Close()
retryableClient.Logger = nil
retryableClient.RetryMax = 1
retryableClient.Backoff = retryablehttp.DefaultBackoff
retryableClient.HTTPClient.Timeout = timeout
retryableClient.RetryWaitMin = time.Millisecond
retryableClient.RetryWaitMax = time.Millisecond
retryableClient.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) {
return true, nil
}
client := retryableClient.StandardClient()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL, nil)
if err != nil {
panic(err)
}
_, err = client.Do(req)
var urlErr *url.Error
if errors.As(err, &urlErr) {
if !urlErr.Timeout() {
panic("urlErr.Timeout() is false")
}
} else {
panic("not url.Error")
}
}
```
Possible solution would be to add some Error type that preserves the original `url.Error`. Though I am sure this could be handled another way.
```go
// Error reports an error and an optional message along with it.
type Error struct {
Err error
Msg string
}
func (e *Error) Error() string {
if e.Msg != "" {
return fmt.Sprintf("%s: %v", e.Msg, e.Err)
}
return e.Err.Error()
}
func (e *Error) Unwrap() error {
return errors.Unwrap(e.Err)
}
// Timeout will check if the e.Err fulfills "Timeout() bool" interface
// This is important for when client.Do returns a url.Error/http.httpError
// because of a timeout.
func (e *Error) Timeout() bool {
t, ok := e.Err.(interface {
Timeout() bool
})
return ok && t.Timeout()
}
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.