Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change noRetryError for temporary errors #478

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions client.go
Expand Up @@ -900,14 +900,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
}
}

Expand All @@ -918,12 +918,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)
Expand All @@ -938,7 +938,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 {
Expand All @@ -951,15 +951,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))
Expand All @@ -974,7 +974,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
Expand Down
11 changes: 5 additions & 6 deletions request.go
Expand Up @@ -745,7 +745,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
}
}
Expand All @@ -755,9 +755,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(
Expand All @@ -780,9 +779,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
}

//‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾
Expand Down
5 changes: 2 additions & 3 deletions retry.go
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions retry_test.go
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
35 changes: 25 additions & 10 deletions util.go
Expand Up @@ -6,6 +6,7 @@ package resty

import (
"bytes"
"errors"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -368,24 +369,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
}
21 changes: 21 additions & 0 deletions util_test.go
Expand Up @@ -6,7 +6,9 @@ package resty

import (
"bytes"
"errors"
"mime/multipart"
"net"
"testing"
)

Expand Down Expand Up @@ -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))
})
}
}