Promote the use of Timeout() over Temporary()

context_post17.go

@@ -1,3 +1,4 @@
+//go:build go1.7
 // +build go1.7
 
 package rehttp

context_pre17.go

@@ -1,3 +1,4 @@
+//go:build !go1.7
 // +build !go1.7
 
 package rehttp

rehttp.go

@@ -9,16 +9,17 @@
 //
 // The package offers common delay strategies as ready-made functions that
 // return a DelayFn:
-//     - ConstDelay(delay time.Duration) DelayFn
-//     - ExpJitterDelay(base, max time.Duration) DelayFn
+//   - ConstDelay(delay time.Duration) DelayFn
+//   - ExpJitterDelay(base, max time.Duration) DelayFn
 //
 // It also provides common retry helpers that return a RetryFn:
-//     - RetryIsErr(func(error) bool) RetryFn
-//     - RetryHTTPMethods(methods ...string) RetryFn
-//     - RetryMaxRetries(max int) RetryFn
-//     - RetryStatuses(statuses ...int) RetryFn
-//     - RetryStatusInterval(fromStatus, toStatus int) RetryFn
-//     - RetryTemporaryErr() RetryFn
+//   - RetryIsErr(func(error) bool) RetryFn
+//   - RetryHTTPMethods(methods ...string) RetryFn
+//   - RetryMaxRetries(max int) RetryFn
+//   - RetryStatuses(statuses ...int) RetryFn
+//   - RetryStatusInterval(fromStatus, toStatus int) RetryFn
+//   - RetryTimeoutErr() RetryFn
+//   - RetryTemporaryErr() RetryFn
 //
 // Those can be combined with RetryAny or RetryAll as needed. RetryAny
 // enables retries if any of the RetryFn return true, while RetryAll
@@ -173,6 +174,8 @@ func RetryIsErr(fn func(error) bool) RetryFn {
 // is a temporary error. A temporary error is one that implements
 // the Temporary() bool method. Most errors from the net package implement
 // this.
+// This interface was deprecated in go 1.18. Favor RetryTimeoutErr.
+// https://github.com/golang/go/issues/45729
 func RetryTemporaryErr() RetryFn {
 	return RetryIsErr(func(err error) bool {
 		if terr, ok := err.(temporaryer); ok {
@@ -182,6 +185,16 @@ func RetryTemporaryErr() RetryFn {
 	})
 }
 
+// RetryTimeoutErr returns a RetryFn that retries if the Attempt's error
+// is a timeout error. Before go 1.13, a timeout error is one that implements
+// the Timeout() bool method. Most errors from the net package implement this.
+// After go 1.13, a timeout error is one that implemts the net.Error interface
+// which includes both Timeout() and Temporary() to make it less likely to
+// falsely identify errors that occurred outside of the net package.
+func RetryTimeoutErr() RetryFn {
+	return RetryIsErr(isTimeoutErr)
+}
+
 // RetryStatusInterval returns a RetryFn that retries if the response's
 // status code is in the provided half-closed interval [fromStatus, toStatus)
 // (that is, it retries if fromStatus <= Response.StatusCode < toStatus, so
@@ -213,7 +226,7 @@ func RetryStatuses(statuses ...int) RetryFn {
 
 // RetryHTTPMethods returns a RetryFn that retries if the request's
 // HTTP method is one of the provided methods. It is meant to be used
-// in conjunction with another RetryFn such as RetryTemporaryErr combined
+// in conjunction with another RetryFn such as RetryTimeoutErr combined
 // using RetryAll, otherwise this function will retry any successful
 // request made with one of the provided methods.
 func RetryHTTPMethods(methods ...string) RetryFn {

rehttp_mock_test.go

@@ -3,6 +3,7 @@ package rehttp
 import (
 	"bytes"
 	"io"
+	"net"
 	"net/http"
 	"net/url"
 	"strings"
@@ -135,3 +136,27 @@ func TestMockClientRetryWithBody(t *testing.T) {
 	assert.Equal(t, 2, mock.Calls())
 	assert.Equal(t, []string{"hello", "hello"}, mock.Bodies())
 }
+
+func TestMockClientRetryTimeout(t *testing.T) {
+	retFn := func(att int, req *http.Request) (*http.Response, error) {
+		return nil, &net.OpError{
+			Err: timeoutErr{},
+		}
+	}
+	mock := &mockRoundTripper{t: t, retFn: retFn}
+
+	tr := NewTransport(mock, RetryAll(RetryMaxRetries(1), RetryTimeoutErr()), ConstDelay(0))
+
+	client := &http.Client{
+		Transport: tr,
+	}
+	_, err := client.Get("http://example.com")
+	if assert.NotNil(t, err) {
+		uerr, ok := err.(*url.Error)
+		require.True(t, ok)
+		assert.Equal(t, &net.OpError{
+			Err: timeoutErr{},
+		}, uerr.Err)
+	}
+	assert.Equal(t, 2, mock.Calls())
+}

rehttp_retryfn_test.go

@@ -2,6 +2,7 @@ package rehttp
 
 import (
 	"io"
+	"net"
 	"net/http"
 	"testing"
 	"time"
@@ -98,6 +99,15 @@ type tempErr struct{}
 func (t tempErr) Error() string   { return "temp error" }
 func (t tempErr) Temporary() bool { return true }
 
+type timeoutErr struct{}
+
+func (t timeoutErr) Error() string { return "temp error" }
+func (t timeoutErr) Timeout() bool { return true }
+
+var timeoutNetErr = net.OpError{
+	Err: timeoutErr{},
+}
+
 func TestRetryTemporaryErr(t *testing.T) {
 	cases := []struct {
 		retries int
@@ -119,6 +129,27 @@ func TestRetryTemporaryErr(t *testing.T) {
 	}
 }
 
+func TestRetryTimeoutErr(t *testing.T) {
+	cases := []struct {
+		retries int
+		err     error
+		att     int
+		want    bool
+	}{
+		{retries: 1, err: nil, att: 0, want: false},
+		{retries: 1, err: nil, att: 1, want: false},
+		{retries: 1, err: io.EOF, att: 0, want: false},
+		{retries: 1, err: &timeoutNetErr, att: 0, want: true},
+		{retries: 1, err: &timeoutNetErr, att: 1, want: false},
+	}
+
+	for i, tc := range cases {
+		fn := RetryAll(RetryMaxRetries(tc.retries), RetryTimeoutErr())
+		got := fn(Attempt{Index: tc.att, Error: tc.err})
+		assert.Equal(t, tc.want, got, "%d", i)
+	}
+}
+
 func TestRetryAll(t *testing.T) {
 	max := RetryMaxRetries(2)
 	status := RetryStatusInterval(500, 600)

timeouterr_post113.go

@@ -0,0 +1,18 @@
+//go:build go1.13
+// +build go1.13
+
+package rehttp
+
+import (
+	"errors"
+	"net"
+)
+
+func isTimeoutErr(err error) bool {
+	var neterr net.Error
+	if errors.As(err, &neterr) && neterr.Timeout() {
+		return true
+	}
+
+	return false
+}

timeouterr_pre113.go

@@ -0,0 +1,15 @@
+//go:build !go1.13
+// +build !go1.13
+
+package rehttp
+
+type timeouter interface {
+	Timeout() bool
+}
+
+func isTimeoutErr(err error) bool {
+	if terr, ok := err.(timeouter); ok {
+		return terr.Timeout()
+	}
+	return false
+}

timeouterr_test.go

@@ -0,0 +1,42 @@
+package rehttp
+
+import (
+	"errors"
+	"net"
+	"testing"
+
+	"golang.org/x/net/context"
+)
+
+func Test_isTimeoutErr(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{
+			name: "context timeouts should be retryable",
+			err:  context.DeadlineExceeded,
+			want: true,
+		},
+		{
+			name: "net op errors are true",
+			err: &net.OpError{
+				Err: timeoutErr{},
+			},
+			want: true,
+		},
+		{
+			name: "non-network related errors should not be retryable",
+			err:  errors.New("fake error"),
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isTimeoutErr(tt.err); got != tt.want {
+				t.Errorf("isTimeoutErr() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}