Restore original transport for each operation.

Signed-off-by: David Calavera <[email protected]>

Author: calavera

go/porcelain/http/http.go

@@ -44,29 +44,33 @@ func (t *RetryableTransport) Submit(op *runtime.ClientOperation) (interface{}, e
 
 	op.Client = client
 
-	return t.tr.Submit(op)
+	res, err := t.tr.Submit(op)
+
+	// restore original transport
+	op.Client.Transport = transport
+
+	return res, err
 }
 
-func (tr *retryableRoundTripper) RoundTrip(req *http.Request) (resp *http.Response, err error) {
+func (t *retryableRoundTripper) RoundTrip(req *http.Request) (resp *http.Response, err error) {
 	rr := autorest.NewRetriableRequest(req)
 
-	// Increment to add the first call (attempts denotes number of retries)
-	attempts := tr.attempts
-	attempts++
-	for attempt := 0; attempt < attempts; attempt++ {
+	for attempt := 0; attempt < t.attempts; attempt++ {
 		err = rr.Prepare()
 		if err != nil {
 			return resp, err
 		}
 
-		resp, err = tr.tr.RoundTrip(rr.Request())
+		resp, err = t.tr.RoundTrip(rr.Request())
 
 		if err != nil || resp.StatusCode != http.StatusTooManyRequests {
 			return resp, err
 		}
 
-		if !delayWithRateLimit(resp, req.Cancel) {
-			return resp, err
+		if attempt+1 < t.attempts { // ignore delay check in the last request attempt
+			if !delayWithRateLimit(resp, req.Cancel) {
+				return resp, err
+			}
 		}
 	}
 

go/porcelain/http/http_test.go

@@ -64,3 +64,81 @@ func TestRetryableTransport(t *testing.T) {
 	actual := res.(string)
 	require.EqualValues(t, "ok", actual)
 }
+
+func TestRetryableTransportExceedsMaxAttempts(t *testing.T) {
+	attempts := 0
+	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		attempts++
+		reset := fmt.Sprintf("%d", time.Now().Add(1*time.Second).Unix())
+		rw.Header().Set("X-RateLimit-Reset", reset)
+		rw.WriteHeader(http.StatusTooManyRequests)
+		_, _ = rw.Write([]byte("rate limited"))
+	}))
+	defer server.Close()
+
+	rwrtr := runtime.ClientRequestWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error {
+		return nil
+	})
+
+	hu, _ := url.Parse(server.URL)
+	rt := NewRetryableTransport(httptransport.New(hu.Host, "/", []string{"http"}), 2)
+
+	_, err := rt.Submit(&runtime.ClientOperation{
+		ID:          "getSites",
+		Method:      "GET",
+		PathPattern: "/",
+		Params:      rwrtr,
+		Reader: runtime.ClientResponseReaderFunc(func(response runtime.ClientResponse, consumer runtime.Consumer) (interface{}, error) {
+			if response.Code() == 200 {
+				var result string
+				if err := consumer.Consume(response.Body(), &result); err != nil {
+					return nil, err
+				}
+				return result, nil
+			}
+			return nil, errors.New("Generic error")
+		}),
+	})
+
+	require.Error(t, err)
+	require.Equal(t, 2, attempts)
+}
+
+func TestRetryableWithDifferentError(t *testing.T) {
+	attempts := 0
+
+	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		attempts++
+
+		rw.WriteHeader(http.StatusNotFound)
+		_, _ = rw.Write([]byte("not found"))
+	}))
+	defer server.Close()
+
+	rwrtr := runtime.ClientRequestWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error {
+		return nil
+	})
+
+	hu, _ := url.Parse(server.URL)
+	rt := NewRetryableTransport(httptransport.New(hu.Host, "/", []string{"http"}), 2)
+
+	_, err := rt.Submit(&runtime.ClientOperation{
+		ID:          "getSites",
+		Method:      "GET",
+		PathPattern: "/",
+		Params:      rwrtr,
+		Reader: runtime.ClientResponseReaderFunc(func(response runtime.ClientResponse, consumer runtime.Consumer) (interface{}, error) {
+			if response.Code() == 200 {
+				var result string
+				if err := consumer.Consume(response.Body(), &result); err != nil {
+					return nil, err
+				}
+				return result, nil
+			}
+			return nil, errors.New("Generic error")
+		}),
+	})
+
+	require.Error(t, err)
+	require.Equal(t, 1, attempts)
+}