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

Fix multipart file readers not being reset when doing a retry #549

Merged
Merged
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
10 changes: 10 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ type Client struct {
RetryConditions []RetryConditionFunc
RetryHooks []OnRetryFunc
RetryAfter RetryAfterFunc
RetryResetReaders bool
JSONMarshal func(v interface{}) ([]byte, error)
JSONUnmarshal func(data []byte, v interface{}) error
XMLMarshal func(v interface{}) ([]byte, error)
Expand Down Expand Up @@ -702,6 +703,15 @@ func (c *Client) AddRetryHook(hook OnRetryFunc) *Client {
return c
}

// SetRetryResetReaders method enables the Resty client to seek the start of all
// file readers given as multipart files, if the given object implements `io.ReadSeeker`.
//
// Since ...
func (c *Client) SetRetryResetReaders(b bool) *Client {
c.RetryResetReaders = b
return c
}

// SetTLSClientConfig method sets TLSClientConfig for underling client Transport.
//
// For Example:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module github.com/go-resty/resty/v2

require golang.org/x/net v0.0.0-20211029224645-99673261e6eb
require golang.org/x/net v0.0.0-20220325170049-de3da57026de

go 1.11
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
golang.org/x/net v0.0.0-20211029224645-99673261e6eb h1:pirldcYWx7rx7kE5r+9WsOXPXK0+WH5+uZ7uPmJ44uM=
golang.org/x/net v0.0.0-20211029224645-99673261e6eb/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/net v0.0.0-20220325170049-de3da57026de h1:pZB1TWnKi+o4bENlbzAgLrEbY4RMYmUIRobMcSmfeYc=
golang.org/x/net v0.0.0-20220325170049-de3da57026de/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
1 change: 1 addition & 0 deletions request.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,7 @@ func (r *Request) Execute(method, url string) (*Response, error) {
MaxWaitTime(r.client.RetryMaxWaitTime),
RetryConditions(append(r.retryConditions, r.client.RetryConditions...)),
RetryHooks(r.client.RetryHooks),
ResetMultipartReaders(r.client.RetryResetReaders),
)

r.client.onErrorHooks(r, resp, unwrapNoRetryErr(err))
Expand Down
4 changes: 4 additions & 0 deletions resty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ func createFilePostServer(t *testing.T) *httptest.Server {
size, _ := io.Copy(f, r.Body)

fmt.Fprintf(w, "File Uploaded successfully, file size: %v", size)
case "/set-reset-multipart-readers-test":
w.Header().Set(hdrContentTypeKey, "application/json; charset=utf-8")
w.WriteHeader(http.StatusInternalServerError)
_, _ = fmt.Fprintf(w, `{ "message": "error" }`)
}
})

Expand Down
28 changes: 28 additions & 0 deletions retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package resty

import (
"context"
"io"
"math"
"math/rand"
"sync"
Expand Down Expand Up @@ -43,6 +44,7 @@ type (
maxWaitTime time.Duration
retryConditions []RetryConditionFunc
retryHooks []OnRetryFunc
resetReaders bool
}
)

Expand Down Expand Up @@ -81,6 +83,14 @@ func RetryHooks(hooks []OnRetryFunc) Option {
}
}

// ResetMultipartReaders sets a boolean value which will lead the start being seeked out
// on all multipart file readers, if they implement io.ReadSeeker
func ResetMultipartReaders(value bool) Option {
return func(o *Options) {
o.resetReaders = value
}
}

// Backoff retries with increasing timeout duration up until X amount of retries
// (Default is 3 attempts, Override with option Retries(n))
func Backoff(operation func() (*Response, error), options ...Option) error {
Expand Down Expand Up @@ -125,6 +135,12 @@ func Backoff(operation func() (*Response, error), options ...Option) error {
return err
}

if opts.resetReaders {
if err := resetFileReaders(resp.Request.multipartFiles); err != nil {
return err
}
}

for _, hook := range opts.retryHooks {
hook(resp, err)
}
Expand Down Expand Up @@ -219,3 +235,15 @@ func newRnd() *rand.Rand {
var src = rand.NewSource(seed)
return rand.New(src)
}

func resetFileReaders(files []*File) error {
for _, f := range files {
if rs, ok := f.Reader.(io.ReadSeeker); ok {
if _, err := rs.Seek(0, io.SeekStart); err != nil {
return err
}
}
}

return nil
}
78 changes: 77 additions & 1 deletion retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
package resty

import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"reflect"
"strconv"
Expand Down Expand Up @@ -55,7 +58,7 @@ func TestBackoffNoWaitForLastRetry(t *testing.T) {
externalCounter++
return resp, nil
}, RetryConditions([]RetryConditionFunc{func(response *Response, err error) bool {
if externalCounter == attempts + numRetries {
if externalCounter == attempts+numRetries {
// Backoff returns context canceled if goes to sleep after last retry.
cancel()
}
Expand Down Expand Up @@ -733,3 +736,76 @@ func TestClientRetryHook(t *testing.T) {
func filler(*Response, error) bool {
return false
}

var seekFailure = fmt.Errorf("failing seek test")

type failingSeeker struct {
reader *bytes.Reader
}

func (f failingSeeker) Read(b []byte) (n int, err error) {
return f.reader.Read(b)
}

func (f failingSeeker) Seek(offset int64, whence int) (int64, error) {
if offset == 0 && whence == io.SeekStart {
return 0, seekFailure
}

return f.reader.Seek(offset, whence)
}

func TestResetMultipartReaderSeekStartError(t *testing.T) {
ts := createFilePostServer(t)
defer ts.Close()

testSeeker := &failingSeeker{
bytes.NewReader([]byte("test")),
}

c := dc().
SetRetryCount(2).
SetTimeout(time.Second * 3).
SetRetryResetReaders(true).
AddRetryAfterErrorCondition()

resp, err := c.R().
SetFileReader("name", "filename", testSeeker).
Post(ts.URL + "/set-reset-multipart-readers-test")

assertEqual(t, 500, resp.StatusCode())
assertEqual(t, err.Error(), seekFailure.Error())
}

func TestResetMultipartReaders(t *testing.T) {
ts := createFilePostServer(t)
defer ts.Close()

str := "test"
buf := []byte(str)

bufReader := bytes.NewReader(buf)
bufCpy := make([]byte, len(buf))

c := dc().
SetRetryCount(2).
SetTimeout(time.Second * 3).
SetRetryResetReaders(true).
AddRetryAfterErrorCondition().
AddRetryHook(
func(response *Response, _ error) {
read, err := bufReader.Read(bufCpy)

assertNil(t, err)
assertEqual(t, len(buf), read)
assertEqual(t, str, string(bufCpy))
},
)

resp, err := c.R().
SetFileReader("name", "filename", bufReader).
Post(ts.URL + "/set-reset-multipart-readers-test")

assertEqual(t, 500, resp.StatusCode())
assertNil(t, err)
}