new/maniphttp: built in retry

PaloAltoNetworks · Mar 26, 2019 · 691bfdb · 691bfdb
1 parent ef3cf14
commit 691bfdb
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)
+	})
 }