Propogate Do() HTTP error up when retries are exceeded

Add propagation of the HTTP request/response error up the call stack when the
number of retries are exceeded. Because the ErrorHandler does not receive a copy
of the request, it's impossible to use the ErrorHandler to generate an enhanced
version of the default Retries Exceeded error message.

This change takes the non-API breaking approach to solving this, by including
the error string in the returned error (if an error has occurred).

Because the error string returned from `Do()` uses `%w`, this change requires Go
1.13+.

Fixes #69

.travis.yml

@@ -3,7 +3,8 @@ sudo: false
 language: go
 
 go:
-  - 1.12.4
+  - 1.13.x
+  - 1.14.x
 
 branches:
   only:

client.go

@@ -425,7 +425,7 @@ func DefaultRetryPolicy(ctx context.Context, resp *http.Response, err error) (bo
 	// errors and may relate to outages on the server side. This will catch
 	// invalid response codes as well, like 0 and 999.
 	if resp.StatusCode == 0 || (resp.StatusCode >= 500 && resp.StatusCode != 501) {
-		return true, nil
+		return true, fmt.Errorf("unexpected HTTP status %s", resp.Status)
 	}
 
 	return false, nil
@@ -506,9 +506,12 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 	}
 
 	var resp *http.Response
+	var attempt int
 	var err error
 
 	for i := 0; ; i++ {
+		attempt++
+
 		var code int // HTTP response code
 
 		// Always rewind the request body when non-nil.
@@ -569,12 +572,12 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 		}
 
 		// Now decide if we should continue.
+		if checkErr != nil {
+			err = checkErr
+		}
+
 		if !checkOK {
-			if checkErr != nil {
-				err = checkErr
-			}
-			c.HTTPClient.CloseIdleConnections()
-			return resp, err
+			break
 		}
 
 		// We do this before drainBody because there's no need for the I/O if
@@ -610,19 +613,24 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 		}
 	}
 
+	if err == nil {
+		return resp, nil
+	}
+
+	defer c.HTTPClient.CloseIdleConnections()
+
 	if c.ErrorHandler != nil {
-		c.HTTPClient.CloseIdleConnections()
-		return c.ErrorHandler(resp, err, c.RetryMax+1)
+		return c.ErrorHandler(resp, err, attempt)
 	}
 
 	// By default, we close the response body and return an error without
 	// returning the response
 	if resp != nil {
-		resp.Body.Close()
+		c.drainBody(resp.Body)
 	}
-	c.HTTPClient.CloseIdleConnections()
-	return nil, fmt.Errorf("%s %s giving up after %d attempts",
-		req.Method, req.URL, c.RetryMax+1)
+
+	return nil, fmt.Errorf("%s %s giving up after %d attempt(s): %w",
+		req.Method, req.URL, attempt, err)
 }
 
 // Try to read the response body so we can reuse this connection.

client_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
@@ -462,8 +463,10 @@ func TestClient_RequestWithContext(t *testing.T) {
 		t.Fatalf("CheckRetry called %d times, expected 1", called)
 	}
 
-	if err != context.Canceled {
-		t.Fatalf("Expected context.Canceled err, got: %v", err)
+	e := fmt.Sprintf("GET %s giving up after 1 attempt(s): %s", ts.URL, context.Canceled.Error())
+
+	if err.Error() != e {
+		t.Fatalf("Expected err to contain %s, got: %v", e, err)
 	}
 }
 
@@ -493,7 +496,7 @@ func TestClient_CheckRetry(t *testing.T) {
 		t.Fatalf("CheckRetry called %d times, expected 1", called)
 	}
 
-	if err != retryErr {
+	if err.Error() != fmt.Sprintf("GET %s giving up after 2 attempt(s): retryError", ts.URL) {
 		t.Fatalf("Expected retryError, got:%v", err)
 	}
 

go.mod

@@ -1,5 +1,7 @@
 module github.com/hashicorp/go-retryablehttp
 
+go 1.13
+
 require (
 	github.com/hashicorp/go-cleanhttp v0.5.1
 	github.com/hashicorp/go-hclog v0.9.2