Skip to content

Commit

Permalink
Use cleanhttp.DefaultPooledConnection and enforce cleanup (#57)
Browse files Browse the repository at this point in the history
* Use cleanhttp.DefaultPooledConnection and enforce cleanup

This allows reuse of the existing connection on retries, since they're
going to the same host, but we explicitly close idle connections to
ensure they don't stick around forever after the series of requests is
done.

Fixes #44

* Fix merge
  • Loading branch information
jefferai committed Aug 26, 2019
1 parent 6bb8533 commit 7ccf08b
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
11 changes: 10 additions & 1 deletion client.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ type Client struct {
// NewClient creates a new Client with default settings.
func NewClient() *Client {
return &Client{
HTTPClient: cleanhttp.DefaultClient(),
HTTPClient: cleanhttp.DefaultPooledClient(),
RetryWaitMin: defaultRetryWaitMin,
RetryWaitMax: defaultRetryWaitMax,
RetryMax: defaultRetryMax,
Expand Down Expand Up @@ -418,6 +418,10 @@ func PassthroughErrorHandler(resp *http.Response, err error, _ int) (*http.Respo

// Do wraps calling an HTTP method with retries.
func (c *Client) Do(req *Request) (*http.Response, error) {
if c.HTTPClient == nil {
c.HTTPClient = cleanhttp.DefaultPooledClient()
}

if c.logger() != nil {
switch v := c.logger().(type) {
case Logger:
Expand All @@ -437,6 +441,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
if req.body != nil {
body, err := req.body()
if err != nil {
c.HTTPClient.CloseIdleConnections()
return resp, err
}
if c, ok := body.(io.ReadCloser); ok {
Expand Down Expand Up @@ -496,6 +501,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
if checkErr != nil {
err = checkErr
}
c.HTTPClient.CloseIdleConnections()
return resp, err
}

Expand Down Expand Up @@ -526,12 +532,14 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
}
select {
case <-req.Context().Done():
c.HTTPClient.CloseIdleConnections()
return nil, req.Context().Err()
case <-time.After(wait):
}
}

if c.ErrorHandler != nil {
c.HTTPClient.CloseIdleConnections()
return c.ErrorHandler(resp, err, c.RetryMax+1)
}

Expand All @@ -540,6 +548,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
if resp != nil {
resp.Body.Close()
}
c.HTTPClient.CloseIdleConnections()
return nil, fmt.Errorf("%s %s giving up after %d attempts",
req.Method, req.URL, c.RetryMax+1)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/hashicorp/go-retryablehttp

require (
github.com/hashicorp/go-cleanhttp v0.5.0
github.com/hashicorp/go-cleanhttp v0.5.1
github.com/hashicorp/go-hclog v0.9.2
)
7 changes: 5 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/hashicorp/go-cleanhttp v0.5.0 h1:wvCrVc9TjDls6+YGAF2hAifE1E5U1+b4tH6KdvN3Gig=
github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM=
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI=
github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=

0 comments on commit 7ccf08b

Please sign in to comment.