From 0886b1c5201b26f900e10fc701db66c680b0ec91 Mon Sep 17 00:00:00 2001 From: Zack Zeng Date: Sat, 1 Nov 2025 23:47:21 +0800 Subject: [PATCH] refactor: always attemp to reset readers during retry Signed-off-by: Zack Zeng --- client.go | 8 -------- middleware.go | 26 +++++++++++--------------- multipart_test.go | 3 +-- request.go | 1 - request_test.go | 2 +- retry.go | 21 ++++++++------------- retry_test.go | 6 +----- 7 files changed, 22 insertions(+), 45 deletions(-) diff --git a/client.go b/client.go index ebc93c5..2b88d7f 100644 --- a/client.go +++ b/client.go @@ -120,7 +120,6 @@ 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) @@ -778,13 +777,6 @@ func (c *Client) AddRetryHook(hook OnRetryFunc) *Client { return c } -// SetRetryResetReaders method enables the Resty client to seek the start of all -// file readers are given as multipart files if the object implements [io.ReadSeeker]. -func (c *Client) SetRetryResetReaders(b bool) *Client { - c.RetryResetReaders = b - return c -} - // SetTLSClientConfig method sets TLSClientConfig for underlying client Transport. // // For Example: diff --git a/middleware.go b/middleware.go index acbcdb2..27133ab 100644 --- a/middleware.go +++ b/middleware.go @@ -209,16 +209,23 @@ func parseRequestBody(c *Client, r *Request) error { } func createHTTPRequest(c *Client, r *Request) (err error) { + // Enable trace + if c.trace || r.trace { + r.clientTrace = &clientTrace{} + r.ctx = r.clientTrace.createContext(r.Context()) + } + + var reqBody io.Reader if r.bodyReadSeeker == nil { if reader, ok := r.Body.(io.Reader); ok && isPayloadSupported(r.Method, c.AllowGetMethodPayload) { - r.RawRequest, err = http.NewRequest(r.Method, r.URL, reader) + reqBody = reader } else { - r.RawRequest, err = http.NewRequest(r.Method, r.URL, nil) + reqBody = nil } } else { - r.RawRequest, err = http.NewRequest(r.Method, r.URL, r.bodyReadSeeker) + reqBody = r.bodyReadSeeker } - + r.RawRequest, err = http.NewRequestWithContext(r.Context(), r.Method, r.URL, reqBody) if err != nil { return } @@ -239,17 +246,6 @@ func createHTTPRequest(c *Client, r *Request) (err error) { r.RawRequest.AddCookie(cookie) } - // Enable trace - if c.trace || r.trace { - r.clientTrace = &clientTrace{} - r.ctx = r.clientTrace.createContext(r.Context()) - } - - // Use context if it was specified - if r.ctx != nil { - r.RawRequest = r.RawRequest.WithContext(r.ctx) - } - return } diff --git a/multipart_test.go b/multipart_test.go index 17bde69..8cf2854 100644 --- a/multipart_test.go +++ b/multipart_test.go @@ -49,8 +49,7 @@ func TestRetryMultiPartUploadWithStreaming(t *testing.T) { basePath := getTestDataPath() c := New(). SetRetryCount(3). - AddRetryAfterErrorCondition(). - SetRetryResetReaders(true) + AddRetryAfterErrorCondition() resp, err := c.R(). SetFile("file1", filepath.Join(basePath, "test-img.png")). SetFile("file2", filepath.Join(basePath, "text-file.txt")). diff --git a/request.go b/request.go index 11c89fd..7147e8d 100644 --- a/request.go +++ b/request.go @@ -1051,7 +1051,6 @@ 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), ) if err != nil { diff --git a/request_test.go b/request_test.go index c116afa..f2bf3cb 100644 --- a/request_test.go +++ b/request_test.go @@ -2234,7 +2234,7 @@ func TestRequestGH917(t *testing.T) { func(r *Response, err error) bool { return err != nil || r.StatusCode() > 499 }, - ).SetRetryCount(3).SetRetryResetReaders(true) + ).SetRetryCount(3) wg := sync.WaitGroup{} // Run tests concurrently to make the issue easily to observe. diff --git a/retry.go b/retry.go index 9b3019e..d4654c5 100644 --- a/retry.go +++ b/retry.go @@ -44,7 +44,6 @@ type ( maxWaitTime time.Duration retryConditions []RetryConditionFunc retryHooks []OnRetryFunc - resetReaders bool } ) @@ -83,14 +82,6 @@ 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 { @@ -135,17 +126,21 @@ func Backoff(operation func() (*Response, error), options ...Option) error { return err } - if resp != nil && resp.Request.bodyReadSeeker != nil { - silently(resp.Request.bodyReadSeeker.Seek(0, io.SeekStart)) - } + if resp != nil { + if resp.Request.bodyReadSeeker != nil { + _, err := resp.Request.bodyReadSeeker.Seek(0, io.SeekStart) + if err != nil { + return err + } + } - if opts.resetReaders { if rs, ok := resp.Request.Body.(io.ReadSeeker); ok { _, err := rs.Seek(0, io.SeekStart) if err != nil { return err } } + if err := resetFieldReaders(resp.Request.multipartFields); err != nil { return err } diff --git a/retry_test.go b/retry_test.go index 27b286c..ba41d3f 100644 --- a/retry_test.go +++ b/retry_test.go @@ -246,7 +246,7 @@ func TestClientRetryWait(t *testing.T) { slept := time.Duration(retryIntervals[i]) // Ensure that client has slept some duration between // waitTime and maxWaitTime for consequent requests - isExceed := (slept - retryMaxWaitTime) > time.Millisecond // avoid flaky test due to time measurement precision + isExceed := (slept - retryMaxWaitTime) > 100*time.Millisecond // avoid flaky test due to time measurement precision if slept < retryWaitTime || isExceed { t.Errorf("Client has slept %f seconds before retry %d", slept.Seconds(), i) } @@ -781,7 +781,6 @@ func TestResetMultipartReaderSeekStartError(t *testing.T) { c := dc(). SetRetryCount(2). SetTimeout(time.Second * 3). - SetRetryResetReaders(true). AddRetryAfterErrorCondition() resp, err := c.R(). @@ -805,7 +804,6 @@ func TestResetMultipartReaders(t *testing.T) { c := dc(). SetRetryCount(2). SetTimeout(time.Second * 3). - SetRetryResetReaders(true). AddRetryAfterErrorCondition(). AddRetryHook( func(response *Response, _ error) { @@ -836,7 +834,6 @@ func TestResetMultipartFieldReaderSeekStartError(t *testing.T) { c := dc(). SetRetryCount(2). SetTimeout(time.Second * 3). - SetRetryResetReaders(true). AddRetryAfterErrorCondition() resp, err := c.R(). @@ -860,7 +857,6 @@ func TestResetMultipartFieldReaders(t *testing.T) { c := dc(). SetRetryCount(2). SetTimeout(time.Second * 3). - SetRetryResetReaders(true). AddRetryAfterErrorCondition(). AddRetryHook( func(response *Response, _ error) {