Backport maxretries fix 7.12 (#242)

* Transport: Add tests for Max Retries validation

* Transport: Fix maxretries mechanism

* Transport: Test: Keep ReadAll invocation from ioutil until next major version of Go

Author: Anaethelion

estransport/estransport.go

@@ -231,7 +231,7 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
 		}
 	}
 
-	for i := 1; i <= c.maxRetries; i++ {
+	for i := 0; i <= c.maxRetries; i++ {
 		var (
 			conn            *Connection
 			shouldRetry     bool
@@ -253,7 +253,7 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
 		c.setReqURL(conn.URL, req)
 		c.setReqAuth(conn.URL, req)
 
-		if !c.disableRetry && i > 1 && req.Body != nil && req.Body != http.NoBody {
+		if !c.disableRetry && i > 0 && req.Body != nil && req.Body != http.NoBody {
 			body, err := req.GetBody()
 			if err != nil {
 				return nil, fmt.Errorf("cannot get request body: %s", err)
@@ -336,7 +336,7 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
 
 		// Delay the retry if a backoff function is configured
 		if c.retryBackoff != nil {
-			time.Sleep(c.retryBackoff(i))
+			time.Sleep(c.retryBackoff(i + 1))
 		}
 	}
 

estransport/estransport_integration_test.go

@@ -64,8 +64,8 @@ func TestTransportRetries(t *testing.T) {
 			fmt.Println("> GET", req.URL)
 			fmt.Printf("< %s (tries: %d)\n", bytes.TrimSpace(body), counter)
 
-			if counter != 3 {
-				t.Errorf("Unexpected number of retries, want=3, got=%d", counter)
+			if counter != 4 {
+				t.Errorf("Unexpected number of attempts, want=4, got=%d", counter)
 			}
 		})
 	}

estransport/estransport_internal_test.go

@@ -529,7 +529,7 @@ func TestTransportPerformRetries(t *testing.T) {
 			t.Fatalf("Unexpected error: %s", err)
 		}
 
-		if i != numReqs {
+		if i != numReqs+1 {
 			t.Errorf("Unexpected number of requests, want=%d, got=%d", numReqs, i)
 		}
 
@@ -575,7 +575,8 @@ func TestTransportPerformRetries(t *testing.T) {
 			t.Errorf("Unexpected response: %+v", res)
 		}
 
-		if i != numReqs {
+		// Should be initial HTTP request + 3 retries
+		if i != numReqs+1 {
 			t.Errorf("Unexpected number of requests, want=%d, got=%d", numReqs, i)
 		}
 	})
@@ -604,8 +605,8 @@ func TestTransportPerformRetries(t *testing.T) {
 		}
 		_ = res
 
-		if n := len(bodies); n != 3 {
-			t.Fatalf("expected 3 requests, got %d", n)
+		if n := len(bodies); n != 4 {
+			t.Fatalf("expected 4 requests, got %d", n)
 		}
 		for i, body := range bodies {
 			if body != "FOOBAR" {
@@ -674,14 +675,15 @@ func TestTransportPerformRetries(t *testing.T) {
 	t.Run("Delay the retry with a backoff function", func(t *testing.T) {
 		var (
 			i                int
-			numReqs          = 3
+			numReqs          = 4
 			start            = time.Now()
-			expectedDuration = time.Duration(numReqs*100) * time.Millisecond
+			expectedDuration = time.Duration((numReqs-1)*100) * time.Millisecond
 		)
 
 		u, _ := url.Parse("http://foo.bar")
 		tp, _ := New(Config{
-			URLs: []*url.URL{u, u, u},
+			MaxRetries: numReqs,
+			URLs:       []*url.URL{u, u, u},
 			Transport: &mockTransp{
 				RoundTripFunc: func(req *http.Request) (*http.Response, error) {
 					i++
@@ -796,3 +798,73 @@ func TestMetaHeader(t *testing.T) {
 		}
 	})
 }
+
+func TestMaxRetries(t *testing.T) {
+	tests := []struct {
+		name              string
+		maxRetries        int
+		disableRetry      bool
+		expectedCallCount int
+	}{
+		{
+			name:              "MaxRetries Active set to default",
+			disableRetry:      false,
+			expectedCallCount: 4,
+		},
+		{
+			name:              "MaxRetries Active set to 1",
+			maxRetries:        1,
+			disableRetry:      false,
+			expectedCallCount: 2,
+		},
+		{
+			name:              "Max Retries Active set to 2",
+			maxRetries:        2,
+			disableRetry:      false,
+			expectedCallCount: 3,
+		},
+		{
+			name:              "Max Retries Active set to 3",
+			maxRetries:        3,
+			disableRetry:      false,
+			expectedCallCount: 4,
+		},
+		{
+			name:              "MaxRetries Inactive set to 0",
+			maxRetries:        0,
+			disableRetry:      true,
+			expectedCallCount: 1,
+		},
+		{
+			name:              "MaxRetries Inactive set to 3",
+			maxRetries:        3,
+			disableRetry:      true,
+			expectedCallCount: 1,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var callCount int
+			c, _ := New(Config{
+				URLs: []*url.URL{{}},
+				Transport: &mockTransp{
+					RoundTripFunc: func(req *http.Request) (*http.Response, error) {
+						callCount++
+						return &http.Response{
+							StatusCode: http.StatusBadGateway,
+							Status:     "MOCK",
+						}, nil
+					},
+				},
+				MaxRetries:   test.maxRetries,
+				DisableRetry: test.disableRetry,
+			})
+
+			c.Perform(&http.Request{URL: &url.URL{}, Header: make(http.Header)}) // errcheck ignore
+
+			if test.expectedCallCount != callCount {
+				t.Errorf("Bad retry call count, got : %d, want : %d", callCount, test.expectedCallCount)
+			}
+		})
+	}
+}