Retry after context cancel

[zdebra]

Hello I have a use case where I'd like to impose a timeout on my http client with retries. This means that http client's context cancellation should be retryable action. According to my testing it is not:

func TestRetryableHTTP(t *testing.T) {
	client := retryablehttp.NewClient()
	client.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) {
		if ctx.Err() != nil {
			return true, ctx.Err()
		}
		return retryablehttp.DefaultRetryPolicy(ctx, resp, err)
	}

	attempts := 0
	handlerWithDelay := func(delay time.Duration) http.HandlerFunc {
		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			attempts++
			ticker := time.NewTicker(delay)
			select {
			case <-r.Context().Done():
				t.Log("connection droppped from client")
				return
			case <-ticker.C:
			}
			t.Log("responding 200 OK")
			w.WriteHeader(http.StatusOK)
		})
	}
	ts := httptest.NewServer(handlerWithDelay(10 * time.Second))
	defer ts.Close()

	ctx, cancelFn := context.WithTimeout(context.Background(), time.Second)
	defer cancelFn()
	req, err := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL, nil)
	require.NoError(t, err)

	retReq, err := retryablehttp.FromRequest(req)
	require.NoError(t, err)
	_, err = client.Do(retReq)
	assert.NoError(t, err)

	assert.Equal(t, 5, attempts) // 4 retries + first call

}

Test output:

=== RUN   TestRetryableHTTP
2022/05/16 19:18:41 [DEBUG] GET http://127.0.0.1:49948
2022/05/16 19:18:42 [ERR] GET http://127.0.0.1:49948 request failed: Get "http://127.0.0.1:49948": context deadline exceeded
2022/05/16 19:18:42 [DEBUG] GET http://127.0.0.1:49948: retrying in 1s (4 left)
    sanehttp_test.go:168: connection droppped from client
    sanehttp_test.go:187:
        	Error Trace:	sanehttp_test.go:187
        	Error:      	Received unexpected error:
        	            	context deadline exceeded
        	Test:       	TestRetryableHTTP
    sanehttp_test.go:189:
        	Error Trace:	sanehttp_test.go:189
        	Error:      	Not equal:
        	            	expected: 5
        	            	actual  : 1
        	Test:       	TestRetryableHTTP
--- FAIL: TestRetryableHTTP (1.01s)

You can see there was only 1 attempt and no retries.

I have also tried to use the http client Timeout field instead of context cancellation but it yields the same result.

I was checking the source code and it looks like you are not retrying on errors from client.Do() which is the case. Do you know if there is a way how to make my use case work?

Thank you.

[exoson]

Also interested in the possibility of having per retry timeouts. Preferably through context so wouldn't need a custom HTTP client for each request with different timeouts.

[fideloper]

Looks like this is related to

go-retryablehttp/client.go

Line 695 in 493aa4c

case <-req.Context().Done():

Snippet near that line:

timer := time.NewTimer(wait)
select {
case <-req.Context().Done():
	timer.Stop()
	c.HTTPClient.CloseIdleConnections()
	return nil, req.Context().Err()
case <-timer.C:
}

Where the request context trumps the CheckRetry logic. That makes sense on the surface, but it also makes it impossible (in its current state) to retry based on client timeout settings.