From e16f958491054beac9d5fb81027c3b3744d92978 Mon Sep 17 00:00:00 2001 From: Justen Walker Date: Mon, 9 Aug 2021 13:20:47 -0400 Subject: [PATCH 01/59] support HTTPDate in Retry-After header According to [RFC7231](https://httpwg.org/specs/rfc7231.html#header.retry-after) the Retry-After header may be specified in both number of seconds OR an HTTPDate > The value of this field can be either an HTTP-date or a number of seconds to delay after the response is received. > > Retry-After = HTTP-date / delay-seconds > > A delay-seconds value is a non-negative decimal integer, representing time in seconds. > > delay-seconds = 1*DIGIT > > Two examples of its use are > > Retry-After: Fri, 31 Dec 1999 23:59:59 GMT > Retry-After: 120 This change adds support for parsing the date if the format of the header contains non-numeric characters --- client.go | 47 +++++++++++++++++++++++++++++++++++++++++++---- client_test.go | 28 ++++++++++++++++++++++++---- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/client.go b/client.go index adbdd92..457f930 100644 --- a/client.go +++ b/client.go @@ -69,6 +69,14 @@ 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 number of seconds to delay + // when parsing the Retry-After header + retryAfterSecondsRe = regexp.MustCompile(`^[0-9]+$`) + + // timeNow sets the function that returns the current time. + // This defaults to time.Now. Changes to this should only be done in tests. + timeNow = time.Now ) // ReaderFunc is the type of function that can be given natively to NewRequest @@ -472,10 +480,8 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) { 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 s, ok := resp.Header["Retry-After"]; ok { - if sleep, err := strconv.ParseInt(s[0], 10, 64); err == nil { - return time.Second * time.Duration(sleep) - } + if sleep, ok := parseRetryAfterHeader(resp.Header["Retry-After"]); ok { + return sleep } } } @@ -488,6 +494,39 @@ func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response) return sleep } +// parseRetryAfterHeader parses the Retry-After header and returns the +// delay duration according to the spec: https://httpwg.org/specs/rfc7231.html#header.retry-after +// +// Retry-After headers come in two flavors: Seconds or HTTP-Date +// +// Examples: +// * Retry-After: Fri, 31 Dec 1999 23:59:59 GMT +// * Retry-After: 120 +func parseRetryAfterHeader(headers []string) (time.Duration, bool) { + if len(headers) == 0 || headers[0] == "" { + return 0, false + } + header := headers[0] + // Retry-After: 120 + if retryAfterSecondsRe.MatchString(header) { + if sleep, err := strconv.ParseInt(header, 10, 64); err == nil { + return time.Second * time.Duration(sleep), true + } + return 0, false + } + + // Retry-After: Mon, 02 Jan 2006 15:04:05 MST + retryTime, err := time.Parse(time.RFC1123, header) + if err != nil { + return 0, false + } + if until := retryTime.Sub(timeNow()); until > 0 { + return until, true + } + // date is in the past + return 0, true +} + // LinearJitterBackoff provides a callback for Client.Backoff which will // perform linear backoff based on the attempt number and with jitter to // prevent a thundering herd. diff --git a/client_test.go b/client_test.go index 082b407..401b5ea 100644 --- a/client_test.go +++ b/client_test.go @@ -524,11 +524,31 @@ func TestClient_CheckRetry(t *testing.T) { } func TestClient_DefaultBackoff(t *testing.T) { - for _, code := range []int{http.StatusTooManyRequests, http.StatusServiceUnavailable} { - t.Run(fmt.Sprintf("http_%d", code), func(t *testing.T) { + timeNow = func() time.Time { + now, err := time.Parse(time.RFC1123, "Fri, 31 Dec 1999 23:59:57 GMT") + if err != nil { + panic(err) + } + return now + } + defer func() { + timeNow = time.Now + }() + tests := []struct { + name string + code int + retryHeader string + }{ + {"http_429_seconds", http.StatusTooManyRequests, "2"}, + {"http_429_date", http.StatusTooManyRequests, "Fri, 31 Dec 1999 23:59:59 GMT"}, + {"http_503_seconds", http.StatusServiceUnavailable, "2"}, + {"http_503_date", http.StatusTooManyRequests, "Fri, 31 Dec 1999 23:59:59 GMT"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Retry-After", "2") - http.Error(w, fmt.Sprintf("test_%d_body", code), code) + w.Header().Set("Retry-After", test.retryHeader) + http.Error(w, fmt.Sprintf("test_%d_body", test.code), test.code) })) defer ts.Close() From 0dc51cc2ee4ea10708cb9dad85141c7c3a83c37d Mon Sep 17 00:00:00 2001 From: Justen Walker Date: Fri, 13 Aug 2021 11:40:56 -0400 Subject: [PATCH 02/59] tests: fix DefaultBackoff case --- client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 401b5ea..063f258 100644 --- a/client_test.go +++ b/client_test.go @@ -542,7 +542,7 @@ func TestClient_DefaultBackoff(t *testing.T) { {"http_429_seconds", http.StatusTooManyRequests, "2"}, {"http_429_date", http.StatusTooManyRequests, "Fri, 31 Dec 1999 23:59:59 GMT"}, {"http_503_seconds", http.StatusServiceUnavailable, "2"}, - {"http_503_date", http.StatusTooManyRequests, "Fri, 31 Dec 1999 23:59:59 GMT"}, + {"http_503_date", http.StatusServiceUnavailable, "Fri, 31 Dec 1999 23:59:59 GMT"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { From 0ed3cd7540edad37b50599779c656c6340ffc7a3 Mon Sep 17 00:00:00 2001 From: Justen Walker Date: Fri, 13 Aug 2021 11:41:37 -0400 Subject: [PATCH 03/59] tests: helper to set static time.Now --- client_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client_test.go b/client_test.go index 063f258..e14e606 100644 --- a/client_test.go +++ b/client_test.go @@ -523,7 +523,7 @@ func TestClient_CheckRetry(t *testing.T) { } } -func TestClient_DefaultBackoff(t *testing.T) { +func testStaticTime(t *testing.T) { timeNow = func() time.Time { now, err := time.Parse(time.RFC1123, "Fri, 31 Dec 1999 23:59:57 GMT") if err != nil { @@ -531,9 +531,13 @@ func TestClient_DefaultBackoff(t *testing.T) { } return now } - defer func() { + t.Cleanup(func() { timeNow = time.Now - }() + }) +} + +func TestClient_DefaultBackoff(t *testing.T) { + testStaticTime(t) tests := []struct { name string code int From 012f144018e17693cf92c653a60f7060f262e20d Mon Sep 17 00:00:00 2001 From: Justen Walker Date: Fri, 13 Aug 2021 11:42:17 -0400 Subject: [PATCH 04/59] tests: add test cases for parseRetryAfterHeader --- client_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/client_test.go b/client_test.go index e14e606..9fdb758 100644 --- a/client_test.go +++ b/client_test.go @@ -536,6 +536,37 @@ func testStaticTime(t *testing.T) { }) } +func TestParseRetryAfterHeader(t *testing.T) { + testStaticTime(t) + tests := []struct { + name string + headers []string + sleep time.Duration + ok bool + }{ + {"seconds", []string{"2"}, time.Second * 2, true}, + {"date", []string{"Fri, 31 Dec 1999 23:59:59 GMT"}, time.Second * 2, true}, + {"past-date", []string{"Fri, 31 Dec 1999 23:59:00 GMT"}, 0, true}, + {"nil", nil, 0, false}, + {"two-headers", []string{"2", "3"}, time.Second * 2, true}, + {"empty", []string{""}, 0, false}, + {"negative", []string{"-2"}, 0, false}, + {"bad-date", []string{"Fri, 32 Dec 1999 23:59:59 GMT"}, 0, false}, + {"bad-date-format", []string{"badbadbad"}, 0, false}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + sleep, ok := parseRetryAfterHeader(test.headers) + if ok != test.ok { + t.Fatalf("expected ok=%t, got ok=%t", test.ok, ok) + } + if sleep != test.sleep { + t.Fatalf("expected sleep=%v, got sleep=%v", test.sleep, sleep) + } + }) + } +} + func TestClient_DefaultBackoff(t *testing.T) { testStaticTime(t) tests := []struct { From f2396f056078c3077dd837617ce8ba7ca7c4413d Mon Sep 17 00:00:00 2001 From: Justen Walker Date: Fri, 13 Aug 2021 11:42:47 -0400 Subject: [PATCH 05/59] remove regexp for parseRetryAfterHeader Removes dependency on regular expressions for detecting numeric header values vs date values --- client.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/client.go b/client.go index 457f930..3b104be 100644 --- a/client.go +++ b/client.go @@ -70,10 +70,6 @@ var ( // specifically so we resort to matching on the error string. schemeErrorRe = regexp.MustCompile(`unsupported protocol scheme`) - // A regular expression to match the number of seconds to delay - // when parsing the Retry-After header - retryAfterSecondsRe = regexp.MustCompile(`^[0-9]+$`) - // timeNow sets the function that returns the current time. // This defaults to time.Now. Changes to this should only be done in tests. timeNow = time.Now @@ -496,6 +492,8 @@ func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response) // parseRetryAfterHeader parses the Retry-After header and returns the // delay duration according to the spec: https://httpwg.org/specs/rfc7231.html#header.retry-after +// The bool returned will be true if the header was successfully parsed. +// Otherwise, the header was either not present, or was not parseable according to the spec. // // Retry-After headers come in two flavors: Seconds or HTTP-Date // @@ -508,14 +506,14 @@ func parseRetryAfterHeader(headers []string) (time.Duration, bool) { } header := headers[0] // Retry-After: 120 - if retryAfterSecondsRe.MatchString(header) { - if sleep, err := strconv.ParseInt(header, 10, 64); err == nil { - return time.Second * time.Duration(sleep), true + if sleep, err := strconv.ParseInt(header, 10, 64); err == nil { + if sleep < 0 { // a negative sleep doesn't make sense + return 0, false } - return 0, false + return time.Second * time.Duration(sleep), true } - // Retry-After: Mon, 02 Jan 2006 15:04:05 MST + // Retry-After: Fri, 31 Dec 1999 23:59:59 GMT retryTime, err := time.Parse(time.RFC1123, header) if err != nil { return 0, false From 676f2f06de023ec306b2c6f0fced21af2d192d9a Mon Sep 17 00:00:00 2001 From: Marko Kevac Date: Thu, 10 Feb 2022 15:28:59 +0300 Subject: [PATCH 06/59] Change URL from godoc to pkg.go.dev in README.md Change URL from godoc to pkg.go.dev in README.md as godoc is not supported anymore. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8943bec..145a62f 100644 --- a/README.md +++ b/README.md @@ -59,4 +59,4 @@ standardClient := retryClient.StandardClient() // *http.Client ``` For more usage and examples see the -[godoc](http://godoc.org/github.com/hashicorp/go-retryablehttp). +[pkg.go.dev](https://pkg.go.dev/github.com/hashicorp/go-retryablehttp). From af0a5a37a37732896cc3ad63503ee2915de1d266 Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Mon, 11 Apr 2022 15:12:36 -0500 Subject: [PATCH 07/59] fixing existing tests --- client.go | 8 ++++++++ client_test.go | 13 +++++++++---- roundtripper_test.go | 8 ++++---- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/client.go b/client.go index 7a073d8..fdc83a0 100644 --- a/client.go +++ b/client.go @@ -69,6 +69,11 @@ 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 @@ -445,6 +450,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 } diff --git a/client_test.go b/client_test.go index 901c86d..0c80e4c 100644 --- a/client_test.go +++ b/client_test.go @@ -167,13 +167,13 @@ func testClientDo(t *testing.T, body interface{}) { // Send the request var resp *http.Response doneCh := make(chan struct{}) + errCh := make(chan error, 1) go func() { defer close(doneCh) + defer close(errCh) var err error resp, err = client.Do(req) - if err != nil { - t.Fatalf("err: %v", err) - } + errCh <- err }() select { @@ -247,6 +247,11 @@ func testClientDo(t *testing.T, body interface{}) { if retryCount < 0 { t.Fatal("request log hook was not called") } + + err = <-errCh + if err != nil { + t.Fatalf("err: %v", err) + } } func TestClient_Do_fails(t *testing.T) { @@ -598,7 +603,7 @@ func TestClient_DefaultRetryPolicy_TLS(t *testing.T) { func TestClient_DefaultRetryPolicy_redirects(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, "/", 302) + http.Redirect(w, r, "/", http.StatusFound) })) defer ts.Close() diff --git a/roundtripper_test.go b/roundtripper_test.go index 93ff0c5..ed38f95 100644 --- a/roundtripper_test.go +++ b/roundtripper_test.go @@ -107,12 +107,12 @@ func TestRoundTripper_TransportFailureErrorHandling(t *testing.T) { expectedError := &url.Error{ Op: "Get", - URL: "http://this-url-does-not-exist-ed2fb.com/", + URL: "http://asdfsa.com/", Err: &net.OpError{ Op: "dial", Net: "tcp", Err: &net.DNSError{ - Name: "this-url-does-not-exist-ed2fb.com", + Name: "asdfsa.com", Err: "no such host", IsNotFound: true, }, @@ -121,10 +121,10 @@ func TestRoundTripper_TransportFailureErrorHandling(t *testing.T) { // Get the standard client and execute the request. client := retryClient.StandardClient() - _, err := client.Get("http://this-url-does-not-exist-ed2fb.com/") + _, err := client.Get("http://asdfsa.com/") // assert expectations - if !reflect.DeepEqual(normalizeError(err), expectedError) { + if !reflect.DeepEqual(expectedError, normalizeError(err)) { t.Fatalf("expected %q, got %q", expectedError, err) } } From 5e7d7d1b17bfb675b896e0b04b12801d5774fd29 Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Mon, 11 Apr 2022 16:00:15 -0500 Subject: [PATCH 08/59] add option to pass retryable response handler --- client.go | 25 ++++++++++++++-- client_test.go | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/client.go b/client.go index fdc83a0..7e4f976 100644 --- a/client.go +++ b/client.go @@ -553,6 +553,11 @@ func PassthroughErrorHandler(resp *http.Response, err error, _ int) (*http.Respo // Do wraps calling an HTTP method with retries. func (c *Client) Do(req *Request) (*http.Response, error) { + return c.DoWithResponseHandler(req, nil) +} + +// DoWithResponseHandler wraps calling an HTTP method plus a response handler with retries. +func (c *Client) DoWithResponseHandler(req *Request, handler func(*http.Response) (shouldRetry bool)) (*http.Response, error) { c.clientInit.Do(func() { if c.HTTPClient == nil { c.HTTPClient = cleanhttp.DefaultPooledClient() @@ -606,9 +611,6 @@ func (c *Client) Do(req *Request) (*http.Response, error) { // Attempt the request resp, doErr = c.HTTPClient.Do(req.Request) - // Check if we should continue with retries. - shouldRetry, checkErr = c.CheckRetry(req.Context(), resp, doErr) - if doErr != nil { switch v := logger.(type) { case LeveledLogger: @@ -632,6 +634,13 @@ func (c *Client) Do(req *Request) (*http.Response, error) { } } + // Check if we should continue with retries. + shouldRetry, checkErr = c.CheckRetry(req.Context(), resp, doErr) + + successSoFar := !shouldRetry && doErr == nil && checkErr == nil + if successSoFar && handler != nil { + shouldRetry = handler(resp) + } if !shouldRetry { break } @@ -739,6 +748,16 @@ func (c *Client) Get(url string) (*http.Response, error) { return c.Do(req) } +// GetWithResponseHandler is a helper for doing a GET request followed by a function on the response. +// The intention is for this to be used when errors in the response handling should also be retried. +func (c *Client) GetWithResponseHandler(url string, handler func(*http.Response) (shouldRetry bool)) (*http.Response, error) { + req, err := NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + return c.DoWithResponseHandler(req, handler) +} + // Head is a shortcut for doing a HEAD request without making a new client. func Head(url string) (*http.Response, error) { return defaultClient.Head(url) diff --git a/client_test.go b/client_test.go index 0c80e4c..04f3f5e 100644 --- a/client_test.go +++ b/client_test.go @@ -254,6 +254,83 @@ func testClientDo(t *testing.T, body interface{}) { } } +func TestClient_DoWithHandler(t *testing.T) { + // Create the client. Use short retry windows so we fail faster. + client := NewClient() + client.RetryWaitMin = 10 * time.Millisecond + client.RetryWaitMax = 10 * time.Millisecond + client.RetryMax = 2 + + var attempts int + client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) { + attempts++ + return DefaultRetryPolicy(context.TODO(), resp, err) + } + + // Mock server which always responds 200. + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer ts.Close() + + alternatingBool := false + tests := []struct { + name string + handler func(*http.Response) bool + expectedAttempts int + err string + }{ + { + name: "nil handler", + handler: nil, + expectedAttempts: 1, + }, + { + name: "handler never should retry", + handler: func(*http.Response) bool { return false }, + expectedAttempts: 1, + }, + { + name: "handler alternates should retry", + handler: func(*http.Response) bool { + alternatingBool = !alternatingBool + return alternatingBool + }, + expectedAttempts: 2, + }, + { + name: "handler always should retry", + handler: func(*http.Response) bool { return true }, + expectedAttempts: 3, + err: "giving up after 3 attempt(s)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + attempts = 0 + // Create the request + req, err := NewRequest("GET", ts.URL, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Send the request. + _, err = client.DoWithResponseHandler(req, tt.handler) + if err != nil && !strings.Contains(err.Error(), tt.err) { + t.Fatalf("error does not match expectation, expected: %s, got: %s", tt.err, err.Error()) + } + if err == nil && tt.err != "" { + t.Fatalf("no error, expected: %s", tt.err) + } + + if attempts != tt.expectedAttempts { + t.Fatalf("expected %d attempts, got %d attempts", tt.expectedAttempts, attempts) + } + }) + } +} + func TestClient_Do_fails(t *testing.T) { // Mock server which always responds 500. ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 5081362c3d96348ec5bcabb167462741ae43966b Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Mon, 11 Apr 2022 16:11:53 -0500 Subject: [PATCH 09/59] add new response handler method to README --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index 8943bec..89517ed 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,21 @@ 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, but then for later processing to fail, and sometimes this requires retrying the full request. E.g. a GET that returns a lot of data may "succeed" but the connection may drop while reading all of the data. + +In such a case, you can use `DoWithResponseHandler` (or `GetWithResponseHandler`) rather than `Do` (or `Get`). A toy example (which will retry the full request and succeed on the second attempt) is shown below: + +```go +c := retryablehttp.NewClient() +handlerShouldRetry := false +c.GetWithResponseHandler("/foo", func(*http.Response) bool { + handlerShouldRetry = !handlerShouldRetry + return handlerShouldRetry +}) +``` + ## Getting a stdlib `*http.Client` with retries It's possible to convert a `*retryablehttp.Client` directly to a `*http.Client`. From c9eb34c54dc8b8980cc2a7c19dd2e58dbf5aed20 Mon Sep 17 00:00:00 2001 From: Gavi Hirsch <100231082+gavriel-hc@users.noreply.github.com> Date: Tue, 12 Apr 2022 20:31:23 -0700 Subject: [PATCH 10/59] accepting README.md suggestion Co-authored-by: Ryan Uber --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 89517ed..3de1694 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ call would block and retry with exponential backoff. ## Retrying cases that fail after a seeming success -It's possible for a request to succeed, but then for later processing to fail, and sometimes this requires retrying the full request. E.g. a GET that returns a lot of data may "succeed" but the connection may drop while reading all of the data. +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. In such a case, you can use `DoWithResponseHandler` (or `GetWithResponseHandler`) rather than `Do` (or `Get`). A toy example (which will retry the full request and succeed on the second attempt) is shown below: From 78bf7347081a3d126b6868f983f560777af73e82 Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Wed, 13 Apr 2022 13:30:13 -0500 Subject: [PATCH 11/59] addressing comments --- client.go | 54 +++++++++++++++++---------------- client_test.go | 72 +++++++++++++++++++++++++++----------------- roundtripper_test.go | 6 ++-- 3 files changed, 76 insertions(+), 56 deletions(-) diff --git a/client.go b/client.go index 7e4f976..0e32376 100644 --- a/client.go +++ b/client.go @@ -79,6 +79,11 @@ var ( // ReaderFunc is the type of function that can be given natively to NewRequest type ReaderFunc func() (io.Reader, error) +// ResponseHandlingFunc 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 ResponseHandlingFunc 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 { @@ -91,6 +96,8 @@ type Request struct { // used to rewind the request data in between retries. body ReaderFunc + responseHandler ResponseHandlingFunc + // Embed an HTTP request directly. This makes a *Request act exactly // like an *http.Request so that all meta methods are supported. *http.Request @@ -100,11 +107,17 @@ type Request struct { // with its context changed to ctx. The provided ctx must be non-nil. func (r *Request) WithContext(ctx context.Context) *Request { return &Request{ - body: r.body, - Request: r.Request.WithContext(ctx), + body: r.body, + responseHandler: r.responseHandler, + Request: r.Request.WithContext(ctx), } } +// SetResponseHandler allows setting the response handler. +func (r *Request) SetResponseHandler(fn ResponseHandlingFunc) { + r.responseHandler = fn +} + // BodyBytes allows accessing the request body. It is an analogue to // http.Request's Body variable, but it returns a copy of the underlying data // rather than consuming it. @@ -259,7 +272,7 @@ 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. @@ -283,7 +296,7 @@ func NewRequestWithContext(ctx context.Context, method, url string, rawBody inte } httpReq.ContentLength = contentLength - return &Request{bodyReader, httpReq}, nil + return &Request{body: bodyReader, Request: httpReq}, nil } // Logger interface allows to use other loggers than @@ -553,11 +566,6 @@ func PassthroughErrorHandler(resp *http.Response, err error, _ int) (*http.Respo // Do wraps calling an HTTP method with retries. func (c *Client) Do(req *Request) (*http.Response, error) { - return c.DoWithResponseHandler(req, nil) -} - -// DoWithResponseHandler wraps calling an HTTP method plus a response handler with retries. -func (c *Client) DoWithResponseHandler(req *Request, handler func(*http.Response) (shouldRetry bool)) (*http.Response, error) { c.clientInit.Do(func() { if c.HTTPClient == nil { c.HTTPClient = cleanhttp.DefaultPooledClient() @@ -578,7 +586,7 @@ func (c *Client) DoWithResponseHandler(req *Request, handler func(*http.Response var resp *http.Response var attempt int var shouldRetry bool - var doErr, checkErr error + var doErr, respErr, checkErr error for i := 0; ; i++ { attempt++ @@ -636,11 +644,11 @@ func (c *Client) DoWithResponseHandler(req *Request, handler func(*http.Response // Check if we should continue with retries. shouldRetry, checkErr = c.CheckRetry(req.Context(), resp, doErr) - - successSoFar := !shouldRetry && doErr == nil && checkErr == nil - if successSoFar && handler != nil { - shouldRetry = handler(resp) + if !shouldRetry && doErr == nil && req.responseHandler != nil { + respErr = req.responseHandler(resp) + shouldRetry, checkErr = c.CheckRetry(req.Context(), resp, respErr) } + if !shouldRetry { break } @@ -686,15 +694,19 @@ func (c *Client) DoWithResponseHandler(req *Request, handler func(*http.Response } // 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 { @@ -748,16 +760,6 @@ func (c *Client) Get(url string) (*http.Response, error) { return c.Do(req) } -// GetWithResponseHandler is a helper for doing a GET request followed by a function on the response. -// The intention is for this to be used when errors in the response handling should also be retried. -func (c *Client) GetWithResponseHandler(url string, handler func(*http.Response) (shouldRetry bool)) (*http.Response, error) { - req, err := NewRequest("GET", url, nil) - if err != nil { - return nil, err - } - return c.DoWithResponseHandler(req, handler) -} - // Head is a shortcut for doing a HEAD request without making a new client. func Head(url string) (*http.Response, error) { return defaultClient.Head(url) diff --git a/client_test.go b/client_test.go index 04f3f5e..470e6e5 100644 --- a/client_test.go +++ b/client_test.go @@ -254,16 +254,19 @@ func testClientDo(t *testing.T, body interface{}) { } } -func TestClient_DoWithHandler(t *testing.T) { +func TestClient_Do_WithResponseHandler(t *testing.T) { // Create the client. Use short retry windows so we fail faster. client := NewClient() client.RetryWaitMin = 10 * time.Millisecond client.RetryWaitMax = 10 * time.Millisecond client.RetryMax = 2 - var attempts int + var checks int client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) { - attempts++ + checks++ + if err != nil && strings.Contains(err.Error(), "nonretryable") { + return false, nil + } return DefaultRetryPolicy(context.TODO(), resp, err) } @@ -273,50 +276,65 @@ func TestClient_DoWithHandler(t *testing.T) { })) defer ts.Close() - alternatingBool := false + var shouldSucceed bool tests := []struct { - name string - handler func(*http.Response) bool - expectedAttempts int - err string + name string + handler ResponseHandlingFunc + expectedChecks int // often 2x number of attempts since we check twice + err string }{ { - name: "nil handler", - handler: nil, - expectedAttempts: 1, + name: "nil handler", + handler: nil, + expectedChecks: 1, + }, + { + name: "handler always succeeds", + handler: func(*http.Response) error { + return nil + }, + expectedChecks: 2, }, { - name: "handler never should retry", - handler: func(*http.Response) bool { return false }, - expectedAttempts: 1, + name: "handler always fails in a retryable way", + handler: func(*http.Response) error { + return errors.New("retryable failure") + }, + expectedChecks: 6, }, { - name: "handler alternates should retry", - handler: func(*http.Response) bool { - alternatingBool = !alternatingBool - return alternatingBool + name: "handler always fails in a nonretryable way", + handler: func(*http.Response) error { + return errors.New("nonretryable failure") }, - expectedAttempts: 2, + expectedChecks: 2, }, { - name: "handler always should retry", - handler: func(*http.Response) bool { return true }, - expectedAttempts: 3, - err: "giving up after 3 attempt(s)", + name: "handler succeeds on second attempt", + handler: func(*http.Response) error { + if shouldSucceed { + return nil + } + shouldSucceed = true + return errors.New("retryable failure") + }, + expectedChecks: 4, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - attempts = 0 + checks = 0 + shouldSucceed = false // Create the request req, err := NewRequest("GET", ts.URL, nil) if err != nil { t.Fatalf("err: %v", err) } + req.SetResponseHandler(tt.handler) // Send the request. - _, err = client.DoWithResponseHandler(req, tt.handler) + _, err = client.Do(req) if err != nil && !strings.Contains(err.Error(), tt.err) { t.Fatalf("error does not match expectation, expected: %s, got: %s", tt.err, err.Error()) } @@ -324,8 +342,8 @@ func TestClient_DoWithHandler(t *testing.T) { t.Fatalf("no error, expected: %s", tt.err) } - if attempts != tt.expectedAttempts { - t.Fatalf("expected %d attempts, got %d attempts", tt.expectedAttempts, attempts) + if checks != tt.expectedChecks { + t.Fatalf("expected %d attempts, got %d attempts", tt.expectedChecks, checks) } }) } diff --git a/roundtripper_test.go b/roundtripper_test.go index ed38f95..ed818bb 100644 --- a/roundtripper_test.go +++ b/roundtripper_test.go @@ -107,12 +107,12 @@ func TestRoundTripper_TransportFailureErrorHandling(t *testing.T) { expectedError := &url.Error{ Op: "Get", - URL: "http://asdfsa.com/", + URL: "http://999.999.999.999:999/", Err: &net.OpError{ Op: "dial", Net: "tcp", Err: &net.DNSError{ - Name: "asdfsa.com", + Name: "999.999.999.999", Err: "no such host", IsNotFound: true, }, @@ -121,7 +121,7 @@ func TestRoundTripper_TransportFailureErrorHandling(t *testing.T) { // Get the standard client and execute the request. client := retryClient.StandardClient() - _, err := client.Get("http://asdfsa.com/") + _, err := client.Get("http://999.999.999.999:999/") // assert expectations if !reflect.DeepEqual(expectedError, normalizeError(err)) { From 8efde528a5308f94c00e7c5ae015311e00c6ec42 Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Wed, 13 Apr 2022 13:37:06 -0500 Subject: [PATCH 12/59] fix README toy example --- README.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 3de1694..09f5eaf 100644 --- a/README.md +++ b/README.md @@ -49,14 +49,18 @@ call would block and retry with exponential backoff. 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. -In such a case, you can use `DoWithResponseHandler` (or `GetWithResponseHandler`) rather than `Do` (or `Get`). A toy example (which will retry the full request and succeed on the second attempt) is shown below: +A toy example (which will retry the full request and succeed on the second attempt) is shown below: ```go c := retryablehttp.NewClient() -handlerShouldRetry := false -c.GetWithResponseHandler("/foo", func(*http.Response) bool { - handlerShouldRetry = !handlerShouldRetry - return handlerShouldRetry +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") }) ``` From 5bd1a6f34ca48c431c2c4d4b0f63a6b2a349287f Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Wed, 13 Apr 2022 16:47:44 -0500 Subject: [PATCH 13/59] addressing comments v2 --- client.go | 33 +++++++++++++++++++-------------- client_test.go | 2 +- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/client.go b/client.go index 0e32376..57116e9 100644 --- a/client.go +++ b/client.go @@ -79,10 +79,10 @@ var ( // ReaderFunc is the type of function that can be given natively to NewRequest type ReaderFunc func() (io.Reader, error) -// ResponseHandlingFunc is a type of function that takes in a Response, and does something with it. +// 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 ResponseHandlingFunc func(*http.Response) error +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. @@ -96,7 +96,7 @@ type Request struct { // used to rewind the request data in between retries. body ReaderFunc - responseHandler ResponseHandlingFunc + responseHandler ResponseHandlerFunc // Embed an HTTP request directly. This makes a *Request act exactly // like an *http.Request so that all meta methods are supported. @@ -114,7 +114,7 @@ func (r *Request) WithContext(ctx context.Context) *Request { } // SetResponseHandler allows setting the response handler. -func (r *Request) SetResponseHandler(fn ResponseHandlingFunc) { +func (r *Request) SetResponseHandler(fn ResponseHandlerFunc) { r.responseHandler = fn } @@ -589,6 +589,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) { var doErr, respErr, checkErr error for i := 0; ; i++ { + doErr, respErr = nil, nil attempt++ // Always rewind the request body when non-nil. @@ -619,12 +620,23 @@ func (c *Client) Do(req *Request) (*http.Response, error) { // Attempt the request resp, doErr = c.HTTPClient.Do(req.Request) - if doErr != nil { + // 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) + } + + 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, @@ -642,13 +654,6 @@ func (c *Client) Do(req *Request) (*http.Response, error) { } } - // 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 !shouldRetry { break } diff --git a/client_test.go b/client_test.go index 470e6e5..b3f8e6d 100644 --- a/client_test.go +++ b/client_test.go @@ -279,7 +279,7 @@ func TestClient_Do_WithResponseHandler(t *testing.T) { var shouldSucceed bool tests := []struct { name string - handler ResponseHandlingFunc + handler ResponseHandlerFunc expectedChecks int // often 2x number of attempts since we check twice err string }{ From fb2a4c5378816f7b54de9d6d8cce9c3ad2a7496d Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Mon, 25 Apr 2022 17:20:45 -0700 Subject: [PATCH 14/59] improve docs for response handlers --- README.md | 19 ------------------- client.go | 7 +++++-- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 09f5eaf..8943bec 100644 --- a/README.md +++ b/README.md @@ -45,25 +45,6 @@ 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/client.go b/client.go index 57116e9..e75b649 100644 --- a/client.go +++ b/client.go @@ -80,8 +80,11 @@ var ( 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. +// If an error is returned, the client's retry policy will be used to determine +// whether to retry the whole request (including this handler). +// NOTE: It only runs if the initial part of the request was successful. +// Make sure to check status codes! Even if the request was "successful" it may have a non-2xx status code. +// NOTE: If there is a response body, users should make sure to close it to avoid a memory leak. type ResponseHandlerFunc func(*http.Response) error // LenReader is an interface implemented by many in-memory io.Reader's. Used From b0b736350062d61f30b247294ce92c0ac888c06a Mon Sep 17 00:00:00 2001 From: Gavriel Hirsch Date: Tue, 10 May 2022 10:29:36 -0700 Subject: [PATCH 15/59] PR feedback --- client.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/client.go b/client.go index e75b649..f40d241 100644 --- a/client.go +++ b/client.go @@ -80,11 +80,15 @@ var ( type ReaderFunc func() (io.Reader, error) // ResponseHandlerFunc is a type of function that takes in a Response, and does something with it. -// If an error is returned, the client's retry policy will be used to determine +// The ResponseHandlerFunc is called when the HTTP client successfully receives a response and the +// CheckRetry function indicates that a retry of the base request is not necessary. +// If an error is returned from this function, the CheckRetry policy will be used to determine // whether to retry the whole request (including this handler). -// NOTE: It only runs if the initial part of the request was successful. -// Make sure to check status codes! Even if the request was "successful" it may have a non-2xx status code. -// NOTE: If there is a response body, users should make sure to close it to avoid a memory leak. +// +// Make sure to check status codes! Even if the request was completed it may have a non-2xx status code. +// +// The response body is not automatically closed. It must be closed either by the ResponseHandlerFunc or +// by the caller out-of-band. Failure to do so will result in a memory leak. type ResponseHandlerFunc func(*http.Response) error // LenReader is an interface implemented by many in-memory io.Reader's. Used From 0489443ca8467ad12f64de5488677076c97826bb Mon Sep 17 00:00:00 2001 From: Luke Young Date: Tue, 3 Jan 2023 07:54:25 -0800 Subject: [PATCH 16/59] Update go-cleanhttp to v0.5.2 (#178) --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 7cc02b7..d05df1b 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,7 @@ module github.com/hashicorp/go-retryablehttp require ( - github.com/hashicorp/go-cleanhttp v0.5.1 + github.com/hashicorp/go-cleanhttp v0.5.2 github.com/hashicorp/go-hclog v0.9.2 ) diff --git a/go.sum b/go.sum index 71afe56..593b856 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM= github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= +github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= +github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI= github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= From a53bec1353a135f9548327f10bb8459bfe96e2d8 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 3 Jan 2023 10:55:18 -0500 Subject: [PATCH 17/59] Run go mod tidy --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index 593b856..62d791e 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,5 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM= -github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI= From b94a2575994c07b6366c480e232a5d352f2155d6 Mon Sep 17 00:00:00 2001 From: "hashicorp-copywrite[bot]" <110428419+hashicorp-copywrite[bot]@users.noreply.github.com> Date: Wed, 4 Jan 2023 16:08:07 +0000 Subject: [PATCH 18/59] [COMPLIANCE] Update MPL-2.0 LICENSE --- LICENSE | 2 ++ 1 file changed, 2 insertions(+) diff --git a/LICENSE b/LICENSE index e87a115..f4f97ee 100644 --- a/LICENSE +++ b/LICENSE @@ -1,3 +1,5 @@ +Copyright (c) 2015 HashiCorp, Inc. + Mozilla Public License, version 2.0 1. Definitions From 8fcd8cdc364fe4432fcb969c80324d06e01be13e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Mon, 3 Apr 2023 16:49:54 +0200 Subject: [PATCH 19/59] Add more nil Logger tests (#183) --- client_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/client_test.go b/client_test.go index b3f8e6d..c1171b0 100644 --- a/client_test.go +++ b/client_test.go @@ -424,6 +424,12 @@ func TestClient_RequestLogHook(t *testing.T) { t.Run("RequestLogHook successfully called with nil Logger", func(t *testing.T) { testClientRequestLogHook(t, nil) }) + t.Run("RequestLogHook successfully called with nil typed Logger", func(t *testing.T) { + testClientRequestLogHook(t, Logger(nil)) + }) + t.Run("RequestLogHook successfully called with nil typed LeveledLogger", func(t *testing.T) { + testClientRequestLogHook(t, LeveledLogger(nil)) + }) } func testClientRequestLogHook(t *testing.T, logger interface{}) { @@ -485,6 +491,14 @@ func TestClient_ResponseLogHook(t *testing.T) { buf := new(bytes.Buffer) testClientResponseLogHook(t, nil, buf) }) + t.Run("ResponseLogHook successfully called with nil typed Logger", func(t *testing.T) { + buf := new(bytes.Buffer) + testClientResponseLogHook(t, Logger(nil), buf) + }) + t.Run("ResponseLogHook successfully called with nil typed LeveledLogger", func(t *testing.T) { + buf := new(bytes.Buffer) + testClientResponseLogHook(t, LeveledLogger(nil), buf) + }) } func testClientResponseLogHook(t *testing.T, l interface{}, buf *bytes.Buffer) { From eadc8800296ad1d6ea010811442421251bab27a1 Mon Sep 17 00:00:00 2001 From: "hashicorp-copywrite[bot]" <110428419+hashicorp-copywrite[bot]@users.noreply.github.com> Date: Mon, 3 Apr 2023 10:50:38 -0400 Subject: [PATCH 20/59] [COMPLIANCE] Add Copyright and License Headers (#180) Co-authored-by: hashicorp-copywrite[bot] <110428419+hashicorp-copywrite[bot]@users.noreply.github.com> --- .circleci/config.yml | 3 +++ client.go | 3 +++ client_test.go | 3 +++ roundtripper.go | 3 +++ roundtripper_test.go | 3 +++ 5 files changed, 15 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index c58e7ba..920c350 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,3 +1,6 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + version: 2.1 orbs: diff --git a/client.go b/client.go index f40d241..7f482af 100644 --- a/client.go +++ b/client.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + // Package retryablehttp provides a familiar HTTP client interface with // automatic retries and exponential backoff. It is a thin wrapper over the // standard net/http client library and exposes nearly the same public API. diff --git a/client_test.go b/client_test.go index c1171b0..5aaed05 100644 --- a/client_test.go +++ b/client_test.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package retryablehttp import ( diff --git a/roundtripper.go b/roundtripper.go index 8f3ee35..8c407ad 100644 --- a/roundtripper.go +++ b/roundtripper.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package retryablehttp import ( diff --git a/roundtripper_test.go b/roundtripper_test.go index ed818bb..dcb02df 100644 --- a/roundtripper_test.go +++ b/roundtripper_test.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package retryablehttp import ( From e8eb669553c6683750797c90588643d54733fc83 Mon Sep 17 00:00:00 2001 From: hc-github-team-es-release-engineering <82989873+hc-github-team-es-release-engineering@users.noreply.github.com> Date: Mon, 3 Apr 2023 14:28:14 -0700 Subject: [PATCH 21/59] Add workflow hashicorp/go-retryablehttp/go-retryablehttp --- .github/workflows/go-retryablehttp.yml | 39 ++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 .github/workflows/go-retryablehttp.yml diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml new file mode 100644 index 0000000..31c7fe6 --- /dev/null +++ b/.github/workflows/go-retryablehttp.yml @@ -0,0 +1,39 @@ +name: hashicorp/go-retryablehttp/go-retryablehttp +on: + push: + branches: + - master +jobs: + run-tests: + runs-on: ubuntu-latest + env: + TEST_RESULTS: "/tmp/test-results" + strategy: + matrix: + go-version: + - 1.14.2 + steps: + - uses: actions/checkout@v3.5.0 + - run: mkdir -p $TEST_RESULTS/go-retryablyhttp + - uses: actions/setup-go@v4.0.0 + with: + cache: true + - run: go mod download + - name: Run go format + run: |- + files=$(go fmt ./...) + if [ -n "$files" ]; then + echo "The following file(s) do not conform to go fmt:" + echo "$files" + exit 1 + fi + - name: Run tests with gotestsum + run: |- + PACKAGE_NAMES=$(go list ./...) + gotestsum --format=short-verbose --junitfile $TEST_RESULTS/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES + - uses: actions/upload-artifact@v3.1.1 + with: + path: "/tmp/test-results" + - uses: actions/upload-artifact@v3.1.1 + with: + path: "/tmp/test-results" From fe5cccf2acfa368ec224fa0743ea94937381302d Mon Sep 17 00:00:00 2001 From: claire-labry Date: Mon, 3 Apr 2023 21:28:44 +0000 Subject: [PATCH 22/59] SHA-pin all 3rd-party actions --- .github/workflows/go-retryablehttp.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index 31c7fe6..c467bd5 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -13,9 +13,9 @@ jobs: go-version: - 1.14.2 steps: - - uses: actions/checkout@v3.5.0 + - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - run: mkdir -p $TEST_RESULTS/go-retryablyhttp - - uses: actions/setup-go@v4.0.0 + - uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 with: cache: true - run: go mod download @@ -31,9 +31,9 @@ jobs: run: |- PACKAGE_NAMES=$(go list ./...) gotestsum --format=short-verbose --junitfile $TEST_RESULTS/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES - - uses: actions/upload-artifact@v3.1.1 + - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 with: path: "/tmp/test-results" - - uses: actions/upload-artifact@v3.1.1 + - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 with: path: "/tmp/test-results" From 9f67a2a83ee7af22931b27ced026f3c1e9fbfa71 Mon Sep 17 00:00:00 2001 From: claire-labry Date: Mon, 3 Apr 2023 21:28:44 +0000 Subject: [PATCH 23/59] Restrict workflow permissions --- .github/workflows/go-retryablehttp.yml | 54 +++++++++++++------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index c467bd5..ca03f14 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -2,7 +2,7 @@ name: hashicorp/go-retryablehttp/go-retryablehttp on: push: branches: - - master + - master jobs: run-tests: runs-on: ubuntu-latest @@ -11,29 +11,31 @@ jobs: strategy: matrix: go-version: - - 1.14.2 + - 1.14.2 steps: - - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - - run: mkdir -p $TEST_RESULTS/go-retryablyhttp - - uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 - with: - cache: true - - run: go mod download - - name: Run go format - run: |- - files=$(go fmt ./...) - if [ -n "$files" ]; then - echo "The following file(s) do not conform to go fmt:" - echo "$files" - exit 1 - fi - - name: Run tests with gotestsum - run: |- - PACKAGE_NAMES=$(go list ./...) - gotestsum --format=short-verbose --junitfile $TEST_RESULTS/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES - - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 - with: - path: "/tmp/test-results" - - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 - with: - path: "/tmp/test-results" + - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 + - run: mkdir -p $TEST_RESULTS/go-retryablyhttp + - uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 + with: + cache: true + - run: go mod download + - name: Run go format + run: |- + files=$(go fmt ./...) + if [ -n "$files" ]; then + echo "The following file(s) do not conform to go fmt:" + echo "$files" + exit 1 + fi + - name: Run tests with gotestsum + run: |- + PACKAGE_NAMES=$(go list ./...) + gotestsum --format=short-verbose --junitfile $TEST_RESULTS/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES + - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 + with: + path: "/tmp/test-results" + - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 + with: + path: "/tmp/test-results" +permissions: + contents: read From 0c83c6c736a72dc369a2fe9bcb795f4a25b65ed2 Mon Sep 17 00:00:00 2001 From: claire-labry Date: Mon, 3 Apr 2023 21:28:45 +0000 Subject: [PATCH 24/59] Add actionslint --- .github/workflows/actionlint.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .github/workflows/actionlint.yml diff --git a/.github/workflows/actionlint.yml b/.github/workflows/actionlint.yml new file mode 100644 index 0000000..be82ea5 --- /dev/null +++ b/.github/workflows/actionlint.yml @@ -0,0 +1,14 @@ +# If the repository is public, be sure to change to GitHub hosted runners +name: Lint GitHub Actions Workflows +on: + push: + pull_request: +permissions: + contents: read +jobs: + actionlint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 + - name: "Check workflow files" + uses: docker://docker.mirror.hashicorp.services/rhysd/actionlint:latest From 7a652ae1ca980e8dc5e7ed501760f14ecab18352 Mon Sep 17 00:00:00 2001 From: claire-labry Date: Mon, 3 Apr 2023 21:28:45 +0000 Subject: [PATCH 25/59] Add dependabot --- .github/dependabot.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..8a90cca --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,7 @@ +version: 2 + +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" \ No newline at end of file From e20c187a835fc44399da7e5e7f5df58a6d7d7da2 Mon Sep 17 00:00:00 2001 From: claire-labry Date: Mon, 3 Apr 2023 21:28:45 +0000 Subject: [PATCH 26/59] Add CODEOWNERS --- CODEOWNERS | 1 + 1 file changed, 1 insertion(+) create mode 100644 CODEOWNERS diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 0000000..7d1ad67 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1 @@ +# Submit a helpdesk ticket to add any team other than yours referenced in the CODEOWNERS file -- they must be added to collaborators and teams in the repository settings with maintainer privileges. Remove this file as soon as you have completed this From e39c7c4785a8c6fae715fb2ba799cf09a2b57297 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 Apr 2023 16:37:11 -0500 Subject: [PATCH 27/59] test --- .github/workflows/actionlint.yml | 13 +++++++++---- .github/workflows/go-retryablehttp.yml | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/actionlint.yml b/.github/workflows/actionlint.yml index be82ea5..006bb53 100644 --- a/.github/workflows/actionlint.yml +++ b/.github/workflows/actionlint.yml @@ -1,14 +1,19 @@ -# If the repository is public, be sure to change to GitHub hosted runners -name: Lint GitHub Actions Workflows +name: actionlint + on: push: - pull_request: + paths: + - .github/** + permissions: contents: read + jobs: actionlint: runs-on: ubuntu-latest steps: - uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 - - name: "Check workflow files" + - name: "Check GitHub workflow files" uses: docker://docker.mirror.hashicorp.services/rhysd/actionlint:latest + with: + args: -color -ignore SC2086 \ No newline at end of file diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index ca03f14..dae7b3b 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -1,8 +1,9 @@ -name: hashicorp/go-retryablehttp/go-retryablehttp +name: Build on: push: branches: - master + - convert-hashicorp-go-retryablehttp-to-actions-20230403-212814 jobs: run-tests: runs-on: ubuntu-latest From 3a6ed802f35dfb9352daabde0e520475f615316b Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 Apr 2023 16:40:02 -0500 Subject: [PATCH 28/59] add gotestsum --- .github/workflows/go-retryablehttp.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index dae7b3b..03d13bc 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -17,8 +17,15 @@ jobs: - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - run: mkdir -p $TEST_RESULTS/go-retryablyhttp - uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 + - name: Setup cache for go modules + uses: actions/cache@v3 with: - cache: true + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- - run: go mod download - name: Run go format run: |- @@ -28,7 +35,9 @@ jobs: echo "$files" exit 1 fi - - name: Run tests with gotestsum + - name: Install gotestsum + run: go get gotest.tools/gotestsum + - name: Run unit tests run: |- PACKAGE_NAMES=$(go list ./...) gotestsum --format=short-verbose --junitfile $TEST_RESULTS/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES From ff1a9677f9e9a6abad705b7f478c71555ac7fd7f Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 Apr 2023 16:42:25 -0500 Subject: [PATCH 29/59] remove unknown cmd --- .github/workflows/go-retryablehttp.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index 03d13bc..17b3886 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -40,7 +40,7 @@ jobs: - name: Run unit tests run: |- PACKAGE_NAMES=$(go list ./...) - gotestsum --format=short-verbose --junitfile $TEST_RESULTS/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES + gotestsum --junitfile "${TEST_RESULTS}"/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 with: path: "/tmp/test-results" From 1ceb4de605dd71c10e47d1f14c1ac57e3d291957 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 Apr 2023 16:52:07 -0500 Subject: [PATCH 30/59] add setup go --- .github/workflows/go-retryablehttp.yml | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index 17b3886..02a04bf 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -9,23 +9,19 @@ jobs: runs-on: ubuntu-latest env: TEST_RESULTS: "/tmp/test-results" - strategy: - matrix: - go-version: - - 1.14.2 steps: + - name: Setup go + uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 + with: + go-version: 1.14.2 - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - run: mkdir -p $TEST_RESULTS/go-retryablyhttp - - uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 - - name: Setup cache for go modules - uses: actions/cache@v3 + - name: restore_cache + uses: actions/cache@69d9d449aced6a2ede0bc19182fadc3a0a42d2b0 # v3.2.6 with: - path: | - ~/.cache/go-build - ~/go/pkg/mod - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- + key: go-mod-v1-{{ checksum "go.sum" }} + restore-keys: go-mod-v1-{{ checksum "go.sum" }} + path: "/go/pkg/mod" - run: go mod download - name: Run go format run: |- From d77790c0d1dd38d4566a66bd10de5f9b797cf787 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 Apr 2023 17:02:58 -0500 Subject: [PATCH 31/59] last minute clean up --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index 7d1ad67..f8389c9 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -# Submit a helpdesk ticket to add any team other than yours referenced in the CODEOWNERS file -- they must be added to collaborators and teams in the repository settings with maintainer privileges. Remove this file as soon as you have completed this +* @hashicorp/release-engineering \ No newline at end of file From 82ed15fd9d3034850812f184d79886bd2bb434c5 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 Apr 2023 17:04:10 -0500 Subject: [PATCH 32/59] remove CCI config --- .circleci/config.yml | 58 -------------------------------------------- 1 file changed, 58 deletions(-) delete mode 100644 .circleci/config.yml diff --git a/.circleci/config.yml b/.circleci/config.yml deleted file mode 100644 index 920c350..0000000 --- a/.circleci/config.yml +++ /dev/null @@ -1,58 +0,0 @@ -# Copyright (c) HashiCorp, Inc. -# SPDX-License-Identifier: MPL-2.0 - -version: 2.1 - -orbs: - go: circleci/go@1.1.1 - -references: - environments: - tmp: &TEST_RESULTS_PATH /tmp/test-results - -environment: &ENVIRONMENT - TEST_RESULTS: "/tmp/test-results" - -jobs: - run-tests: - executor: - name: go/default - tag: << parameters.go-version >> - parameters: - go-version: - type: string - environment: - TEST_RESULTS: *TEST_RESULTS_PATH - steps: - - checkout - - run: mkdir -p $TEST_RESULTS/go-retryablyhttp - - go/load-cache - - go/mod-download - - go/save-cache - - run: - name: Run go format - command: | - files=$(go fmt ./...) - if [ -n "$files" ]; then - echo "The following file(s) do not conform to go fmt:" - echo "$files" - exit 1 - fi - - run: - name: Run tests with gotestsum - command: | - PACKAGE_NAMES=$(go list ./...) - gotestsum --format=short-verbose --junitfile $TEST_RESULTS/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES - - store_test_results: - path: *TEST_RESULTS_PATH - - store_artifacts: - path: *TEST_RESULTS_PATH - -workflows: - go-retryablehttp: - jobs: - - run-tests: - matrix: - parameters: - go-version: ["1.14.2"] - name: test-go-<< matrix.go-version >> From 1165860abed97cc6edf1c263e745a2ed3b34a2fc Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 4 Apr 2023 12:33:03 -0500 Subject: [PATCH 33/59] PR feedback --- .github/dependabot.yml | 7 ++++++- .github/workflows/actionlint.yml | 4 ++-- .github/workflows/go-retryablehttp.yml | 6 ++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 8a90cca..0cb37bd 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,4 +4,9 @@ updates: - package-ecosystem: "github-actions" directory: "/" schedule: - interval: "daily" \ No newline at end of file + interval: "daily" + + - package-ecosystem: "gomod" + directory: "/" + schedule: + interval: "weekly" \ No newline at end of file diff --git a/.github/workflows/actionlint.yml b/.github/workflows/actionlint.yml index 006bb53..00a13c2 100644 --- a/.github/workflows/actionlint.yml +++ b/.github/workflows/actionlint.yml @@ -12,8 +12,8 @@ jobs: actionlint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 + - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - name: "Check GitHub workflow files" uses: docker://docker.mirror.hashicorp.services/rhysd/actionlint:latest with: - args: -color -ignore SC2086 \ No newline at end of file + args: -color \ No newline at end of file diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index 02a04bf..9751243 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -1,9 +1,6 @@ name: Build on: push: - branches: - - master - - convert-hashicorp-go-retryablehttp-to-actions-20230403-212814 jobs: run-tests: runs-on: ubuntu-latest @@ -15,7 +12,7 @@ jobs: with: go-version: 1.14.2 - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - - run: mkdir -p $TEST_RESULTS/go-retryablyhttp + - run: mkdir -p "$TEST_RESULTS"/go-retryablyhttp - name: restore_cache uses: actions/cache@69d9d449aced6a2ede0bc19182fadc3a0a42d2b0 # v3.2.6 with: @@ -36,6 +33,7 @@ jobs: - name: Run unit tests run: |- PACKAGE_NAMES=$(go list ./...) + # shellcheck disable=SC2086 # can't quote package list gotestsum --junitfile "${TEST_RESULTS}"/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 with: From 7bf089a067d53f8b743b6bfd8ab4038455539792 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Fri, 5 May 2023 13:15:17 +0100 Subject: [PATCH 34/59] Bugfix: fix mishandling of POST requests with no body, causing net/http to send request without a Content-Length --- client.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/client.go b/client.go index 7f482af..cad96bd 100644 --- a/client.go +++ b/client.go @@ -260,10 +260,17 @@ func getBodyReaderAndContentLength(rawBody interface{}) (ReaderFunc, int64, erro if err != nil { return nil, 0, err } - bodyReader = func() (io.Reader, error) { - return bytes.NewReader(buf), nil + if len(buf) == 0 { + bodyReader = func() (io.Reader, error) { + return http.NoBody, nil + } + contentLength = 0 + } else { + bodyReader = func() (io.Reader, error) { + return bytes.NewReader(buf), nil + } + contentLength = int64(len(buf)) } - contentLength = int64(len(buf)) // No body provided, nothing to do case nil: From ee107da66013a1d21239e9403012b09039703516 Mon Sep 17 00:00:00 2001 From: Mariano Asselborn Date: Mon, 15 May 2023 11:25:19 -0400 Subject: [PATCH 35/59] Start changelog file --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..03038d2 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,7 @@ +## 0.7.3 (15 May 2023) + +Initial release + +BUG FIXES + +- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP2 [GH-194] \ No newline at end of file From 3899851f2b38c8a02e3fce600e3539ec8a26e09a Mon Sep 17 00:00:00 2001 From: longshine Date: Fri, 19 May 2023 16:53:12 +0800 Subject: [PATCH 36/59] Avoid read all from bytes.Reader when get request body --- client.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/client.go b/client.go index 7f482af..c345af6 100644 --- a/client.go +++ b/client.go @@ -234,14 +234,12 @@ func getBodyReaderAndContentLength(rawBody interface{}) (ReaderFunc, int64, erro // deal with it seeking so want it to match here instead of the // io.ReadSeeker case. case *bytes.Reader: - buf, err := ioutil.ReadAll(body) - if err != nil { - return nil, 0, err - } + snapshot := *body bodyReader = func() (io.Reader, error) { - return bytes.NewReader(buf), nil + r := snapshot + return &r, nil } - contentLength = int64(len(buf)) + contentLength = int64(body.Len()) // Compat case case io.ReadSeeker: From 9fa45396ee56c4ed2eb2c58b5eafb66f5569baad Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 6 Jun 2023 19:29:50 +0100 Subject: [PATCH 37/59] fix changelog for v0.7.4 --- CHANGELOG.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03038d2..33686e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,9 @@ -## 0.7.3 (15 May 2023) - -Initial release +## 0.7.4 (Jun 6, 2023) BUG FIXES -- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP2 [GH-194] \ No newline at end of file +- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP/2 [GH-194] + +## 0.7.3 (May 15, 2023) + +Initial release From 9bb2062addb0a39dc43fbbd39ba3856c9df12543 Mon Sep 17 00:00:00 2001 From: Sebastian Rivera Date: Mon, 6 Nov 2023 11:21:52 -0500 Subject: [PATCH 38/59] Sets request's GetBody field on wrapper This commit addresses two key issues: - Upon receiving a temporary redirect, net/http will only preserve the request's body if GetBody() is defined. - Upon receiving a GOAWAY frame, the client will create a new connection. We define GetBody() in order to reuse the body sent in the last stream on the now terminated connection. --- client.go | 25 ++++++++++++++++----- client_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/client.go b/client.go index cad96bd..c9edbd0 100644 --- a/client.go +++ b/client.go @@ -160,6 +160,20 @@ func (r *Request) SetBody(rawBody interface{}) error { } r.body = bodyReader r.ContentLength = contentLength + if bodyReader != nil { + r.GetBody = func() (io.ReadCloser, error) { + body, err := bodyReader() + if err != nil { + return nil, err + } + if rc, ok := body.(io.ReadCloser); ok { + return rc, nil + } + return io.NopCloser(body), nil + } + } else { + r.GetBody = func() (io.ReadCloser, error) { return http.NoBody, nil } + } return nil } @@ -302,18 +316,19 @@ func NewRequest(method, url string, rawBody interface{}) (*Request, error) { // 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) + httpReq, err := http.NewRequestWithContext(ctx, method, url, nil) if err != nil { return nil, err } - httpReq, err := http.NewRequestWithContext(ctx, method, url, nil) - if err != nil { + req := &Request{ + Request: httpReq, + } + if err := req.SetBody(rawBody); err != nil { return nil, err } - httpReq.ContentLength = contentLength - return &Request{body: bodyReader, Request: httpReq}, nil + return req, nil } // Logger interface allows to use other loggers than diff --git a/client_test.go b/client_test.go index 5aaed05..c5e98a5 100644 --- a/client_test.go +++ b/client_test.go @@ -978,3 +978,62 @@ func TestClient_StandardClient(t *testing.T) { t.Fatalf("expected %v, got %v", client, v) } } + +func TestClient_RedirectWithBody(t *testing.T) { + var redirects int32 + // Mock server which always responds 200. + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/redirect": + w.Header().Set("Location", "/target") + w.WriteHeader(http.StatusTemporaryRedirect) + case "/target": + atomic.AddInt32(&redirects, 1) + w.WriteHeader(http.StatusCreated) + default: + t.Fatalf("bad uri: %s", r.RequestURI) + } + })) + defer ts.Close() + + client := NewClient() + client.RequestLogHook = func(logger Logger, req *http.Request, retryNumber int) { + if _, err := req.GetBody(); err != nil { + t.Fatalf("unexpected error with GetBody: %v", err) + } + } + // create a request with a body + req, err := NewRequest(http.MethodPost, ts.URL+"/redirect", strings.NewReader(`{"foo":"bar"}`)) + if err != nil { + t.Fatalf("err: %v", err) + } + + resp, err := client.Do(req) + if err != nil { + t.Fatalf("err: %v", err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + t.Fatalf("expected status code 201, got: %d", resp.StatusCode) + } + + // now one without a body + if err := req.SetBody(nil); err != nil { + t.Fatalf("err: %v", err) + } + + resp, err = client.Do(req) + if err != nil { + t.Fatalf("err: %v", err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + t.Fatalf("expected status code 201, got: %d", resp.StatusCode) + } + + if atomic.LoadInt32(&redirects) != 2 { + t.Fatalf("Expected the client to be redirected 2 times, got: %d", atomic.LoadInt32(&redirects)) + } +} From f95735f4e6993de5149791308b471d581ed12f78 Mon Sep 17 00:00:00 2001 From: Sebastian Rivera Date: Mon, 6 Nov 2023 11:37:58 -0500 Subject: [PATCH 39/59] Update workflow to use go v1.18 --- .github/workflows/go-retryablehttp.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index 9751243..dad25d8 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -10,7 +10,7 @@ jobs: - name: Setup go uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 with: - go-version: 1.14.2 + go-version: 1.18 - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - run: mkdir -p "$TEST_RESULTS"/go-retryablyhttp - name: restore_cache @@ -20,6 +20,7 @@ jobs: restore-keys: go-mod-v1-{{ checksum "go.sum" }} path: "/go/pkg/mod" - run: go mod download + - run: go mod tidy - name: Run go format run: |- files=$(go fmt ./...) @@ -29,7 +30,7 @@ jobs: exit 1 fi - name: Install gotestsum - run: go get gotest.tools/gotestsum + run: go install gotest.tools/gotestsum@latest - name: Run unit tests run: |- PACKAGE_NAMES=$(go list ./...) From 6c37e02416644e1c333a496542642cbd2299c820 Mon Sep 17 00:00:00 2001 From: Sebastian Rivera Date: Wed, 8 Nov 2023 11:23:51 -0500 Subject: [PATCH 40/59] v0.7.5 changelog update --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33686e4..7a17b9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.7.5 (Nov 8, 2023) + +BUG FIXES + +- client: fixes an issue where the request body is not preserved on temporary redirects or re-established HTTP/2 connections [GH-207] + ## 0.7.4 (Jun 6, 2023) BUG FIXES From a004c57d62e13f820c7ab31d06dc7794637cb147 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 1 Dec 2023 17:04:13 -0500 Subject: [PATCH 41/59] Don't retry after "invalid request header" error. --- client.go | 10 ++++++++++ client_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/client.go b/client.go index c9edbd0..0ef1eda 100644 --- a/client.go +++ b/client.go @@ -73,6 +73,11 @@ var ( // 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 a + // request header or value is invalid. This error isn't typed + // specifically so we resort to matching on the error string. + invalidHeaderErrorRe = regexp.MustCompile(`invalid header`) + // 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. @@ -494,6 +499,11 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) { return false, v } + // Don't retry if the error was due to an invalid header. + if invalidHeaderErrorRe.MatchString(v.Error()) { + return false, v + } + // Don't retry if the error was due to TLS cert verification failure. if notTrustedErrorRe.MatchString(v.Error()) { return false, v diff --git a/client_test.go b/client_test.go index c5e98a5..85eb1d7 100644 --- a/client_test.go +++ b/client_test.go @@ -758,6 +758,60 @@ func TestClient_DefaultRetryPolicy_invalidscheme(t *testing.T) { } } +func TestClient_DefaultRetryPolicy_invalidheadername(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer ts.Close() + + attempts := 0 + client := NewClient() + client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) { + attempts++ + return DefaultRetryPolicy(context.TODO(), resp, err) + } + + req, err := http.NewRequest(http.MethodGet, ts.URL, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Header.Set("Header-Name-\033", "header value") + _, err = client.StandardClient().Do(req) + if err == nil { + t.Fatalf("expected header error, got nil") + } + if attempts != 1 { + t.Fatalf("expected 1 attempt, got %d", attempts) + } +} + +func TestClient_DefaultRetryPolicy_invalidheadervalue(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer ts.Close() + + attempts := 0 + client := NewClient() + client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) { + attempts++ + return DefaultRetryPolicy(context.TODO(), resp, err) + } + + req, err := http.NewRequest(http.MethodGet, ts.URL, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Header.Set("Header-Name", "bad header value \033") + _, err = client.StandardClient().Do(req) + if err == nil { + t.Fatalf("expected header value error, got nil") + } + if attempts != 1 { + t.Fatalf("expected 1 attempt, got %d", attempts) + } +} + func TestClient_CheckRetryStop(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "test_500_body", http.StatusInternalServerError) From a1a8ab82eb1779b8e09b2d6d2605bbf6fd059a17 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 1 Dec 2023 17:05:21 -0500 Subject: [PATCH 42/59] Fix certificate error detection on Go 1.20 and 1.21. --- cert_error_go119.go | 14 ++++++++++++++ cert_error_go120.go | 14 ++++++++++++++ client.go | 3 +-- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 cert_error_go119.go create mode 100644 cert_error_go120.go diff --git a/cert_error_go119.go b/cert_error_go119.go new file mode 100644 index 0000000..b2b27e8 --- /dev/null +++ b/cert_error_go119.go @@ -0,0 +1,14 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build !go1.20 +// +build !go1.20 + +package retryablehttp + +import "crypto/x509" + +func isCertError(err error) bool { + _, ok := err.(x509.UnknownAuthorityError) + return ok +} diff --git a/cert_error_go120.go b/cert_error_go120.go new file mode 100644 index 0000000..a3cd315 --- /dev/null +++ b/cert_error_go120.go @@ -0,0 +1,14 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build go1.20 +// +build go1.20 + +package retryablehttp + +import "crypto/tls" + +func isCertError(err error) bool { + _, ok := err.(*tls.CertificateVerificationError) + return ok +} diff --git a/client.go b/client.go index 0ef1eda..bcfd19e 100644 --- a/client.go +++ b/client.go @@ -27,7 +27,6 @@ package retryablehttp import ( "bytes" "context" - "crypto/x509" "fmt" "io" "io/ioutil" @@ -508,7 +507,7 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) { if notTrustedErrorRe.MatchString(v.Error()) { return false, v } - if _, ok := v.Err.(x509.UnknownAuthorityError); ok { + if isCertError(v.Err) { return false, v } } From c872c98a89ad5ee0c84b2c3250716901fd81aecf Mon Sep 17 00:00:00 2001 From: Michal Wojcik Date: Mon, 5 Feb 2024 14:58:56 +0100 Subject: [PATCH 43/59] DXE-3480 Re-sign request on retry --- client.go | 18 ++++++++++++++++++ go.mod | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/client.go b/client.go index c9edbd0..0a7c9dc 100644 --- a/client.go +++ b/client.go @@ -393,6 +393,9 @@ type Backoff func(min, max time.Duration, attemptNum int, resp *http.Response) t // attempted. If overriding this, be sure to close the body if needed. type ErrorHandler func(resp *http.Response, err error, numTries int) (*http.Response, error) +// Postprocess is called before retry operation. It can be used to for example re-sign the request +type Postprocess func(req *http.Request) error + // Client is used to make HTTP requests. It adds additional functionality // like automatic retries to tolerate minor outages. type Client struct { @@ -421,6 +424,9 @@ type Client struct { // ErrorHandler specifies the custom error handler to use, if any ErrorHandler ErrorHandler + // Postprocess can prepare the request for retry operation, for example re-sign it + Postprocess Postprocess + loggerInit sync.Once clientInit sync.Once } @@ -435,6 +441,7 @@ func NewClient() *Client { RetryMax: defaultRetryMax, CheckRetry: DefaultRetryPolicy, Backoff: DefaultBackoff, + Postprocess: DefaultPostprocess, } } @@ -551,6 +558,12 @@ func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response) return sleep } +// DefaultPostprocess is performing noop during postprocess +func DefaultPostprocess(_ *http.Request) error { + // noop + return nil +} + // LinearJitterBackoff provides a callback for Client.Backoff which will // perform linear backoff based on the attempt number and with jitter to // prevent a thundering herd. @@ -728,6 +741,11 @@ func (c *Client) Do(req *Request) (*http.Response, error) { // without racing against the closeBody call in persistConn.writeLoop. httpreq := *req.Request req.Request = &httpreq + + if err := c.Postprocess(req.Request); err != nil { + checkErr = err + break + } } // this is the closest we have to success criteria diff --git a/go.mod b/go.mod index d05df1b..9257b1e 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/hashicorp/go-retryablehttp +module github.com/mgwoj/go-retryablehttp require ( github.com/hashicorp/go-cleanhttp v0.5.2 From b2e8d9b4aee31e67ab6505f4633082ba4dd08075 Mon Sep 17 00:00:00 2001 From: Michal Wojcik Date: Fri, 16 Feb 2024 13:28:27 +0100 Subject: [PATCH 44/59] DXE-3480 Configurable retry --- client.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/client.go b/client.go index 0a7c9dc..c8fe8f4 100644 --- a/client.go +++ b/client.go @@ -393,8 +393,8 @@ type Backoff func(min, max time.Duration, attemptNum int, resp *http.Response) t // attempted. If overriding this, be sure to close the body if needed. type ErrorHandler func(resp *http.Response, err error, numTries int) (*http.Response, error) -// Postprocess is called before retry operation. It can be used to for example re-sign the request -type Postprocess func(req *http.Request) error +// PrepareRetry is called before retry operation. It can be used for example to re-sign the request +type PrepareRetry func(req *http.Request) error // Client is used to make HTTP requests. It adds additional functionality // like automatic retries to tolerate minor outages. @@ -424,8 +424,8 @@ type Client struct { // ErrorHandler specifies the custom error handler to use, if any ErrorHandler ErrorHandler - // Postprocess can prepare the request for retry operation, for example re-sign it - Postprocess Postprocess + // PrepareRetry can prepare the request for retry operation, for example re-sign it + PrepareRetry PrepareRetry loggerInit sync.Once clientInit sync.Once @@ -441,7 +441,7 @@ func NewClient() *Client { RetryMax: defaultRetryMax, CheckRetry: DefaultRetryPolicy, Backoff: DefaultBackoff, - Postprocess: DefaultPostprocess, + PrepareRetry: DefaultPrepareRetry, } } @@ -558,8 +558,8 @@ func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response) return sleep } -// DefaultPostprocess is performing noop during postprocess -func DefaultPostprocess(_ *http.Request) error { +// DefaultPrepareRetry is performing noop during prepare retry +func DefaultPrepareRetry(_ *http.Request) error { // noop return nil } @@ -631,10 +631,10 @@ func (c *Client) Do(req *Request) (*http.Response, error) { var resp *http.Response var attempt int var shouldRetry bool - var doErr, respErr, checkErr error + var doErr, respErr, checkErr, prepareErr error for i := 0; ; i++ { - doErr, respErr = nil, nil + doErr, respErr, prepareErr = nil, nil, nil attempt++ // Always rewind the request body when non-nil. @@ -742,21 +742,23 @@ func (c *Client) Do(req *Request) (*http.Response, error) { httpreq := *req.Request req.Request = &httpreq - if err := c.Postprocess(req.Request); err != nil { - checkErr = err + if err := c.PrepareRetry(req.Request); err != nil { + prepareErr = err break } } // this is the closest we have to success criteria - if doErr == nil && respErr == nil && checkErr == nil && !shouldRetry { + if doErr == nil && respErr == nil && checkErr == nil && prepareErr == nil && !shouldRetry { return resp, nil } defer c.HTTPClient.CloseIdleConnections() var err error - if checkErr != nil { + if prepareErr != nil { + err = prepareErr + } else if checkErr != nil { err = checkErr } else if respErr != nil { err = respErr From 25093acfa1aeefdda6357469f760700bb0a7a6c6 Mon Sep 17 00:00:00 2001 From: Michal Wojcik Date: Fri, 8 Mar 2024 11:32:52 +0100 Subject: [PATCH 45/59] DXE-3480 Unit test --- client_test.go | 123 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/client_test.go b/client_test.go index c5e98a5..a751d3f 100644 --- a/client_test.go +++ b/client_test.go @@ -15,6 +15,7 @@ import ( "net/http/httptest" "net/http/httputil" "net/url" + "strconv" "strings" "sync/atomic" "testing" @@ -352,6 +353,128 @@ func TestClient_Do_WithResponseHandler(t *testing.T) { } } +func TestClient_Do_WithPrepareRetry(t *testing.T) { + // Create the client. Use short retry windows so we fail faster. + client := NewClient() + client.RetryWaitMin = 10 * time.Millisecond + client.RetryWaitMax = 10 * time.Millisecond + client.RetryMax = 2 + + var checks int + client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) { + checks++ + if err != nil && strings.Contains(err.Error(), "nonretryable") { + return false, nil + } + return DefaultRetryPolicy(context.TODO(), resp, err) + } + + var prepareChecks int + client.PrepareRetry = func(req *http.Request) error { + prepareChecks++ + req.Header.Set("foo", strconv.Itoa(prepareChecks)) + return nil + } + + // Mock server which always responds 200. + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer ts.Close() + + var shouldSucceed bool + tests := []struct { + name string + handler ResponseHandlerFunc + expectedChecks int // often 2x number of attempts since we check twice + expectedPrepareChecks int + err string + }{ + { + name: "nil handler", + handler: nil, + expectedChecks: 1, + expectedPrepareChecks: 0, + }, + { + name: "handler always succeeds", + handler: func(*http.Response) error { + return nil + }, + expectedChecks: 2, + expectedPrepareChecks: 0, + }, + { + name: "handler always fails in a retryable way", + handler: func(*http.Response) error { + return errors.New("retryable failure") + }, + expectedChecks: 6, + expectedPrepareChecks: 2, + }, + { + name: "handler always fails in a nonretryable way", + handler: func(*http.Response) error { + return errors.New("nonretryable failure") + }, + expectedChecks: 2, + expectedPrepareChecks: 0, + }, + { + name: "handler succeeds on second attempt", + handler: func(*http.Response) error { + if shouldSucceed { + return nil + } + shouldSucceed = true + return errors.New("retryable failure") + }, + expectedChecks: 4, + expectedPrepareChecks: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + checks = 0 + prepareChecks = 0 + shouldSucceed = false + // Create the request + req, err := NewRequest("GET", ts.URL, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.SetResponseHandler(tt.handler) + + // Send the request. + _, err = client.Do(req) + if err != nil && !strings.Contains(err.Error(), tt.err) { + t.Fatalf("error does not match expectation, expected: %s, got: %s", tt.err, err.Error()) + } + if err == nil && tt.err != "" { + t.Fatalf("no error, expected: %s", tt.err) + } + + if checks != tt.expectedChecks { + t.Fatalf("expected %d attempts, got %d attempts", tt.expectedChecks, checks) + } + + if prepareChecks != tt.expectedPrepareChecks { + t.Fatalf("expected %d attempts of prepare check, got %d attempts", tt.expectedPrepareChecks, prepareChecks) + } + header := req.Request.Header.Get("foo") + if tt.expectedPrepareChecks == 0 && header != "" { + t.Fatalf("expected no changes to request header 'foo', but got '%s'", header) + } + expectedHeader := strconv.Itoa(tt.expectedPrepareChecks) + if tt.expectedPrepareChecks != 0 && header != expectedHeader { + t.Fatalf("expected changes in request header 'foo' '%s', but got '%s'", expectedHeader, header) + } + + }) + } +} + func TestClient_Do_fails(t *testing.T) { // Mock server which always responds 500. ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From c5939ed658c9297bbff723e81b1d3712cf668484 Mon Sep 17 00:00:00 2001 From: Michal Wojcik <32574975+mgwoj@users.noreply.github.com> Date: Fri, 8 Mar 2024 16:44:31 +0100 Subject: [PATCH 46/59] Update go.mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 9257b1e..d05df1b 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/mgwoj/go-retryablehttp +module github.com/hashicorp/go-retryablehttp require ( github.com/hashicorp/go-cleanhttp v0.5.2 From edadfe12db0d0c8b117a497a283a199cdb6627f1 Mon Sep 17 00:00:00 2001 From: Michal Wojcik Date: Fri, 26 Apr 2024 09:04:04 +0200 Subject: [PATCH 47/59] DXE-3480 Simplify code for PrepareRetry --- client.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/client.go b/client.go index c8fe8f4..9b09c21 100644 --- a/client.go +++ b/client.go @@ -441,7 +441,6 @@ func NewClient() *Client { RetryMax: defaultRetryMax, CheckRetry: DefaultRetryPolicy, Backoff: DefaultBackoff, - PrepareRetry: DefaultPrepareRetry, } } @@ -558,12 +557,6 @@ func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response) return sleep } -// DefaultPrepareRetry is performing noop during prepare retry -func DefaultPrepareRetry(_ *http.Request) error { - // noop - return nil -} - // LinearJitterBackoff provides a callback for Client.Backoff which will // perform linear backoff based on the attempt number and with jitter to // prevent a thundering herd. @@ -742,9 +735,11 @@ func (c *Client) Do(req *Request) (*http.Response, error) { httpreq := *req.Request req.Request = &httpreq - if err := c.PrepareRetry(req.Request); err != nil { - prepareErr = err - break + if c.PrepareRetry != nil { + if err := c.PrepareRetry(req.Request); err != nil { + prepareErr = err + break + } } } From 40a3a3381372fec325bd2570e5688d3d7198cd61 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 8 May 2024 21:02:53 +0100 Subject: [PATCH 48/59] Change of maintainer --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index f8389c9..d6dd78a 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @hashicorp/release-engineering \ No newline at end of file +* @hashicorp/go-retryablehttp-maintainers From 1661d7b8da5e51f0170fca2b3b9f143ace31d43b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 9 May 2024 13:57:24 +0000 Subject: [PATCH 49/59] Bump actions/cache from 3.2.6 to 4.0.2 Bumps [actions/cache](https://github.com/actions/cache) from 3.2.6 to 4.0.2. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](https://github.com/actions/cache/compare/69d9d449aced6a2ede0bc19182fadc3a0a42d2b0...0c45773b623bea8c8e75f6c82b208c3cf94ea4f9) --- updated-dependencies: - dependency-name: actions/cache dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/go-retryablehttp.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index dad25d8..ee0ef06 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -14,7 +14,7 @@ jobs: - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - run: mkdir -p "$TEST_RESULTS"/go-retryablyhttp - name: restore_cache - uses: actions/cache@69d9d449aced6a2ede0bc19182fadc3a0a42d2b0 # v3.2.6 + uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2 with: key: go-mod-v1-{{ checksum "go.sum" }} restore-keys: go-mod-v1-{{ checksum "go.sum" }} From 003233786801a56fccb9d5deb110c400e1decca3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 9 May 2024 13:58:30 +0000 Subject: [PATCH 50/59] Bump actions/setup-go from 4.0.0 to 5.0.1 Bumps [actions/setup-go](https://github.com/actions/setup-go) from 4.0.0 to 5.0.1. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](https://github.com/actions/setup-go/compare/4d34df0c2316fe8122ab82dc22947d607c0c91f9...cdcb36043654635271a94b9a6d1392de5bb323a7) --- updated-dependencies: - dependency-name: actions/setup-go dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/go-retryablehttp.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index dad25d8..de02e99 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -8,7 +8,7 @@ jobs: TEST_RESULTS: "/tmp/test-results" steps: - name: Setup go - uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version: 1.18 - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 From 4c2e07b94906d2fed1a53ab4460de157649ce900 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 9 May 2024 14:10:05 +0000 Subject: [PATCH 51/59] Bump actions/checkout from 3.5.0 to 4.1.5 Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.0 to 4.1.5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/8f4b7f84864484a7bf31766abe9204da3cbe65b3...44c2b7a8a4ea60a981eaca3cf939b5f4305c123b) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/actionlint.yml | 2 +- .github/workflows/go-retryablehttp.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/actionlint.yml b/.github/workflows/actionlint.yml index 00a13c2..e0d2f4f 100644 --- a/.github/workflows/actionlint.yml +++ b/.github/workflows/actionlint.yml @@ -12,7 +12,7 @@ jobs: actionlint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 + - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 - name: "Check GitHub workflow files" uses: docker://docker.mirror.hashicorp.services/rhysd/actionlint:latest with: diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index dad25d8..1bdf390 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -11,7 +11,7 @@ jobs: uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 with: go-version: 1.18 - - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 + - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 - run: mkdir -p "$TEST_RESULTS"/go-retryablyhttp - name: restore_cache uses: actions/cache@69d9d449aced6a2ede0bc19182fadc3a0a42d2b0 # v3.2.6 From 6d0f2e8894525409ace25b36d67a668e7249d052 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 9 May 2024 16:11:26 +0100 Subject: [PATCH 52/59] Changelog updates for #138, #197, #216 --- CHANGELOG.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a17b9f..e16887d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,22 @@ +## 0.7.6 (Unreleased) + +ENHANCEMENTS: + +- client: support a `RetryPrepare` function for modifying the request before retrying (#216) +- client: support HTTP-date values for `Retry-After` header value (#138) +- client: avoid reading entire body when the body is a `*bytes.Reader` (#197) + ## 0.7.5 (Nov 8, 2023) -BUG FIXES +BUG FIXES: -- client: fixes an issue where the request body is not preserved on temporary redirects or re-established HTTP/2 connections [GH-207] +- client: fixes an issue where the request body is not preserved on temporary redirects or re-established HTTP/2 connections (#207) ## 0.7.4 (Jun 6, 2023) -BUG FIXES +BUG FIXES: -- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP/2 [GH-194] +- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP/2 (#194) ## 0.7.3 (May 15, 2023) From 834d13d328ba60d75384836528bf4764a4ec3a06 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 9 May 2024 16:44:54 +0100 Subject: [PATCH 53/59] update go version --- .go-version | 1 + go.mod | 11 +++++++++-- go.sum | 34 ++++++++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 .go-version diff --git a/.go-version b/.go-version new file mode 100644 index 0000000..6fee2fe --- /dev/null +++ b/.go-version @@ -0,0 +1 @@ +1.22.2 diff --git a/go.mod b/go.mod index d05df1b..12c7872 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,14 @@ module github.com/hashicorp/go-retryablehttp require ( github.com/hashicorp/go-cleanhttp v0.5.2 - github.com/hashicorp/go-hclog v0.9.2 + github.com/hashicorp/go-hclog v1.6.3 ) -go 1.13 +require ( + github.com/fatih/color v1.16.0 // indirect + github.com/mattn/go-colorable v0.1.13 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + golang.org/x/sys v0.20.0 // indirect +) + +go 1.19 diff --git a/go.sum b/go.sum index 62d791e..a5da2ce 100644 --- a/go.sum +++ b/go.sum @@ -1,10 +1,36 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= +github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= +github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= -github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI= -github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ= +github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k= +github.com/hashicorp/go-hclog v1.6.3/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= +github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= +github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= +github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= +github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= +github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w= -github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.2 h1:4jaiDzPyXQvSd7D0EjG45355tLlV3VOECpq10pLC+8s= +github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= +golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= +golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From e82c7007a33a9963a7a965e814c6c3944ca09390 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 9 May 2024 16:45:32 +0100 Subject: [PATCH 54/59] GHA workflows for unit tests --- .github/workflows/go-retryablehttp.yml | 46 ----------------------- .github/workflows/pr-gofmt.yaml | 23 ++++++++++++ .github/workflows/pr-unit-tests-1.19.yaml | 17 +++++++++ .github/workflows/pr-unit-tests-1.20.yaml | 17 +++++++++ Makefile | 2 +- 5 files changed, 58 insertions(+), 47 deletions(-) delete mode 100644 .github/workflows/go-retryablehttp.yml create mode 100644 .github/workflows/pr-gofmt.yaml create mode 100644 .github/workflows/pr-unit-tests-1.19.yaml create mode 100644 .github/workflows/pr-unit-tests-1.20.yaml diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml deleted file mode 100644 index dd3673e..0000000 --- a/.github/workflows/go-retryablehttp.yml +++ /dev/null @@ -1,46 +0,0 @@ -name: Build -on: - push: -jobs: - run-tests: - runs-on: ubuntu-latest - env: - TEST_RESULTS: "/tmp/test-results" - steps: - - name: Setup go - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 - with: - go-version: 1.18 - - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 - - run: mkdir -p "$TEST_RESULTS"/go-retryablyhttp - - name: restore_cache - uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2 - with: - key: go-mod-v1-{{ checksum "go.sum" }} - restore-keys: go-mod-v1-{{ checksum "go.sum" }} - path: "/go/pkg/mod" - - run: go mod download - - run: go mod tidy - - name: Run go format - run: |- - files=$(go fmt ./...) - if [ -n "$files" ]; then - echo "The following file(s) do not conform to go fmt:" - echo "$files" - exit 1 - fi - - name: Install gotestsum - run: go install gotest.tools/gotestsum@latest - - name: Run unit tests - run: |- - PACKAGE_NAMES=$(go list ./...) - # shellcheck disable=SC2086 # can't quote package list - gotestsum --junitfile "${TEST_RESULTS}"/go-retryablyhttp/gotestsum-report.xml -- $PACKAGE_NAMES - - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 - with: - path: "/tmp/test-results" - - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 - with: - path: "/tmp/test-results" -permissions: - contents: read diff --git a/.github/workflows/pr-gofmt.yaml b/.github/workflows/pr-gofmt.yaml new file mode 100644 index 0000000..cf5847a --- /dev/null +++ b/.github/workflows/pr-gofmt.yaml @@ -0,0 +1,23 @@ +name: Go format check +on: + pull_request: + types: ['opened', 'synchronize'] + +jobs: + run-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + + - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 + with: + go-version-file: ./.go-version + + - name: Run go format + run: |- + files=$(gofmt -s -l .) + if [ -n "$files" ]; then + echo >&2 "The following file(s) are not gofmt compliant:" + echo >&2 "$files" + exit 1 + fi diff --git a/.github/workflows/pr-unit-tests-1.19.yaml b/.github/workflows/pr-unit-tests-1.19.yaml new file mode 100644 index 0000000..e02ee25 --- /dev/null +++ b/.github/workflows/pr-unit-tests-1.19.yaml @@ -0,0 +1,17 @@ +name: Unit tests (Go 1.19) +on: + pull_request: + types: ['opened', 'synchronize'] + +jobs: + run-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + + - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 + with: + go-version: 1.19 + + - name: Run unit tests + run: make test diff --git a/.github/workflows/pr-unit-tests-1.20.yaml b/.github/workflows/pr-unit-tests-1.20.yaml new file mode 100644 index 0000000..bf492c0 --- /dev/null +++ b/.github/workflows/pr-unit-tests-1.20.yaml @@ -0,0 +1,17 @@ +name: Unit tests (Go 1.20+) +on: + pull_request: + types: ['opened', 'synchronize'] + +jobs: + run-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + + - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 + with: + go-version: 1.22 + + - name: Run unit tests + run: make test diff --git a/Makefile b/Makefile index da17640..5255241 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ default: test test: go vet ./... - go test -race ./... + go test -v -race ./... updatedeps: go get -f -t -u ./... From f67cc6e7850a16c067a86828dbca2a28d21ccc46 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 9 May 2024 16:50:30 +0100 Subject: [PATCH 55/59] deprecations, linting --- client.go | 13 ++++++------- client_test.go | 11 +++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/client.go b/client.go index 9b6a20d..f69063c 100644 --- a/client.go +++ b/client.go @@ -30,7 +30,6 @@ import ( "crypto/x509" "fmt" "io" - "io/ioutil" "log" "math" "math/rand" @@ -264,7 +263,7 @@ func getBodyReaderAndContentLength(rawBody interface{}) (ReaderFunc, int64, erro raw := body bodyReader = func() (io.Reader, error) { _, err := raw.Seek(0, 0) - return ioutil.NopCloser(raw), err + return io.NopCloser(raw), err } if lr, ok := raw.(LenReader); ok { contentLength = int64(lr.Len()) @@ -272,7 +271,7 @@ func getBodyReaderAndContentLength(rawBody interface{}) (ReaderFunc, int64, erro // Read all in so we can reset case io.Reader: - buf, err := ioutil.ReadAll(body) + buf, err := io.ReadAll(body) if err != nil { return nil, 0, err } @@ -619,13 +618,13 @@ func LinearJitterBackoff(min, max time.Duration, attemptNum int, resp *http.Resp } // Seed rand; doing this every time is fine - rand := rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) + source := rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) // Pick a random number that lies somewhere between the min and max and // multiply by the attemptNum. attemptNum starts at zero so we always // increment here. We first get a random percentage, then apply that to the // difference between min and max, and add to min. - jitter := rand.Float64() * float64(max-min) + jitter := source.Float64() * float64(max-min) jitterMin := int64(jitter) + int64(min) return time.Duration(jitterMin * int64(attemptNum)) } @@ -675,7 +674,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) { if c, ok := body.(io.ReadCloser); ok { req.Body = c } else { - req.Body = ioutil.NopCloser(body) + req.Body = io.NopCloser(body) } } @@ -820,7 +819,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) { // Try to read the response body so we can reuse this connection. func (c *Client) drainBody(body io.ReadCloser) { defer body.Close() - _, err := io.Copy(ioutil.Discard, io.LimitReader(body, respReadLimit)) + _, err := io.Copy(io.Discard, io.LimitReader(body, respReadLimit)) if err != nil { if c.logger() != nil { switch v := c.logger().(type) { diff --git a/client_test.go b/client_test.go index e5c8429..3cba8ec 100644 --- a/client_test.go +++ b/client_test.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net" "net/http" "net/http/httptest" @@ -206,7 +205,7 @@ func testClientDo(t *testing.T, body interface{}) { } // Check the payload - body, err := ioutil.ReadAll(r.Body) + body, err := io.ReadAll(r.Body) if err != nil { t.Fatalf("err: %s", err) } @@ -657,7 +656,7 @@ func testClientResponseLogHook(t *testing.T, l interface{}, buf *bytes.Buffer) { } } else { // Log the response body when we get a 500 - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) if err != nil { t.Fatalf("err: %v", err) } @@ -678,7 +677,7 @@ func testClientResponseLogHook(t *testing.T, l interface{}, buf *bytes.Buffer) { // Make sure we can read the response body still, since we did not // read or close it from the response log hook. - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) if err != nil { t.Fatalf("err: %v", err) } @@ -997,7 +996,7 @@ func TestClient_Post(t *testing.T) { } // Check the payload - body, err := ioutil.ReadAll(r.Body) + body, err := io.ReadAll(r.Body) if err != nil { t.Fatalf("err: %s", err) } @@ -1035,7 +1034,7 @@ func TestClient_PostForm(t *testing.T) { } // Check the payload - body, err := ioutil.ReadAll(r.Body) + body, err := io.ReadAll(r.Body) if err != nil { t.Fatalf("err: %s", err) } From 4077b295fe4471093769f60ffe2ea4e9cde191a5 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 9 May 2024 17:15:39 +0100 Subject: [PATCH 56/59] Changelog for #210 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e16887d..1c601a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ ENHANCEMENTS: - client: support HTTP-date values for `Retry-After` header value (#138) - client: avoid reading entire body when the body is a `*bytes.Reader` (#197) +BUG FIXES: + +- client: fix a broken check for invalid server certificate in go 1.20+ (#210) + ## 0.7.5 (Nov 8, 2023) BUG FIXES: From 2ad8ed4a1d9e632284f6937e91b2f9a1d30e8298 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 9 May 2024 17:21:06 +0100 Subject: [PATCH 57/59] v0.7.6 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c601a2..0c4c7a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 0.7.6 (Unreleased) +## 0.7.6 (May 9, 2024) ENHANCEMENTS: From f3e9417dbfcd0dc2b4a02a1dfdeb75f1e636b692 Mon Sep 17 00:00:00 2001 From: guoguangwu Date: Sat, 11 May 2024 10:58:39 +0800 Subject: [PATCH 58/59] chore: remove refs to deprecated io/ioutil Signed-off-by: guoguangwu --- roundtripper_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/roundtripper_test.go b/roundtripper_test.go index dcb02df..975d8b1 100644 --- a/roundtripper_test.go +++ b/roundtripper_test.go @@ -6,7 +6,7 @@ package retryablehttp import ( "context" "errors" - "io/ioutil" + "io" "net" "net/http" "net/http/httptest" @@ -88,7 +88,7 @@ func TestRoundTripper_RoundTrip(t *testing.T) { if resp.StatusCode != 200 { t.Fatalf("expected 200, got %d", resp.StatusCode) } - if v, err := ioutil.ReadAll(resp.Body); err != nil { + if v, err := io.ReadAll(resp.Body); err != nil { t.Fatal(err) } else if string(v) != "success!" { t.Fatalf("expected %q, got %q", "success!", v) From 47fe99e6460cddc5f433aad2b54dcf32281f8a53 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 16 May 2024 22:51:53 +0000 Subject: [PATCH 59/59] Bump actions/checkout from 4.1.5 to 4.1.6 Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.5 to 4.1.6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/44c2b7a8a4ea60a981eaca3cf939b5f4305c123b...a5ac7e51b41094c92402da3b24376905380afc29) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- .github/workflows/actionlint.yml | 2 +- .github/workflows/pr-gofmt.yaml | 2 +- .github/workflows/pr-unit-tests-1.19.yaml | 2 +- .github/workflows/pr-unit-tests-1.20.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/actionlint.yml b/.github/workflows/actionlint.yml index e0d2f4f..42da307 100644 --- a/.github/workflows/actionlint.yml +++ b/.github/workflows/actionlint.yml @@ -12,7 +12,7 @@ jobs: actionlint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + - uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: "Check GitHub workflow files" uses: docker://docker.mirror.hashicorp.services/rhysd/actionlint:latest with: diff --git a/.github/workflows/pr-gofmt.yaml b/.github/workflows/pr-gofmt.yaml index cf5847a..dbe3089 100644 --- a/.github/workflows/pr-gofmt.yaml +++ b/.github/workflows/pr-gofmt.yaml @@ -7,7 +7,7 @@ jobs: run-tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + - uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: diff --git a/.github/workflows/pr-unit-tests-1.19.yaml b/.github/workflows/pr-unit-tests-1.19.yaml index e02ee25..28a21c7 100644 --- a/.github/workflows/pr-unit-tests-1.19.yaml +++ b/.github/workflows/pr-unit-tests-1.19.yaml @@ -7,7 +7,7 @@ jobs: run-tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + - uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: diff --git a/.github/workflows/pr-unit-tests-1.20.yaml b/.github/workflows/pr-unit-tests-1.20.yaml index bf492c0..14fc918 100644 --- a/.github/workflows/pr-unit-tests-1.20.yaml +++ b/.github/workflows/pr-unit-tests-1.20.yaml @@ -7,7 +7,7 @@ jobs: run-tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + - uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: