Skip to content

Commit

Permalink
Merge branch 'main' into noretry-header-cert
Browse files Browse the repository at this point in the history
  • Loading branch information
manicminer committed May 9, 2024
2 parents a1a8ab8 + 4fb315e commit eb08cce
Show file tree
Hide file tree
Showing 14 changed files with 370 additions and 91 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/actionlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
actionlint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
- uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5
- name: "Check GitHub workflow files"
uses: docker://docker.mirror.hashicorp.services/rhysd/actionlint:latest
with:
Expand Down
46 changes: 0 additions & 46 deletions .github/workflows/go-retryablehttp.yml

This file was deleted.

23 changes: 23 additions & 0 deletions .github/workflows/pr-gofmt.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Go format check
on:
pull_request:
types: ['opened', 'synchronize']

jobs:
run-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5

- uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
with:
go-version-file: ./.go-version

- name: Run go format
run: |-
files=$(gofmt -s -l .)
if [ -n "$files" ]; then
echo >&2 "The following file(s) are not gofmt compliant:"
echo >&2 "$files"
exit 1
fi
17 changes: 17 additions & 0 deletions .github/workflows/pr-unit-tests-1.19.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: Unit tests (Go 1.19)
on:
pull_request:
types: ['opened', 'synchronize']

jobs:
run-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5

- uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
with:
go-version: 1.19

- name: Run unit tests
run: make test
17 changes: 17 additions & 0 deletions .github/workflows/pr-unit-tests-1.20.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: Unit tests (Go 1.20+)
on:
pull_request:
types: ['opened', 'synchronize']

jobs:
run-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5

- uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
with:
go-version: 1.22

- name: Run unit tests
run: make test
1 change: 1 addition & 0 deletions .go-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1.22.2
16 changes: 12 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
## 0.7.6 (Unreleased)

ENHANCEMENTS:

- client: support a `RetryPrepare` function for modifying the request before retrying (#216)
- client: support HTTP-date values for `Retry-After` header value (#138)
- client: avoid reading entire body when the body is a `*bytes.Reader` (#197)

## 0.7.5 (Nov 8, 2023)

BUG FIXES
BUG FIXES:

- client: fixes an issue where the request body is not preserved on temporary redirects or re-established HTTP/2 connections [GH-207]
- client: fixes an issue where the request body is not preserved on temporary redirects or re-established HTTP/2 connections (#207)

## 0.7.4 (Jun 6, 2023)

BUG FIXES
BUG FIXES:

- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP/2 [GH-194]
- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP/2 (#194)

## 0.7.3 (May 15, 2023)

Expand Down
2 changes: 1 addition & 1 deletion CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1 +1 @@
* @hashicorp/release-engineering
* @hashicorp/go-retryablehttp-maintainers
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ default: test

test:
go vet ./...
go test -race ./...
go test -v -race ./...

updatedeps:
go get -f -t -u ./...
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,4 @@ standardClient := retryClient.StandardClient() // *http.Client
```

For more usage and examples see the
[godoc](http://godoc.org/github.com/hashicorp/go-retryablehttp).
[pkg.go.dev](https://pkg.go.dev/github.com/hashicorp/go-retryablehttp).
91 changes: 70 additions & 21 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"context"
"fmt"
"io"
"io/ioutil"
"log"
"math"
"math/rand"
Expand Down Expand Up @@ -62,6 +61,10 @@ var (
// limit the size we consume to respReadLimit.
respReadLimit = int64(4096)

// timeNow sets the function that returns the current time.
// This defaults to time.Now. Changes to this should only be done in tests.
timeNow = time.Now

// A regular expression to match the error returned by net/http when the
// configured number of redirects is exhausted. This error isn't typed
// specifically so we resort to matching on the error string.
Expand Down Expand Up @@ -252,29 +255,27 @@ func getBodyReaderAndContentLength(rawBody interface{}) (ReaderFunc, int64, erro
// deal with it seeking so want it to match here instead of the
// io.ReadSeeker case.
case *bytes.Reader:
buf, err := ioutil.ReadAll(body)
if err != nil {
return nil, 0, err
}
snapshot := *body
bodyReader = func() (io.Reader, error) {
return bytes.NewReader(buf), nil
r := snapshot
return &r, nil
}
contentLength = int64(len(buf))
contentLength = int64(body.Len())

// Compat case
case io.ReadSeeker:
raw := body
bodyReader = func() (io.Reader, error) {
_, err := raw.Seek(0, 0)
return ioutil.NopCloser(raw), err
return io.NopCloser(raw), err
}
if lr, ok := raw.(LenReader); ok {
contentLength = int64(lr.Len())
}

// Read all in so we can reset
case io.Reader:
buf, err := ioutil.ReadAll(body)
buf, err := io.ReadAll(body)
if err != nil {
return nil, 0, err
}
Expand Down Expand Up @@ -397,6 +398,9 @@ type Backoff func(min, max time.Duration, attemptNum int, resp *http.Response) t
// attempted. If overriding this, be sure to close the body if needed.
type ErrorHandler func(resp *http.Response, err error, numTries int) (*http.Response, error)

// PrepareRetry is called before retry operation. It can be used for example to re-sign the request
type PrepareRetry func(req *http.Request) error

// Client is used to make HTTP requests. It adds additional functionality
// like automatic retries to tolerate minor outages.
type Client struct {
Expand Down Expand Up @@ -425,6 +429,9 @@ type Client struct {
// ErrorHandler specifies the custom error handler to use, if any
ErrorHandler ErrorHandler

// PrepareRetry can prepare the request for retry operation, for example re-sign it
PrepareRetry PrepareRetry

loggerInit sync.Once
clientInit sync.Once
}
Expand Down Expand Up @@ -544,10 +551,8 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) {
func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response) time.Duration {
if resp != nil {
if resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode == http.StatusServiceUnavailable {
if s, ok := resp.Header["Retry-After"]; ok {
if sleep, err := strconv.ParseInt(s[0], 10, 64); err == nil {
return time.Second * time.Duration(sleep)
}
if sleep, ok := parseRetryAfterHeader(resp.Header["Retry-After"]); ok {
return sleep
}
}
}
Expand All @@ -560,6 +565,41 @@ func DefaultBackoff(min, max time.Duration, attemptNum int, resp *http.Response)
return sleep
}

// parseRetryAfterHeader parses the Retry-After header and returns the
// delay duration according to the spec: https://httpwg.org/specs/rfc7231.html#header.retry-after
// The bool returned will be true if the header was successfully parsed.
// Otherwise, the header was either not present, or was not parseable according to the spec.
//
// Retry-After headers come in two flavors: Seconds or HTTP-Date
//
// Examples:
// * Retry-After: Fri, 31 Dec 1999 23:59:59 GMT
// * Retry-After: 120
func parseRetryAfterHeader(headers []string) (time.Duration, bool) {
if len(headers) == 0 || headers[0] == "" {
return 0, false
}
header := headers[0]
// Retry-After: 120
if sleep, err := strconv.ParseInt(header, 10, 64); err == nil {
if sleep < 0 { // a negative sleep doesn't make sense
return 0, false
}
return time.Second * time.Duration(sleep), true
}

// Retry-After: Fri, 31 Dec 1999 23:59:59 GMT
retryTime, err := time.Parse(time.RFC1123, header)
if err != nil {
return 0, false
}
if until := retryTime.Sub(timeNow()); until > 0 {
return until, true
}
// date is in the past
return 0, true
}

// LinearJitterBackoff provides a callback for Client.Backoff which will
// perform linear backoff based on the attempt number and with jitter to
// prevent a thundering herd.
Expand Down Expand Up @@ -587,13 +627,13 @@ func LinearJitterBackoff(min, max time.Duration, attemptNum int, resp *http.Resp
}

// Seed rand; doing this every time is fine
rand := rand.New(rand.NewSource(int64(time.Now().Nanosecond())))
source := rand.New(rand.NewSource(int64(time.Now().Nanosecond())))

// Pick a random number that lies somewhere between the min and max and
// multiply by the attemptNum. attemptNum starts at zero so we always
// increment here. We first get a random percentage, then apply that to the
// difference between min and max, and add to min.
jitter := rand.Float64() * float64(max-min)
jitter := source.Float64() * float64(max-min)
jitterMin := int64(jitter) + int64(min)
return time.Duration(jitterMin * int64(attemptNum))
}
Expand Down Expand Up @@ -627,10 +667,10 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
var resp *http.Response
var attempt int
var shouldRetry bool
var doErr, respErr, checkErr error
var doErr, respErr, checkErr, prepareErr error

for i := 0; ; i++ {
doErr, respErr = nil, nil
doErr, respErr, prepareErr = nil, nil, nil
attempt++

// Always rewind the request body when non-nil.
Expand All @@ -643,7 +683,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
if c, ok := body.(io.ReadCloser); ok {
req.Body = c
} else {
req.Body = ioutil.NopCloser(body)
req.Body = io.NopCloser(body)
}
}

Expand Down Expand Up @@ -737,17 +777,26 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
// without racing against the closeBody call in persistConn.writeLoop.
httpreq := *req.Request
req.Request = &httpreq

if c.PrepareRetry != nil {
if err := c.PrepareRetry(req.Request); err != nil {
prepareErr = err
break
}
}
}

// this is the closest we have to success criteria
if doErr == nil && respErr == nil && checkErr == nil && !shouldRetry {
if doErr == nil && respErr == nil && checkErr == nil && prepareErr == nil && !shouldRetry {
return resp, nil
}

defer c.HTTPClient.CloseIdleConnections()

var err error
if checkErr != nil {
if prepareErr != nil {
err = prepareErr
} else if checkErr != nil {
err = checkErr
} else if respErr != nil {
err = respErr
Expand Down Expand Up @@ -779,7 +828,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
// Try to read the response body so we can reuse this connection.
func (c *Client) drainBody(body io.ReadCloser) {
defer body.Close()
_, err := io.Copy(ioutil.Discard, io.LimitReader(body, respReadLimit))
_, err := io.Copy(io.Discard, io.LimitReader(body, respReadLimit))
if err != nil {
if c.logger() != nil {
switch v := c.logger().(type) {
Expand Down
Loading

0 comments on commit eb08cce

Please sign in to comment.