puerkitobio / rehttp

Package rehttp implements a Go HTTP transport that handles retries.

rehttp

Package rehttp implements an HTTP Transport (an http.RoundTripper) that handles retries. See the godoc for details.

Please note that rehttp requires Go1.6+, because it uses the http.Request.Cancel field to check for cancelled requests. It should work on Go1.5, but only if there is no timeout set on the *http.Client. Go's stdlib will return an error on the first request if that's the case, because it requires a RoundTripper that implements the (now deprecated in Go1.6) CancelRequest method.

On Go1.7+, it uses the context returned by http.Request.Context to check for cancelled requests.

Installation

$ go get github.com/PuerkitoBio/rehttp

License

The BSD 3-Clause license.

rehttp's Issues

Semver please

Can we have the current master tagged as 0.1.0?

A few feedback items

Hi there, I think this is a great project and using the RoundTripper interface makes it easy to add to any golang HTTP client with no refactor required. I recently evaluated options for implementing HTTP retries at the company I work for, and ended up rolling my own internal package that borrows many ideas from this package. I wanted to share my thoughts and you can feel free to choose any that make sense to you to incorporate.

• (minor one) the Temporary() interface is now deprecated. I used Timeout() instead and it's mostly equivalent.
• I found myself forgetting to take the number of retries into account in my RetryFn, so I added a maxRetries field to the transport. I also didn't provide RetryAny, RetryAll, RetryMaxRetries, etc because they felt redundant at that point.
• Per-attempt timeouts were a dealbreaker to me. Example: attempt 1 times out after 10s, delays for 1s, attempt 2 times out after 10s, delays for 2s, attempt 3 succeeds after 5s for a total time of 28s. As implemented, this library would instead wait on attempt 1 until it succeeds, fails, or the overall timeout (whether on the request context or the http client) is exhausted, and no further attempts will be made. I did this by saving a reference to the original request context at the beginning of RoundTrip(), then creating a new attempt context with the configured per-attempt timeout for each iteration of the loop. It is possible to have both a per-attempt timeout and an overall timeout, but since it may not behave as most devs expect I call out in my docs that it's recommended to just use a per-attempt timeout.
• Being able to override configuration for a single endpoint was another dealbreaker for me. I generally instantiate a single HTTP client for each dependency I'm calling. In general, the different endpoints I call have the same retry policy and per-attempt timeout, but there are always exceptions. I accomplished this by adding a bunch of context setter helper functions such as SetDelayFnOnContext(ctx context.Context, fn DelayFn) context.Context. A library user just has to use request.WithContext() with the returned context. Every option that can be set at instantiation can be overridden by a context key. This makes for more code in RoundTrip(), but it's delightful to use.
• Rather than a single NewTransport() constructor function, I opted for the options pattern. It makes configuring multiple fields on the transport easier.
• Some teams requested a generic hook function to modify the request before it goes out. Main use cases were to re-sign the JWT header with a fresh exp or to look up the IP address to hit based on an SRV lookup (which may change between attempts). Both of those things could be accomplished in an inner RoundTripper, but I added an optional BeforeAttemptFn() to make migration easier for their existing codebases.

I realize some of these changes would be breaking to existing users of this library, but I wanted to share my thought process and the lessons I learned having spent a fair amount of time with this topic. Feel free to close this issue out or direct me to migrate any selection of the above bullet points you feel make sense to their own issues. And thanks for sharing this package in the first place.

Support retrying timeout errors

It would be really useful if there was a RetryTimeout method which retries the request in the case of a timeout error. The reason for this is because some public APIs I request with rehttp tend to hang forever occasionally whilst responding really fast the majority of the time. In this case I'd like to cancel the running request after a reasonably short timeout and retry it.

`rand.Rand` is not safe for concurrent use

I recently learned that random sources from math/rand are not safe for concurrent use. This is relevant to this package as the global PRNG (found in rehttp.go) does not have a mutex to guard against possible runtime panics.

Relevant note from the docs:

Unlike the default Source used by top-level functions, this source is not safe for concurrent use by multiple goroutines.

I have not attempted to reproduce a panic in this package directly, but I have found that they do occur when generating pseudorandom numbers concurrently:

package main

import (
	"fmt"
	"math/rand"
	"sync"
	"time"
)

var src = rand.New(rand.NewSource(time.Now().UnixNano()))

func main() {
	var wg sync.WaitGroup

  for i := 0; i < 10; i++ {
		wg.Add(1)
		go func() {
			genRandom()
			wg.Done()
		}()
	}

	wg.Wait()	
  fmt.Println("done")
}

func genRandom() {
	for i := 0; i < 100_000_000; i++ {
		src.Intn(100)
	}
}

Contemplate not buffering data if Body is NopCloser or some in-memory type?

It seems like if you could figure out whether the Body supported multiple reads (for example, an in-memory buffer), would save some copies.

Unfortunately, it seems that NopCloser isn't exported, but maybe you could export your own wrapper around it that could be used as an indicator?

Cannot retry err [...] after Request.Body was written; define Request.GetBody to avoid this error (affects h2 goaway and 307/308 redirects etc)

The go-retryablehttp package was recently updated to handle retries with a body (hashicorp/go-retryablehttp#207 to fix hashicorp/go-retryablehttp#121).

A similar fix is needed for rehttp.