diff --git a/go.mod b/go.mod index 242179d2153..badbca06cd7 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( github.com/google/go-github/v42 v42.0.0 github.com/google/trillian v1.4.0 github.com/hashicorp/go-cleanhttp v0.5.2 - github.com/hashicorp/go-retryablehttp v0.7.0 + github.com/hashicorp/go-retryablehttp v0.7.1 github.com/hashicorp/go-rootcerts v1.0.2 github.com/hashicorp/go-secure-stdlib/parseutil v0.1.4 github.com/hashicorp/yamux v0.0.0-20211028200310-0bc27b27de87 diff --git a/go.sum b/go.sum index 51de62677c0..ba585bdd774 100644 --- a/go.sum +++ b/go.sum @@ -1326,8 +1326,9 @@ github.com/hashicorp/go-retryablehttp v0.5.3/go.mod h1:9B5zBasrRhHXnJnui7y6sL7es github.com/hashicorp/go-retryablehttp v0.6.4/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY= github.com/hashicorp/go-retryablehttp v0.6.6/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY= github.com/hashicorp/go-retryablehttp v0.6.8/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY= -github.com/hashicorp/go-retryablehttp v0.7.0 h1:eu1EI/mbirUgP5C8hVsTNaGZreBDlYiwC1FZWkvQPQ4= github.com/hashicorp/go-retryablehttp v0.7.0/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY= +github.com/hashicorp/go-retryablehttp v0.7.1 h1:sUiuQAnLlbvmExtFQs72iFW/HXeUn8Z1aJLQ4LJJbTQ= +github.com/hashicorp/go-retryablehttp v0.7.1/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY= github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU= github.com/hashicorp/go-rootcerts v1.0.2 h1:jzhAVGtqPKbwpyCPELlgNWhE1znq+qwJtW5Oi2viEzc= github.com/hashicorp/go-rootcerts v1.0.2/go.mod h1:pqUvnprVnM5bf7AOirdbb01K4ccR319Vf4pU3K5EGc8= diff --git a/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/README.md b/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/README.md index 8943becf19b..09f5eaf2217 100644 --- a/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/README.md +++ b/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/README.md @@ -45,6 +45,25 @@ The returned response object is an `*http.Response`, the same thing you would usually get from `net/http`. Had the request failed one or more times, the above call would block and retry with exponential backoff. +## Retrying cases that fail after a seeming success + +It's possible for a request to succeed in the sense that the expected response headers are received, but then to encounter network-level errors while reading the response body. In go-retryablehttp's most basic usage, this error would not be retryable, due to the out-of-band handling of the response body. In some cases it may be desirable to handle the response body as part of the retryable operation. + +A toy example (which will retry the full request and succeed on the second attempt) is shown below: + +```go +c := retryablehttp.NewClient() +r := retryablehttp.NewRequest("GET", "://foo", nil) +handlerShouldRetry := true +r.SetResponseHandler(func(*http.Response) error { + if !handlerShouldRetry { + return nil + } + handlerShouldRetry = false + return errors.New("retryable error") +}) +``` + ## Getting a stdlib `*http.Client` with retries It's possible to convert a `*retryablehttp.Client` directly to a `*http.Client`. diff --git a/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/client.go b/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/client.go index adbdd92e3ba..57116e96072 100644 --- a/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/client.go +++ b/third_party/VENDOR-LICENSE/github.com/hashicorp/go-retryablehttp/client.go @@ -69,11 +69,21 @@ var ( // scheme specified in the URL is invalid. This error isn't typed // specifically so we resort to matching on the error string. schemeErrorRe = regexp.MustCompile(`unsupported protocol scheme`) + + // A regular expression to match the error returned by net/http when the + // TLS certificate is not trusted. This error isn't typed + // specifically so we resort to matching on the error string. + notTrustedErrorRe = regexp.MustCompile(`certificate is not trusted`) ) // ReaderFunc is the type of function that can be given natively to NewRequest type ReaderFunc func() (io.Reader, error) +// ResponseHandlerFunc is a type of function that takes in a Response, and does something with it. +// It only runs if the initial part of the request was successful. +// If an error is returned, the client's retry policy will be used to determine whether to retry the whole request. +type ResponseHandlerFunc func(*http.Response) error + // LenReader is an interface implemented by many in-memory io.Reader's. Used // for automatically sending the right Content-Length header when possible. type LenReader interface { @@ -86,6 +96,8 @@ type Request struct { // used to rewind the request data in between retries. body ReaderFunc + responseHandler ResponseHandlerFunc + // Embed an HTTP request directly. This makes a *Request act exactly // like an *http.Request so that all meta methods are supported. *http.Request @@ -94,8 +106,16 @@ type Request struct { // WithContext returns wrapped Request with a shallow copy of underlying *http.Request // with its context changed to ctx. The provided ctx must be non-nil. func (r *Request) WithContext(ctx context.Context) *Request { - r.Request = r.Request.WithContext(ctx) - return r + return &Request{ + body: r.body, + responseHandler: r.responseHandler, + Request: r.Request.WithContext(ctx), + } +} + +// SetResponseHandler allows setting the response handler. +func (r *Request) SetResponseHandler(fn ResponseHandlerFunc) { + r.responseHandler = fn } // BodyBytes allows accessing the request body. It is an analogue to @@ -252,23 +272,31 @@ func FromRequest(r *http.Request) (*Request, error) { return nil, err } // Could assert contentLength == r.ContentLength - return &Request{bodyReader, r}, nil + return &Request{body: bodyReader, Request: r}, nil } // NewRequest creates a new wrapped request. func NewRequest(method, url string, rawBody interface{}) (*Request, error) { + return NewRequestWithContext(context.Background(), method, url, rawBody) +} + +// NewRequestWithContext creates a new wrapped request with the provided context. +// +// The context controls the entire lifetime of a request and its response: +// obtaining a connection, sending the request, and reading the response headers and body. +func NewRequestWithContext(ctx context.Context, method, url string, rawBody interface{}) (*Request, error) { bodyReader, contentLength, err := getBodyReaderAndContentLength(rawBody) if err != nil { return nil, err } - httpReq, err := http.NewRequest(method, url, nil) + httpReq, err := http.NewRequestWithContext(ctx, method, url, nil) if err != nil { return nil, err } httpReq.ContentLength = contentLength - return &Request{bodyReader, httpReq}, nil + return &Request{body: bodyReader, Request: httpReq}, nil } // Logger interface allows to use other loggers than @@ -435,6 +463,9 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) { } // Don't retry if the error was due to TLS cert verification failure. + if notTrustedErrorRe.MatchString(v.Error()) { + return false, v + } if _, ok := v.Err.(x509.UnknownAuthorityError); ok { return false, v } @@ -455,7 +486,7 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) { // the server time to recover, as 500's are typically not permanent // errors and may relate to outages on the server side. This will catch // invalid response codes as well, like 0 and 999. - if resp.StatusCode == 0 || (resp.StatusCode >= 500 && resp.StatusCode != 501) { + if resp.StatusCode == 0 || (resp.StatusCode >= 500 && resp.StatusCode != http.StatusNotImplemented) { return true, fmt.Errorf("unexpected HTTP status %s", resp.Status) } @@ -555,13 +586,12 @@ func (c *Client) Do(req *Request) (*http.Response, error) { var resp *http.Response var attempt int var shouldRetry bool - var doErr, checkErr error + var doErr, respErr, checkErr error for i := 0; ; i++ { + doErr, respErr = nil, nil attempt++ - var code int // HTTP response code - // Always rewind the request body when non-nil. if req.body != nil { body, err := req.body() @@ -589,19 +619,24 @@ func (c *Client) Do(req *Request) (*http.Response, error) { // Attempt the request resp, doErr = c.HTTPClient.Do(req.Request) - if resp != nil { - code = resp.StatusCode - } // Check if we should continue with retries. shouldRetry, checkErr = c.CheckRetry(req.Context(), resp, doErr) + if !shouldRetry && doErr == nil && req.responseHandler != nil { + respErr = req.responseHandler(resp) + shouldRetry, checkErr = c.CheckRetry(req.Context(), resp, respErr) + } - if doErr != nil { + err := doErr + if respErr != nil { + err = respErr + } + if err != nil { switch v := logger.(type) { case LeveledLogger: - v.Error("request failed", "error", doErr, "method", req.Method, "url", req.URL) + v.Error("request failed", "error", err, "method", req.Method, "url", req.URL) case Logger: - v.Printf("[ERR] %s %s request failed: %v", req.Method, req.URL, doErr) + v.Printf("[ERR] %s %s request failed: %v", req.Method, req.URL, err) } } else { // Call this here to maintain the behavior of logging all requests, @@ -636,11 +671,11 @@ func (c *Client) Do(req *Request) (*http.Response, error) { } wait := c.Backoff(c.RetryWaitMin, c.RetryWaitMax, i, resp) - desc := fmt.Sprintf("%s %s", req.Method, req.URL) - if code > 0 { - desc = fmt.Sprintf("%s (status: %d)", desc, code) - } if logger != nil { + desc := fmt.Sprintf("%s %s", req.Method, req.URL) + if resp != nil { + desc = fmt.Sprintf("%s (status: %d)", desc, resp.StatusCode) + } switch v := logger.(type) { case LeveledLogger: v.Debug("retrying request", "request", desc, "timeout", wait, "remaining", remain) @@ -648,11 +683,13 @@ func (c *Client) Do(req *Request) (*http.Response, error) { v.Printf("[DEBUG] %s: retrying in %s (%d left)", desc, wait, remain) } } + timer := time.NewTimer(wait) select { case <-req.Context().Done(): + timer.Stop() c.HTTPClient.CloseIdleConnections() return nil, req.Context().Err() - case <-time.After(wait): + case <-timer.C: } // Make shallow copy of http Request so that we can modify its body @@ -662,15 +699,19 @@ func (c *Client) Do(req *Request) (*http.Response, error) { } // this is the closest we have to success criteria - if doErr == nil && checkErr == nil && !shouldRetry { + if doErr == nil && respErr == nil && checkErr == nil && !shouldRetry { return resp, nil } defer c.HTTPClient.CloseIdleConnections() - err := doErr + var err error if checkErr != nil { err = checkErr + } else if respErr != nil { + err = respErr + } else { + err = doErr } if c.ErrorHandler != nil {