Change noRetryError for temporary errors

- Expose an `Unwrap` method to leverage go's standard unwrapping
- Let the error escape, such that hooks and end-users can access the
  `Temporary` information.
- Do not wrap if the error already exposes `Temporary`, to honour it.

client.go

@@ -879,14 +879,14 @@ func (c *Client) execute(req *Request) (*Response, error) {
 	// to modify the *resty.Request object
 	for _, f := range c.udBeforeRequest {
 		if err = f(c, req); err != nil {
-			return nil, wrapNoRetryErr(err)
+			return nil, err
 		}
 	}
 
 	// resty middlewares
 	for _, f := range c.beforeRequest {
 		if err = f(c, req); err != nil {
-			return nil, wrapNoRetryErr(err)
+			return nil, err
 		}
 	}
 
@@ -897,12 +897,12 @@ func (c *Client) execute(req *Request) (*Response, error) {
 	// call pre-request if defined
 	if c.preReqHook != nil {
 		if err = c.preReqHook(c, req.RawRequest); err != nil {
-			return nil, wrapNoRetryErr(err)
+			return nil, err
 		}
 	}
 
 	if err = requestLogger(c, req); err != nil {
-		return nil, wrapNoRetryErr(err)
+		return nil, err
 	}
 
 	req.RawRequest.Body = newRequestBodyReleaser(req.RawRequest.Body, req.bodyBuf)
@@ -917,7 +917,7 @@ func (c *Client) execute(req *Request) (*Response, error) {
 
 	if err != nil || req.notParseResponse || c.notParseResponse {
 		response.setReceivedAt()
-		return response, err
+		return response, wrapTemporaryError(err)
 	}
 
 	if !req.isSaveResponse {
@@ -930,15 +930,15 @@ func (c *Client) execute(req *Request) (*Response, error) {
 				body, err = gzip.NewReader(body)
 				if err != nil {
 					response.setReceivedAt()
-					return response, err
+					return response, wrapTemporaryError(err)
 				}
 				defer closeq(body)
 			}
 		}
 
 		if response.body, err = ioutil.ReadAll(body); err != nil {
 			response.setReceivedAt()
-			return response, err
+			return response, wrapTemporaryError(err)
 		}
 
 		response.size = int64(len(response.body))
@@ -953,7 +953,7 @@ func (c *Client) execute(req *Request) (*Response, error) {
 		}
 	}
 
-	return response, wrapNoRetryErr(err)
+	return response, err
 }
 
 // getting TLS client config if not exists then create one

request.go

@@ -743,7 +743,7 @@ func (r *Request) Execute(method, url string) (*Response, error) {
 	if r.SRV != nil {
 		_, addrs, err = net.LookupSRV(r.SRV.Service, "tcp", r.SRV.Domain)
 		if err != nil {
-			r.client.onErrorHooks(r, nil, err)
+			r.client.onErrorHooks(r, resp, err)
 			return nil, err
 		}
 	}
@@ -753,9 +753,8 @@ func (r *Request) Execute(method, url string) (*Response, error) {
 
 	if r.client.RetryCount == 0 {
 		r.Attempt = 1
-		resp, err = r.client.execute(r)
-		r.client.onErrorHooks(r, resp, unwrapNoRetryErr(err))
-		return resp, unwrapNoRetryErr(err)
+		r.client.onErrorHooks(r, resp, err)
+		return r.client.execute(r)
 	}
 
 	err = Backoff(
@@ -778,9 +777,9 @@ func (r *Request) Execute(method, url string) (*Response, error) {
 		RetryHooks(r.client.RetryHooks),
 	)
 
-	r.client.onErrorHooks(r, resp, unwrapNoRetryErr(err))
+	r.client.onErrorHooks(r, resp, err)
 
-	return resp, unwrapNoRetryErr(err)
+	return resp, err
 }
 
 //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾

retry.go

@@ -111,11 +111,10 @@ func Backoff(operation func() (*Response, error), options ...Option) error {
 			return err
 		}
 
-		err1 := unwrapNoRetryErr(err)           // raw error, it used for return users callback.
-		needsRetry := err != nil && err == err1 // retry on a few operation errors by default
+		needsRetry := isTemporaryError(err) // retry on temporary errors by default
 
 		for _, condition := range opts.retryConditions {
-			needsRetry = condition(resp, err1)
+			needsRetry = condition(resp, err)
 			if needsRetry {
 				break
 			}

retry_test.go

@@ -22,7 +22,7 @@ func TestBackoffSuccess(t *testing.T) {
 	retryErr := Backoff(func() (*Response, error) {
 		externalCounter++
 		if externalCounter < attempts {
-			return nil, errors.New("not yet got the number we're after")
+			return nil, wrapTemporaryError(errors.New("not yet got the number we're after"))
 		}
 
 		return nil, nil
@@ -71,7 +71,7 @@ func TestBackoffTenAttemptsSuccess(t *testing.T) {
 	retryErr := Backoff(func() (*Response, error) {
 		externalCounter++
 		if externalCounter < attempts {
-			return nil, errors.New("not yet got the number we're after")
+			return nil, wrapTemporaryError(errors.New("not yet got the number we're after"))
 		}
 		return nil, nil
 	}, Retries(attempts), WaitTime(5), MaxWaitTime(500))

util.go

@@ -7,6 +7,7 @@ package resty
 import (
 	"bytes"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -369,24 +370,38 @@ func copyHeaders(hdrs http.Header) http.Header {
 	return nh
 }
 
-type noRetryErr struct {
+type temporaryError struct {
 	err error
 }
 
-func (e *noRetryErr) Error() string {
+func (e *temporaryError) Error() string {
 	return e.err.Error()
 }
 
-func wrapNoRetryErr(err error) error {
-	if err != nil {
-		err = &noRetryErr{err: err}
+func (e *temporaryError) Unwrap() error {
+	return e.err
+}
+
+func (e *temporaryError) Temporary() bool {
+	return true
+}
+
+// wrapTemporaryError wraps an error to advertise it should be retryable, if it doesn't specify it already.
+func wrapTemporaryError(err error) error {
+	if err == nil {
+		return nil
 	}
-	return err
+	var tempError interface{ Temporary() bool }
+	if errors.As(err, &tempError) {
+		return err // Already exposes the method, honour it, even if false
+	}
+	return &temporaryError{err}
 }
 
-func unwrapNoRetryErr(err error) error {
-	if e, ok := err.(*noRetryErr); ok {
-		err = e.err
+func isTemporaryError(err error) bool {
+	var tempError interface{ Temporary() bool }
+	if errors.As(err, &tempError) {
+		return tempError.Temporary()
 	}
-	return err
+	return false
 }

util_test.go

@@ -6,7 +6,9 @@ package resty
 
 import (
 	"bytes"
+	"errors"
 	"mime/multipart"
+	"net"
 	"testing"
 )
 
@@ -95,3 +97,22 @@ func TestWriteMultipartFormFileReaderError(t *testing.T) {
 	assertNotNil(t, err)
 	assertEqual(t, "read error", err.Error())
 }
+
+func Test_wrapTemporaryError(t *testing.T) {
+	tests := []struct {
+		name string
+		base error
+		temp bool
+	}{
+		{name: "nil", temp: false},
+		{name: "dns temp", base: &net.DNSError{Err: "err", IsTemporary: true}, temp: true},
+		{name: "dns not temp", base: &net.DNSError{Err: "err"}, temp: false},
+		{name: "other", base: errors.New("foo"), temp: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := wrapTemporaryError(tt.base)
+			assertEqual(t, tt.temp, isTemporaryError(err))
+		})
+	}
+}