From 691bfdb7ae4f188167aa332ad91405906d1ec0d8 Mon Sep 17 00:00:00 2001 From: Antoine Mercadal Date: Tue, 26 Mar 2019 13:20:42 -0700 Subject: [PATCH] new/maniphttp: built in retry --- maniphttp/manipulator.go | 31 +++++++++++++++++++++++-------- maniphttp/manipulator_test.go | 20 ++++++++++---------- maniphttp/options.go | 10 ++++++++++ maniphttp/options_test.go | 6 ++++++ 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/maniphttp/manipulator.go b/maniphttp/manipulator.go index d95cb788..4668f369 100644 --- a/maniphttp/manipulator.go +++ b/maniphttp/manipulator.go @@ -34,6 +34,7 @@ type httpManipulator struct { renewLock *sync.RWMutex renewNotifiers map[string]func(string) renewNotifiersLock *sync.RWMutex + disableAutoRetry bool // optionnable ctx context.Context @@ -516,6 +517,20 @@ func (s *httpManipulator) send(mctx manipulate.Context, method string, requrl st return nil, err } + retryErrCom := func(resp *http.Response, err error) (*http.Response, error) { + + if s.disableAutoRetry { + return resp, err + } + + if try >= 3 { + return resp, err + } + <-time.After(time.Duration(try) * time.Second) + try++ + return s.send(mctx, method, requrl, body, sp, try) + } + s.prepareHeaders(request, mctx) request = request.WithContext(mctx.Context()) @@ -529,27 +544,27 @@ func (s *httpManipulator) send(mctx manipulate.Context, method string, requrl st } } - return response, manipulate.NewErrCannotCommunicate(snip.Snip(err, s.currentPassword()).Error()) + return retryErrCom(response, manipulate.NewErrCannotCommunicate(snip.Snip(err, s.currentPassword()).Error())) } if response.StatusCode == http.StatusBadGateway { - return response, manipulate.NewErrCannotCommunicate("Bad gateway") + return retryErrCom(response, manipulate.NewErrCannotCommunicate("Bad gateway")) } if response.StatusCode == http.StatusServiceUnavailable { - return response, manipulate.NewErrCannotCommunicate("Service unavailable") + return retryErrCom(response, manipulate.NewErrCannotCommunicate("Service unavailable")) } if response.StatusCode == http.StatusGatewayTimeout { - return response, manipulate.NewErrCannotCommunicate("Gateway timeout") + return retryErrCom(response, manipulate.NewErrCannotCommunicate("Gateway timeout")) } if response.StatusCode == http.StatusLocked { - return response, manipulate.NewErrLocked("The api has been locked down by the server.") + return retryErrCom(response, manipulate.NewErrLocked("The api has been locked down by the server.")) } // If we get a forbidden or auth error, we try to renew the token and retry the request 3 times - if (response.StatusCode == http.StatusForbidden || response.StatusCode == http.StatusUnauthorized) && s.tokenManager != nil && try <= 3 { + if (response.StatusCode == http.StatusForbidden || response.StatusCode == http.StatusUnauthorized) && s.tokenManager != nil && try < 3 { <-time.After(time.Duration(try) * time.Second) try++ @@ -577,11 +592,11 @@ func (s *httpManipulator) send(mctx manipulate.Context, method string, requrl st } if response.StatusCode == http.StatusRequestTimeout { - return response, manipulate.NewErrCannotCommunicate(errs.Error()) + return retryErrCom(response, manipulate.NewErrCannotCommunicate(errs.Error())) } if response.StatusCode == http.StatusTooManyRequests { - return response, manipulate.NewErrTooManyRequests(errs.Error()) + return retryErrCom(response, manipulate.NewErrTooManyRequests(errs.Error())) } return response, errs diff --git a/maniphttp/manipulator_test.go b/maniphttp/manipulator_test.go index aa75b92d..fc38a21a 100644 --- a/maniphttp/manipulator_test.go +++ b/maniphttp/manipulator_test.go @@ -980,7 +980,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1005,7 +1005,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1030,7 +1030,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1055,7 +1055,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1080,7 +1080,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1105,7 +1105,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1137,7 +1137,7 @@ func TestHTTP_send(t *testing.T) { Convey("When I call send", func() { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1160,7 +1160,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1185,7 +1185,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) @@ -1212,7 +1212,7 @@ func TestHTTP_send(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := m.(*httpManipulator).send(manipulate.NewContext(ctx), http.MethodPost, ts.URL, nil, sp, 0) diff --git a/maniphttp/options.go b/maniphttp/options.go index cb49e6f5..7c69952e 100644 --- a/maniphttp/options.go +++ b/maniphttp/options.go @@ -78,3 +78,13 @@ func OptionTLSConfig(tlsConfig *tls.Config) Option { m.tlsConfig = tlsConfig } } + +// OptionDisableBuiltInRetry disables the auto retry mechanism +// built in maniphttp Manipulator. +// By default, the manipulator will silently retry on communication +// error 3 times after 1s, 2s, and 3s. +func OptionDisableBuiltInRetry() Option { + return func(m *httpManipulator) { + m.disableAutoRetry = true + } +} diff --git a/maniphttp/options_test.go b/maniphttp/options_test.go index 1d561932..d5fd3515 100644 --- a/maniphttp/options_test.go +++ b/maniphttp/options_test.go @@ -70,4 +70,10 @@ func TestManipHttp_Optionions(t *testing.T) { OptionAdditonalHeaders(h)(m) So(m.globalHeaders, ShouldEqual, h) }) + + Convey("Calling OptionDisableBuiltInRetry should work", t, func() { + m := &httpManipulator{} + OptionDisableBuiltInRetry()(m) + So(m.disableAutoRetry, ShouldBeTrue) + }) }