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

* Added reader seek start for multipart files on request retry
* Review fixes

---------

Co-authored-by: Niklas Paulicks <paulicks@justtrack.io>

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:

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

go.sum

@@ -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=

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))

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" }`)
 		}
 	})
 

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
+}

retry_test.go

@@ -5,9 +5,12 @@
 package resty
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
+	"io"
 	"net/http"
 	"reflect"
 	"strconv"
@@ -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()
 		}
@@ -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)
+}