From f68f062ecbeb57744965e672950eadf816dfd9b0 Mon Sep 17 00:00:00 2001 From: lukasjenicek Date: Tue, 28 Nov 2023 14:47:49 +0100 Subject: [PATCH 1/4] cleanup code: remove extra else branch --- retry.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/retry.go b/retry.go index 9273f30..3025dde 100644 --- a/retry.go +++ b/retry.go @@ -32,7 +32,7 @@ func Retry(baseTransport http.RoundTripper, maxRetries int) func(http.RoundTripp return func(next http.RoundTripper) http.RoundTripper { return RoundTripFunc(func(req *http.Request) (resp *http.Response, err error) { defer func() { - if isRetryable(err, resp) { + if err == nil || !isRetryable(err, resp) { for i := 1; i <= maxRetries; i++ { wait := backOff(resp, i) @@ -50,13 +50,12 @@ func Retry(baseTransport http.RoundTripper, maxRetries int) func(http.RoundTripp startTime := time.Now() resp, err = baseTransport.RoundTrip(req) - if isRetryable(err, resp) { - log.Printf("retrying %d request: %s %s", i, req.Method, req.URL) - log.Printf("response (%v): %v %s", time.Since(startTime), resp.Status, resp.Request.URL) - continue - } else { + if err == nil || !isRetryable(err, resp) { break } + + log.Printf("retrying %d request: %s %s", i, req.Method, req.URL) + log.Printf("response (%v): %v %s", time.Since(startTime), resp.Status, resp.Request.URL) } } }() From d4db2e987ba4478bca5ae9e089e72983b2b582e4 Mon Sep 17 00:00:00 2001 From: lukasjenicek Date: Tue, 28 Nov 2023 14:49:50 +0100 Subject: [PATCH 2/4] remove extra error matching --- retry.go | 40 ++-------------------------------------- 1 file changed, 2 insertions(+), 38 deletions(-) diff --git a/retry.go b/retry.go index 3025dde..c15acfe 100644 --- a/retry.go +++ b/retry.go @@ -1,33 +1,14 @@ package transport import ( - "crypto/x509" "net/http" "net/url" - "regexp" "log" "time" "strconv" "math" ) -var ( - // A regular expression to match the error returned by net/http when the - // configured number of redirects is exhausted. This error isn't typed - // specifically so we resort to matching on the error string. - tooManyRedirectsRe = regexp.MustCompile(`stopped after \d+ redirects\z`) - - // A regular expression to match the error returned by net/http when the - // scheme specified in the URL is invalid. This error isn't typed - // specifically so we resort to matching on the error string. - invalidSchemeRe = 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. - untrustedCertificateRe = regexp.MustCompile(`certificate is not trusted`) -) - func Retry(baseTransport http.RoundTripper, maxRetries int) func(http.RoundTripper) http.RoundTripper { return func(next http.RoundTripper) http.RoundTripper { return RoundTripFunc(func(req *http.Request) (resp *http.Response, err error) { @@ -92,25 +73,8 @@ func isRetryable(err error, resp *http.Response) bool { } // any error returned from Client.Do will be *url.Error - if serverErr, ok := err.(*url.Error); ok { - // Too many redirects. - if tooManyRedirectsRe.MatchString(serverErr.Error()) { - return false - } - - // Invalid protocol scheme. - if invalidSchemeRe.MatchString(serverErr.Error()) { - return false - } - - // TLS cert verification failure. - if untrustedCertificateRe.MatchString(serverErr.Error()) { - return false - } - - if _, ok := serverErr.Err.(x509.UnknownAuthorityError); ok { - return false - } + if _, ok := err.(*url.Error); ok { + return false } // 429 Too Many Requests is recoverable. Sometimes the server puts From b514ecb28544ce057b05b41727787b305c670238 Mon Sep 17 00:00:00 2001 From: lukasjenicek Date: Tue, 28 Nov 2023 14:51:20 +0100 Subject: [PATCH 3/4] fix formatting issue --- transport.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/transport.go b/transport.go index 0a9ed02..299f9a0 100644 --- a/transport.go +++ b/transport.go @@ -26,10 +26,10 @@ func (f RoundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { // authClient := http.Client{ // Transport: transport.Chain( // http.DefaultTransport, -// transport.SetHeader("User-Agent", userAgent), -// transport.SetHeader("Authorization", authHeader), -// transport.SetHeader("x-extra", "value"), -// transport.TraceID, +// transport.SetHeader("User-Agent", userAgent), +// transport.SetHeader("Authorization", authHeader), +// transport.SetHeader("x-extra", "value"), +// transport.TraceID, // ), // Timeout: 15 * time.Second, // } From a4c4f7f36456fd2b3d680cb3926ba8b7e5c7a7ac Mon Sep 17 00:00:00 2001 From: lukasjenicek Date: Tue, 28 Nov 2023 15:05:14 +0100 Subject: [PATCH 4/4] fix retryable pattern --- retry.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/retry.go b/retry.go index c15acfe..299762a 100644 --- a/retry.go +++ b/retry.go @@ -2,7 +2,6 @@ package transport import ( "net/http" - "net/url" "log" "time" "strconv" @@ -13,7 +12,7 @@ func Retry(baseTransport http.RoundTripper, maxRetries int) func(http.RoundTripp return func(next http.RoundTripper) http.RoundTripper { return RoundTripFunc(func(req *http.Request) (resp *http.Response, err error) { defer func() { - if err == nil || !isRetryable(err, resp) { + if isRetryable(resp) { for i := 1; i <= maxRetries; i++ { wait := backOff(resp, i) @@ -31,7 +30,7 @@ func Retry(baseTransport http.RoundTripper, maxRetries int) func(http.RoundTripp startTime := time.Now() resp, err = baseTransport.RoundTrip(req) - if err == nil || !isRetryable(err, resp) { + if !isRetryable(resp) { break } @@ -67,16 +66,11 @@ func backOff(resp *http.Response, attempt int) time.Duration { return sleep } -func isRetryable(err error, resp *http.Response) bool { +func isRetryable(resp *http.Response) bool { if resp == nil { return false } - // any error returned from Client.Do will be *url.Error - if _, ok := err.(*url.Error); ok { - return false - } - // 429 Too Many Requests is recoverable. Sometimes the server puts // Retry-After response header to indicate when the server is will be available again if resp.StatusCode == http.StatusTooManyRequests {