From 3d96ce7768cacdd2284d6609638d51245cab06a6 Mon Sep 17 00:00:00 2001 From: Niklas Paulicks Date: Wed, 15 Jun 2022 12:22:20 +0200 Subject: [PATCH 1/2] Added reader seek start for multipart files on request retry --- client.go | 10 ++++++++++ go.mod | 2 +- go.sum | 7 +------ request.go | 1 + resty_test.go | 4 ++++ retry.go | 28 ++++++++++++++++++++++++++++ retry_test.go | 36 +++++++++++++++++++++++++++++++++++- 7 files changed, 80 insertions(+), 8 deletions(-) diff --git a/client.go b/client.go index f8823e8c..326a4a57 100644 --- a/client.go +++ b/client.go @@ -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) @@ -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: diff --git a/go.mod b/go.mod index 5e78bdc8..d712b6ef 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 07a45152..e930c2d2 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,2 @@ -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/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/request.go b/request.go index 25b4c3b6..adb129e3 100644 --- a/request.go +++ b/request.go @@ -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)) diff --git a/resty_test.go b/resty_test.go index b8154183..b97e0660 100644 --- a/resty_test.go +++ b/resty_test.go @@ -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" }`) } }) diff --git a/retry.go b/retry.go index 00b8514a..f8c76659 100644 --- a/retry.go +++ b/retry.go @@ -6,6 +6,7 @@ package resty import ( "context" + "io" "math" "math/rand" "sync" @@ -43,6 +44,7 @@ type ( maxWaitTime time.Duration retryConditions []RetryConditionFunc retryHooks []OnRetryFunc + resetReaders bool } ) @@ -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 { @@ -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) } @@ -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 +} diff --git a/retry_test.go b/retry_test.go index 9f8fb387..1475c7ae 100644 --- a/retry_test.go +++ b/retry_test.go @@ -5,6 +5,7 @@ package resty import ( + "bytes" "context" "encoding/json" "errors" @@ -55,7 +56,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() } @@ -733,3 +734,36 @@ func TestClientRetryHook(t *testing.T) { func filler(*Response, error) bool { return false } + +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) +} From 23e9fefd0b7c63ba2427931322a3804be67a2dea Mon Sep 17 00:00:00 2001 From: Niklas Paulicks Date: Mon, 6 Mar 2023 08:35:44 +0100 Subject: [PATCH 2/2] Review fixes --- go.sum | 5 +++++ retry_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/go.sum b/go.sum index e930c2d2..7b4eb43e 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,7 @@ 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= diff --git a/retry_test.go b/retry_test.go index 1475c7ae..d979fbfe 100644 --- a/retry_test.go +++ b/retry_test.go @@ -9,6 +9,8 @@ import ( "context" "encoding/json" "errors" + "fmt" + "io" "net/http" "reflect" "strconv" @@ -735,6 +737,46 @@ 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()