From ba1c4d497a4be06c468846090c0361075aacddc9 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Thu, 13 Jul 2023 11:49:20 -0400 Subject: [PATCH 1/9] Add hashicorp:retryablehttp-go Client Option --- go.mod | 2 ++ go.sum | 6 ++++ godo.go | 41 +++++++++++++++++++++++++++ godo_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+) diff --git a/go.mod b/go.mod index aae2a897..d8d6927a 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,8 @@ 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/hashicorp/go-retryablehttp v0.7.4 // 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..dceb9293 100644 --- a/go.sum +++ b/go.sum @@ -102,6 +102,11 @@ 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/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 +123,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 c48a5f78..b6284eb9 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,12 @@ type Client struct { // Optional rate limiter to ensure QoS. rateLimiter *rate.Limiter + + // Optional retry values. Setting the retry max value enables automatically retrying requests + // that fail with 429 or 500-level response codes using the go-retryablehttp client + 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 +278,16 @@ func New(httpClient *http.Client, opts ...ClientOpt) (*Client, error) { } } + // if retryMax is set it will use the retryablehttp client. + if c.retryMax > 0 { + retryableClient := retryablehttp.NewClient() + retryableClient.RetryMax = c.retryMax + retryableClient.RetryWaitMin = time.Duration(c.retryWaitMin * float64(time.Second)) + retryableClient.RetryWaitMax = time.Duration(c.retryWaitMin * float64(time.Second)) + + c.client = retryableClient.StandardClient() + } + return c, nil } @@ -315,6 +332,30 @@ func SetStaticRateLimit(rps float64) ClientOpt { } } +// SetRetryMax sets an optional client-side.... +func SetRetryMax(retryMax int) ClientOpt { + return func(c *Client) error { + c.retryMax = retryMax + return nil + } +} + +// SetRetryWaitMax sets an optional client-side.... +func SetRetryWaitMax(retryWaitMax float64) ClientOpt { + return func(c *Client) error { + c.retryWaitMax = retryWaitMax + return nil + } +} + +// SetRetryWaitMin sets an optional client-side.... +func SetRetryWaitMin(retryWaitMin float64) ClientOpt { + return func(c *Client) error { + c.retryWaitMin = 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..c6faef05 100644 --- a/godo_test.go +++ b/godo_test.go @@ -533,6 +533,7 @@ func TestDo_rateLimit(t *testing.T) { if client.Rate != client.GetRate() { t.Errorf("Client rate is not the same as client.GetRate()") } + } func TestDo_rateLimitRace(t *testing.T) { @@ -603,6 +604,41 @@ func TestDo_rateLimit_errorResponse(t *testing.T) { } } +// TestRetryableClient_DefaultRetryPolicy tests the retryablehttp client's default retry policy. +func TestRetryableClient_DefaultRetryPolicy(t *testing.T) { + // Mock server which always responds 500. + mux = http.NewServeMux() + server = httptest.NewServer(mux) + defer teardown() + + url, _ := url.Parse(server.URL) + mux.HandleFunc("/foo", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(500) + }) + + // Create the client. Use short retry windows so we fail faster. + client, err := New(nil, SetRetryMax(2), SetRetryWaitMax(6.0), SetRetryWaitMin(6.0)) + 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 3 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() @@ -782,6 +818,48 @@ func TestSetStaticRateLimit(t *testing.T) { } } +func TestSetRetryMax(t *testing.T) { + retryMax := 5 + c, err := New(nil, SetRetryMax(retryMax)) + if err != nil { + t.Fatalf("New() unexpected error: %v", err) + } + + expected := retryMax + if got := c.retryMax; got != expected { + t.Errorf("retryMax = %+v; expected %+v", got, expected) + } + +} + +func TestSetRetryWaitMax(t *testing.T) { + retryWaitMax := 1.0 + c, err := New(nil, SetRetryWaitMax(retryWaitMax)) + if err != nil { + t.Fatalf("New() unexpected error: %v", err) + } + + expected := retryWaitMax + if got := c.retryWaitMax; got != expected { + t.Errorf("retryWaitMax = %+v; expected %+v", got, expected) + } + +} + +func TestSetRetryWaitMin(t *testing.T) { + retryWaitMin := 1.0 + c, err := New(nil, SetRetryWaitMin(retryWaitMin)) + if err != nil { + t.Fatalf("New() unexpected error: %v", err) + } + + expected := retryWaitMin + if got := c.retryWaitMin; got != expected { + t.Errorf("retryWaitMin = %+v; expected %+v", got, expected) + } + +} + func TestCustomBaseURL_badURL(t *testing.T) { baseURL := ":" _, err := New(nil, SetBaseURL(baseURL)) From 8cd3b9bdcebbad7e3ac5ca6a7d966859a31c6e16 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Tue, 18 Jul 2023 14:36:02 -0400 Subject: [PATCH 2/9] Add transport config --- godo.go | 58 ++++++++++++++++++++++++--------------------- godo_test.go | 66 ++++++++++++++-------------------------------------- 2 files changed, 48 insertions(+), 76 deletions(-) diff --git a/godo.go b/godo.go index b6284eb9..d28c44e4 100644 --- a/godo.go +++ b/godo.go @@ -94,11 +94,17 @@ type Client struct { // Optional rate limiter to ensure QoS. rateLimiter *rate.Limiter - // Optional retry values. Setting the retry max value enables automatically retrying requests + // 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 - retryMax int - retryWaitMin float64 // Minimum time to wait - retryWaitMax float64 // Maximum time to wait + 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. +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 @@ -279,13 +285,25 @@ func New(httpClient *http.Client, opts ...ClientOpt) (*Client, error) { } // if retryMax is set it will use the retryablehttp client. - if c.retryMax > 0 { + if c.RetryConfig.RetryMax > 0 { retryableClient := retryablehttp.NewClient() - retryableClient.RetryMax = c.retryMax - retryableClient.RetryWaitMin = time.Duration(c.retryWaitMin * float64(time.Second)) - retryableClient.RetryWaitMax = time.Duration(c.retryWaitMin * float64(time.Second)) + retryableClient.RetryMax = c.RetryConfig.RetryMax + retryableClient.RetryWaitMin = time.Duration(c.RetryConfig.RetryWaitMin * float64(time.Second)) + retryableClient.RetryWaitMax = time.Duration(c.RetryConfig.RetryWaitMin * 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 @@ -332,26 +350,12 @@ func SetStaticRateLimit(rps float64) ClientOpt { } } -// SetRetryMax sets an optional client-side.... -func SetRetryMax(retryMax int) ClientOpt { - return func(c *Client) error { - c.retryMax = retryMax - return nil - } -} - -// SetRetryWaitMax sets an optional client-side.... -func SetRetryWaitMax(retryWaitMax float64) ClientOpt { - return func(c *Client) error { - c.retryWaitMax = retryWaitMax - return nil - } -} - -// SetRetryWaitMin sets an optional client-side.... -func SetRetryWaitMin(retryWaitMin float64) ClientOpt { +// WithRetryAndBackoffs sets an +func WithRetryAndBackoffs(retryConfig RetryConfig) ClientOpt { return func(c *Client) error { - c.retryWaitMin = retryWaitMin + c.RetryConfig.RetryMax = retryConfig.RetryMax + c.RetryConfig.RetryWaitMax = retryConfig.RetryWaitMax + c.RetryConfig.RetryWaitMin = retryConfig.RetryWaitMin return nil } } diff --git a/godo_test.go b/godo_test.go index c6faef05..9daed35c 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" ) @@ -604,11 +605,10 @@ func TestDo_rateLimit_errorResponse(t *testing.T) { } } -// TestRetryableClient_DefaultRetryPolicy tests the retryablehttp client's default retry policy. -func TestRetryableClient_DefaultRetryPolicy(t *testing.T) { +// TestWithRetryAndBackoffs tests the retryablehttp client's default retry policy. +func TestWithRetryAndBackoffs(t *testing.T) { // Mock server which always responds 500. - mux = http.NewServeMux() - server = httptest.NewServer(mux) + setup() defer teardown() url, _ := url.Parse(server.URL) @@ -616,8 +616,19 @@ func TestRetryableClient_DefaultRetryPolicy(t *testing.T) { w.WriteHeader(500) }) + tokenSrc := oauth2.StaticTokenSource(&oauth2.Token{ + AccessToken: "new_token", + }) + + oauth_client := oauth2.NewClient(oauth2.NoContext, tokenSrc) + retryConfig := RetryConfig{ + RetryMax: 3, + RetryWaitMin: 3.0, + RetryWaitMax: 6.0, + } + // Create the client. Use short retry windows so we fail faster. - client, err := New(nil, SetRetryMax(2), SetRetryWaitMax(6.0), SetRetryWaitMin(6.0)) + client, err := New(oauth_client, WithRetryAndBackoffs(retryConfig)) client.BaseURL = url if err != nil { t.Fatalf("err: %v", err) @@ -629,8 +640,7 @@ func TestRetryableClient_DefaultRetryPolicy(t *testing.T) { t.Fatalf("err: %v", err) } - expectingErr := "giving up after 3 attempt(s)" - + 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) { @@ -818,48 +828,6 @@ func TestSetStaticRateLimit(t *testing.T) { } } -func TestSetRetryMax(t *testing.T) { - retryMax := 5 - c, err := New(nil, SetRetryMax(retryMax)) - if err != nil { - t.Fatalf("New() unexpected error: %v", err) - } - - expected := retryMax - if got := c.retryMax; got != expected { - t.Errorf("retryMax = %+v; expected %+v", got, expected) - } - -} - -func TestSetRetryWaitMax(t *testing.T) { - retryWaitMax := 1.0 - c, err := New(nil, SetRetryWaitMax(retryWaitMax)) - if err != nil { - t.Fatalf("New() unexpected error: %v", err) - } - - expected := retryWaitMax - if got := c.retryWaitMax; got != expected { - t.Errorf("retryWaitMax = %+v; expected %+v", got, expected) - } - -} - -func TestSetRetryWaitMin(t *testing.T) { - retryWaitMin := 1.0 - c, err := New(nil, SetRetryWaitMin(retryWaitMin)) - if err != nil { - t.Fatalf("New() unexpected error: %v", err) - } - - expected := retryWaitMin - if got := c.retryWaitMin; got != expected { - t.Errorf("retryWaitMin = %+v; expected %+v", got, expected) - } - -} - func TestCustomBaseURL_badURL(t *testing.T) { baseURL := ":" _, err := New(nil, SetBaseURL(baseURL)) From eb9f7c6b894acb50fd36176e6202033a2978290e Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Tue, 18 Jul 2023 14:42:08 -0400 Subject: [PATCH 3/9] add more doc --- godo.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/godo.go b/godo.go index d28c44e4..d7469a94 100644 --- a/godo.go +++ b/godo.go @@ -101,6 +101,9 @@ type Client struct { // 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. +// 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 From e8cc9942b2b7d69af6fa82b4d34ac0476ed44546 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Tue, 18 Jul 2023 15:40:43 -0400 Subject: [PATCH 4/9] add more doc --- godo.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/godo.go b/godo.go index d7469a94..e9aef0c4 100644 --- a/godo.go +++ b/godo.go @@ -353,7 +353,8 @@ func SetStaticRateLimit(rps float64) ClientOpt { } } -// WithRetryAndBackoffs sets an +// 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 From 1eb3081c18e080e7e3878b291aa33848b6e28261 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Tue, 18 Jul 2023 15:45:03 -0400 Subject: [PATCH 5/9] remove extra line --- godo_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/godo_test.go b/godo_test.go index 9daed35c..fc7af582 100644 --- a/godo_test.go +++ b/godo_test.go @@ -534,7 +534,6 @@ func TestDo_rateLimit(t *testing.T) { if client.Rate != client.GetRate() { t.Errorf("Client rate is not the same as client.GetRate()") } - } func TestDo_rateLimitRace(t *testing.T) { From 6778bc0bbac742b6de56f8c84c26f75e0c115744 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Fri, 21 Jul 2023 12:21:15 -0400 Subject: [PATCH 6/9] add retry-after --- godo.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/godo.go b/godo.go index e9aef0c4..a56b7071 100644 --- a/godo.go +++ b/godo.go @@ -30,6 +30,7 @@ const ( headerRateLimit = "RateLimit-Limit" headerRateRemaining = "RateLimit-Remaining" headerRateReset = "RateLimit-Reset" + headerRetryAfter = "Retry-After" ) // Client manages communication with DigitalOcean V2 API. @@ -181,6 +182,9 @@ type Rate struct { // The time at which the current rate limit will reset. Reset Timestamp `json:"reset"` + + // The number of seconds the client is expected to wait before making a follow-up HTTP request + RetryAfter int `json:"retry-after"` } func addOptions(s string, opt interface{}) (string, error) { @@ -441,6 +445,9 @@ func (r *Response) populateRate() { r.Rate.Reset = Timestamp{time.Unix(v, 0)} } } + if retryAfter := r.Header.Get(headerRetryAfter); retryAfter != "" { + r.Rate.RetryAfter, _ = strconv.Atoi(retryAfter) + } } // Do sends an API request and returns the API response. The API response is JSON decoded and stored in the value From f363fea6e86dadbc5d64b317258ecd248ca50b49 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Fri, 21 Jul 2023 14:55:59 -0400 Subject: [PATCH 7/9] add retry-after to godo_test.go --- godo_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/godo_test.go b/godo_test.go index fc7af582..ff3a3060 100644 --- a/godo_test.go +++ b/godo_test.go @@ -498,6 +498,7 @@ func TestDo_rateLimit(t *testing.T) { w.Header().Add(headerRateLimit, "60") w.Header().Add(headerRateRemaining, "59") w.Header().Add(headerRateReset, "1372700873") + w.Header().Add(headerRetryAfter, "23") }) var expected int @@ -508,6 +509,9 @@ func TestDo_rateLimit(t *testing.T) { if expected = 0; client.Rate.Remaining != expected { t.Errorf("Client rate remaining = %v, got %v", client.Rate.Remaining, expected) } + if expected = 0; client.Rate.RetryAfter != expected { + t.Errorf("Client rate retry-after = %v, got %v", client.Rate.RetryAfter, expected) + } if !client.Rate.Reset.IsZero() { t.Errorf("Client rate reset not initialized to zero value") } @@ -527,6 +531,9 @@ func TestDo_rateLimit(t *testing.T) { if expected = 59; client.Rate.Remaining != expected { t.Errorf("Client rate remaining = %v, expected %v", client.Rate.Remaining, expected) } + if expected = 23; client.Rate.RetryAfter != expected { + t.Errorf("Client rate retry-after = %v, expected %v", client.Rate.RetryAfter, expected) + } reset := time.Date(2013, 7, 1, 17, 47, 53, 0, time.UTC) if client.Rate.Reset.UTC() != reset { t.Errorf("Client rate reset = %v, expected %v", client.Rate.Reset, reset) From 32d5d9428718065420955591cd77ac29412dddc8 Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Wed, 2 Aug 2023 15:33:36 -0400 Subject: [PATCH 8/9] add optional retrywaitmix and max --- godo.go | 16 +++++++++++----- godo_test.go | 10 ++++++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/godo.go b/godo.go index e3cbe555..f7b9c0e6 100644 --- a/godo.go +++ b/godo.go @@ -102,13 +102,14 @@ type Client struct { // 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.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. // 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 + RetryWaitMin *float64 // Minimum time to wait + RetryWaitMax *float64 // Maximum time to wait } // RequestCompletionCallback defines the type of the request callback function @@ -295,8 +296,13 @@ func New(httpClient *http.Client, opts ...ClientOpt) (*Client, error) { if c.RetryConfig.RetryMax > 0 { retryableClient := retryablehttp.NewClient() retryableClient.RetryMax = c.RetryConfig.RetryMax - retryableClient.RetryWaitMin = time.Duration(c.RetryConfig.RetryWaitMin * float64(time.Second)) - retryableClient.RetryWaitMax = time.Duration(c.RetryConfig.RetryWaitMin * float64(time.Second)) + + 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 diff --git a/godo_test.go b/godo_test.go index ff3a3060..9f030a7d 100644 --- a/godo_test.go +++ b/godo_test.go @@ -627,10 +627,16 @@ func TestWithRetryAndBackoffs(t *testing.T) { }) oauth_client := oauth2.NewClient(oauth2.NoContext, tokenSrc) + var waitMax *float64 = new(float64) + var waitMin *float64 = new(float64) + + *waitMax = 6.0 + *waitMin = 3.0 + retryConfig := RetryConfig{ RetryMax: 3, - RetryWaitMin: 3.0, - RetryWaitMax: 6.0, + RetryWaitMin: waitMin, + RetryWaitMax: waitMax, } // Create the client. Use short retry windows so we fail faster. From f19d93882fc16892d59abe5f2f2fa10b018d68df Mon Sep 17 00:00:00 2001 From: danaelhe <42972711+danaelhe@users.noreply.github.com> Date: Thu, 3 Aug 2023 11:00:35 -0400 Subject: [PATCH 9/9] Update godoc, remove retryAfter json, go mod tidy --- go.mod | 2 +- go.sum | 1 + godo.go | 14 +++++++------- godo_test.go | 13 ++----------- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index d8d6927a..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 @@ -13,7 +14,6 @@ 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/hashicorp/go-retryablehttp v0.7.4 // 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 dceb9293..08f8021f 100644 --- a/go.sum +++ b/go.sum @@ -104,6 +104,7 @@ github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+ 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= diff --git a/godo.go b/godo.go index f7b9c0e6..04cc42bf 100644 --- a/godo.go +++ b/godo.go @@ -30,7 +30,6 @@ const ( headerRateLimit = "RateLimit-Limit" headerRateRemaining = "RateLimit-Remaining" headerRateReset = "RateLimit-Reset" - headerRetryAfter = "Retry-After" ) // Client manages communication with DigitalOcean V2 API. @@ -104,6 +103,13 @@ type Client struct { // 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 { @@ -183,9 +189,6 @@ type Rate struct { // The time at which the current rate limit will reset. Reset Timestamp `json:"reset"` - - // The number of seconds the client is expected to wait before making a follow-up HTTP request - RetryAfter int `json:"retry-after"` } func addOptions(s string, opt interface{}) (string, error) { @@ -451,9 +454,6 @@ func (r *Response) populateRate() { r.Rate.Reset = Timestamp{time.Unix(v, 0)} } } - if retryAfter := r.Header.Get(headerRetryAfter); retryAfter != "" { - r.Rate.RetryAfter, _ = strconv.Atoi(retryAfter) - } } // Do sends an API request and returns the API response. The API response is JSON decoded and stored in the value diff --git a/godo_test.go b/godo_test.go index 9f030a7d..0badba83 100644 --- a/godo_test.go +++ b/godo_test.go @@ -498,7 +498,6 @@ func TestDo_rateLimit(t *testing.T) { w.Header().Add(headerRateLimit, "60") w.Header().Add(headerRateRemaining, "59") w.Header().Add(headerRateReset, "1372700873") - w.Header().Add(headerRetryAfter, "23") }) var expected int @@ -509,9 +508,6 @@ func TestDo_rateLimit(t *testing.T) { if expected = 0; client.Rate.Remaining != expected { t.Errorf("Client rate remaining = %v, got %v", client.Rate.Remaining, expected) } - if expected = 0; client.Rate.RetryAfter != expected { - t.Errorf("Client rate retry-after = %v, got %v", client.Rate.RetryAfter, expected) - } if !client.Rate.Reset.IsZero() { t.Errorf("Client rate reset not initialized to zero value") } @@ -531,9 +527,6 @@ func TestDo_rateLimit(t *testing.T) { if expected = 59; client.Rate.Remaining != expected { t.Errorf("Client rate remaining = %v, expected %v", client.Rate.Remaining, expected) } - if expected = 23; client.Rate.RetryAfter != expected { - t.Errorf("Client rate retry-after = %v, expected %v", client.Rate.RetryAfter, expected) - } reset := time.Date(2013, 7, 1, 17, 47, 53, 0, time.UTC) if client.Rate.Reset.UTC() != reset { t.Errorf("Client rate reset = %v, expected %v", client.Rate.Reset, reset) @@ -627,11 +620,9 @@ func TestWithRetryAndBackoffs(t *testing.T) { }) oauth_client := oauth2.NewClient(oauth2.NoContext, tokenSrc) - var waitMax *float64 = new(float64) - var waitMin *float64 = new(float64) - *waitMax = 6.0 - *waitMin = 3.0 + waitMax := PtrTo(6.0) + waitMin := PtrTo(3.0) retryConfig := RetryConfig{ RetryMax: 3,