From bc798b06c70f9e69e2495b9d87b55d9bfb989724 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Thu, 3 Aug 2023 15:27:56 -0400 Subject: [PATCH] Add retryablehttp Client Option (#619) * Add hashicorp:retryablehttp-go Client Option * Add transport config * add more doc * add more doc * remove extra line * add retry-after * add retry-after to godo_test.go * add optional retrywaitmix and max * Update godoc, remove retryAfter json, go mod tidy --- go.mod | 2 ++ go.sum | 7 ++++++ godo.go | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ godo_test.go | 49 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+) diff --git a/go.mod b/go.mod index aae2a897..9b25316f 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.18 require ( github.com/google/go-querystring v1.1.0 + github.com/hashicorp/go-retryablehttp v0.7.4 github.com/stretchr/testify v1.4.0 golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 golang.org/x/time v0.0.0-20220922220347-f3bd1da661af @@ -12,6 +13,7 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/golang/protobuf v1.5.2 // indirect + github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/net v0.7.0 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/go.sum b/go.sum index 7bbdf5b4..08f8021f 100644 --- a/go.sum +++ b/go.sum @@ -102,6 +102,12 @@ github.com/google/pprof v0.0.0-20200708004538-1a94d8640e99/go.mod h1:ZgVRPoUq/hf github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= +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-retryablehttp v0.7.4 h1:ZQgVdpTdAL7WpMIwLzCfbalOcSUdkDZnpUv3/+BxzFA= +github.com/hashicorp/go-retryablehttp v0.7.4/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= @@ -118,6 +124,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= +github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= diff --git a/godo.go b/godo.go index b858f67e..04cc42bf 100644 --- a/godo.go +++ b/godo.go @@ -16,6 +16,7 @@ import ( "time" "github.com/google/go-querystring/query" + "github.com/hashicorp/go-retryablehttp" "golang.org/x/oauth2" "golang.org/x/time/rate" ) @@ -92,6 +93,29 @@ type Client struct { // Optional rate limiter to ensure QoS. rateLimiter *rate.Limiter + + // Optional retry values. Setting the RetryConfig.RetryMax value enables automatically retrying requests + // that fail with 429 or 500-level response codes using the go-retryablehttp client + RetryConfig RetryConfig +} + +// RetryConfig sets the values used for enabling retries and backoffs for +// requests that fail with 429 or 500-level response codes using the go-retryablehttp client. +// RetryConfig.RetryMax must be configured to enable this behavior. RetryConfig.RetryWaitMin and +// RetryConfig.RetryWaitMax are optional, with the default values being 1.0 and 30.0, respectively. +// +// You can use +// +// godo.PtrTo(1.0) +// +// to explicitly set the RetryWaitMin and RetryWaitMax values. +// +// Note: Opting to use the go-retryablehttp client will overwrite any custom HTTP client passed into New(). +// Only the custom HTTP client's custom transport and timeout will be maintained. +type RetryConfig struct { + RetryMax int + RetryWaitMin *float64 // Minimum time to wait + RetryWaitMax *float64 // Maximum time to wait } // RequestCompletionCallback defines the type of the request callback function @@ -271,6 +295,33 @@ func New(httpClient *http.Client, opts ...ClientOpt) (*Client, error) { } } + // if retryMax is set it will use the retryablehttp client. + if c.RetryConfig.RetryMax > 0 { + retryableClient := retryablehttp.NewClient() + retryableClient.RetryMax = c.RetryConfig.RetryMax + + if c.RetryConfig.RetryWaitMin != nil { + retryableClient.RetryWaitMin = time.Duration(*c.RetryConfig.RetryWaitMin * float64(time.Second)) + } + if c.RetryConfig.RetryWaitMax != nil { + retryableClient.RetryWaitMax = time.Duration(*c.RetryConfig.RetryWaitMax * float64(time.Second)) + } + + // if timeout is set, it is maintained before overwriting client with StandardClient() + retryableClient.HTTPClient.Timeout = c.client.Timeout + + var source *oauth2.Transport + if _, ok := c.client.Transport.(*oauth2.Transport); ok { + source = c.client.Transport.(*oauth2.Transport) + } + c.client = retryableClient.StandardClient() + c.client.Transport = &oauth2.Transport{ + Base: c.client.Transport, + Source: source.Source, + } + + } + return c, nil } @@ -315,6 +366,17 @@ func SetStaticRateLimit(rps float64) ClientOpt { } } +// WithRetryAndBackoffs sets retry values. Setting the RetryConfig.RetryMax value enables automatically retrying requests +// that fail with 429 or 500-level response codes using the go-retryablehttp client +func WithRetryAndBackoffs(retryConfig RetryConfig) ClientOpt { + return func(c *Client) error { + c.RetryConfig.RetryMax = retryConfig.RetryMax + c.RetryConfig.RetryWaitMax = retryConfig.RetryWaitMax + c.RetryConfig.RetryWaitMin = retryConfig.RetryWaitMin + return nil + } +} + // NewRequest creates an API request. A relative URL can be provided in urlStr, which will be resolved to the // BaseURL of the Client. Relative URLS should always be specified without a preceding slash. If specified, the // value pointed to by body is JSON encoded and included in as the request body. diff --git a/godo_test.go b/godo_test.go index 62f49798..0badba83 100644 --- a/godo_test.go +++ b/godo_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + "golang.org/x/oauth2" "golang.org/x/time/rate" ) @@ -603,6 +604,54 @@ func TestDo_rateLimit_errorResponse(t *testing.T) { } } +// TestWithRetryAndBackoffs tests the retryablehttp client's default retry policy. +func TestWithRetryAndBackoffs(t *testing.T) { + // Mock server which always responds 500. + setup() + defer teardown() + + url, _ := url.Parse(server.URL) + mux.HandleFunc("/foo", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(500) + }) + + tokenSrc := oauth2.StaticTokenSource(&oauth2.Token{ + AccessToken: "new_token", + }) + + oauth_client := oauth2.NewClient(oauth2.NoContext, tokenSrc) + + waitMax := PtrTo(6.0) + waitMin := PtrTo(3.0) + + retryConfig := RetryConfig{ + RetryMax: 3, + RetryWaitMin: waitMin, + RetryWaitMax: waitMax, + } + + // Create the client. Use short retry windows so we fail faster. + client, err := New(oauth_client, WithRetryAndBackoffs(retryConfig)) + client.BaseURL = url + if err != nil { + t.Fatalf("err: %v", err) + } + + // Create the request + req, err := client.NewRequest(ctx, http.MethodGet, "/foo", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + + expectingErr := "giving up after 4 attempt(s)" + // Send the request. + _, err = client.Do(context.Background(), req, nil) + if err == nil || !strings.HasSuffix(err.Error(), expectingErr) { + t.Fatalf("expected giving up error, got: %#v", err) + } + +} + func checkCurrentPage(t *testing.T, resp *Response, expectedPage int) { links := resp.Links p, err := links.CurrentPage()