feat!: redesign request retry flow, retry-after header, default retry conditions #886

README.md

@@ -700,6 +700,8 @@ client.RemoveProxy()
 Resty uses [backoff](http://www.awsarchitectureblog.com/2015/03/backoff.html)
 to increase retry intervals after each attempt.
 
+TODO update retry docs
+
 Usage example:
 
 ```go

client.go

@@ -1,6 +1,7 @@
-// Copyright (c) 2015-2024 Jeevanandam M ([email protected]), All rights reserved.
+// Copyright (c) 2015-present Jeevanandam M ([email protected]), All rights reserved.
 // resty source code and usage is governed by a MIT style
 // license that can be found in the LICENSE file.
+// SPDX-License-Identifier: MIT
 
 package resty
 
@@ -63,9 +64,9 @@ var (
 	hdrContentLengthKey   = http.CanonicalHeaderKey("Content-Length")
 	hdrContentEncodingKey = http.CanonicalHeaderKey("Content-Encoding")
 	hdrContentDisposition = http.CanonicalHeaderKey("Content-Disposition")
-	hdrLocationKey        = http.CanonicalHeaderKey("Location")
 	hdrAuthorizationKey   = http.CanonicalHeaderKey("Authorization")
 	hdrWwwAuthenticateKey = http.CanonicalHeaderKey("WWW-Authenticate")
+	hdrRetryAfterKey      = http.CanonicalHeaderKey("Retry-After")
 
 	plainTextType   = "text/plain; charset=utf-8"
 	jsonContentType = "application/json"
@@ -178,9 +179,9 @@ type Client struct {
 	retryWaitTime            time.Duration
 	retryMaxWaitTime         time.Duration
 	retryConditions          []RetryConditionFunc
-	retryHooks               []OnRetryFunc
-	retryAfter               RetryAfterFunc
-	retryResetReaders        bool
+	retryHooks               []RetryHookFunc
+	retryStrategy            RetryStrategyFunc
+	isRetryDefaultConditions bool
 	headerAuthorizationKey   string
 	responseBodyLimit        int64
 	resBodyUnlimitedReads    bool
@@ -198,7 +199,6 @@ type Client struct {
 	proxyURL                 *url.URL
 	requestLog               RequestLogCallback
 	responseLog              ResponseLogCallback
-	rateLimiter              RateLimiter
 	generateCurlOnDebug      bool
 	loadBalancer             LoadBalancer
 	beforeRequest            []RequestMiddleware
@@ -635,7 +635,8 @@ func (c *Client) R() *Request {
 		RetryCount:                 c.retryCount,
 		RetryWaitTime:              c.retryWaitTime,
 		RetryMaxWaitTime:           c.retryMaxWaitTime,
-		RetryResetReaders:          c.retryResetReaders,
+		RetryStrategy:              c.retryStrategy,
+		IsRetryDefaultConditions:   c.isRetryDefaultConditions,
 		CloseConnection:            c.closeConnection,
 		DoNotParseResponse:         c.notParseResponse,
 		DebugBodyLimit:             c.debugBodyLimit,
@@ -1204,20 +1205,57 @@ func (c *Client) SetRetryMaxWaitTime(maxWaitTime time.Duration) *Client {
 	return c
 }
 
-// RetryAfter method returns the retry after callback function, that is
-// used to calculate wait time between retries if it's registered; otherwise, it is nil.
-func (c *Client) RetryAfter() RetryAfterFunc {
+// RetryStrategy method returns the retry strategy function; otherwise, it is nil.
+//
+// See [Client.SetRetryStrategy]
+func (c *Client) RetryStrategy() RetryStrategyFunc {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+	return c.retryStrategy
+}
+
+// SetRetryStrategy method used to set the custom Retry strategy into Resty client,
+// it is used to get wait time before each retry. It can be overridden at request
+// level, see [Request.SetRetryStrategy]
+//
+// Default (nil) implies exponential backoff with a jitter strategy
+func (c *Client) SetRetryStrategy(rs RetryStrategyFunc) *Client {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+	c.retryStrategy = rs
+	return c
+}
+
+// EnableRetryDefaultConditions method enables the Resty's default retry conditions
+func (c *Client) EnableRetryDefaultConditions() *Client {
+	c.SetRetryDefaultConditions(true)
+	return c
+}
+
+// DisableRetryDefaultConditions method disables the Resty's default retry conditions
+func (c *Client) DisableRetryDefaultConditions() *Client {
+	c.SetRetryDefaultConditions(false)
+	return c
+}
+
+// IsRetryDefaultConditions method returns true if Resty's default retry conditions
+// are enabled otherwise false
+//
+// Default value is `true`
+func (c *Client) IsRetryDefaultConditions() bool {
 	c.lock.RLock()
 	defer c.lock.RUnlock()
-	return c.retryAfter
+	return c.isRetryDefaultConditions
 }
 
-// SetRetryAfter sets a callback to calculate the wait time between retries.
-// Default (nil) implies exponential backoff with jitter
-func (c *Client) SetRetryAfter(callback RetryAfterFunc) *Client {
+// SetRetryDefaultConditions method is used to enable/disable the Resty's default
+// retry conditions
+//
+// It can be overridden at request level, see [Request.SetRetryDefaultConditions]
+func (c *Client) SetRetryDefaultConditions(b bool) *Client {
 	c.lock.Lock()
 	defer c.lock.Unlock()
-	c.retryAfter = callback
+	c.isRetryDefaultConditions = b
 	return c
 }
 
@@ -1241,47 +1279,22 @@ func (c *Client) AddRetryCondition(condition RetryConditionFunc) *Client {
 	return c
 }
 
-// AddRetryAfterErrorCondition adds the basic condition of retrying after encountering
-// an error from the HTTP response
-func (c *Client) AddRetryAfterErrorCondition() *Client {
-	c.AddRetryCondition(func(response *Response, err error) bool {
-		return response.IsError()
-	})
-	return c
-}
-
 // RetryHooks method returns all the retry hook functions.
-func (c *Client) RetryHooks() []OnRetryFunc {
+func (c *Client) RetryHooks() []RetryHookFunc {
 	c.lock.RLock()
 	defer c.lock.RUnlock()
 	return c.retryHooks
 }
 
 // AddRetryHook adds a side-effecting retry hook to an array of hooks
 // that will be executed on each retry.
-func (c *Client) AddRetryHook(hook OnRetryFunc) *Client {
+func (c *Client) AddRetryHook(hook RetryHookFunc) *Client {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 	c.retryHooks = append(c.retryHooks, hook)
 	return c
 }
 
-// RetryResetReaders method returns true if the retry reset readers are enabled; otherwise, it is nil.
-func (c *Client) RetryResetReaders() bool {
-	c.lock.RLock()
-	defer c.lock.RUnlock()
-	return c.retryResetReaders
-}
-
-// 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.lock.Lock()
-	defer c.lock.Unlock()
-	c.retryResetReaders = b
-	return c
-}
-
 // SetTLSClientConfig method sets TLSClientConfig for underlying client Transport.
 //
 // For Example:
@@ -1539,22 +1552,6 @@ func (c *Client) SetOutputDirectory(dirPath string) *Client {
 	return c
 }
 
-// RateLimiter method returns the rate limiter interface
-func (c *Client) RateLimiter() RateLimiter {
-	c.lock.RLock()
-	defer c.lock.RUnlock()
-	return c.rateLimiter
-}
-
-// SetRateLimiter sets an optional [RateLimiter]. If set, the rate limiter will control
-// all requests were made by this client.
-func (c *Client) SetRateLimiter(rl RateLimiter) *Client {
-	c.lock.Lock()
-	defer c.lock.Unlock()
-	c.rateLimiter = rl
-	return c
-}
-
 // Transport method returns [http.Transport] currently in use or error
 // in case the currently used `transport` is not a [http.Transport].
 //
@@ -1947,30 +1944,18 @@ func (c *Client) Close() error {
 func (c *Client) executeBefore(req *Request) error {
 	var err error
 
-	if isStringEmpty(req.Method) {
-		req.Method = MethodGet
-	}
-
 	// user defined on before request methods
 	// to modify the *resty.Request object
 	for _, f := range c.beforeRequestMiddlewares() {
 		if err = f(c, req); err != nil {
-			return wrapNoRetryErr(err)
-		}
-	}
-
-	// If there is a rate limiter set for this client, the Execute call
-	// will return an error if the rate limit is exceeded.
-	if req.client.RateLimiter() != nil {
-		if !req.client.RateLimiter().Allow() {
-			return ErrRateLimitExceeded
+			return err
 		}
 	}
 
 	// resty middlewares
 	for _, f := range c.beforeRequest {
 		if err = f(c, req); err != nil {
-			return wrapNoRetryErr(err)
+			return err
 		}
 	}
 
@@ -1981,7 +1966,7 @@ func (c *Client) executeBefore(req *Request) error {
 	// call pre-request if defined
 	if c.preReqHook != nil {
 		if err = c.preReqHook(c, req.RawRequest); err != nil {
-			return wrapNoRetryErr(err)
+			return err
 		}
 	}
 
@@ -1996,10 +1981,9 @@ func (c *Client) execute(req *Request) (*Response, error) {
 	}
 
 	if err := requestDebugLogger(c, req); err != nil {
-		return nil, wrapNoRetryErr(err)
+		return nil, err
 	}
 
-	req.RawRequest.Body = wrapRequestBufferReleaser(req)
 	req.Time = time.Now()
 	resp, err := c.Client().Do(req.RawRequest)
 
@@ -2046,7 +2030,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

client_test.go

@@ -1,6 +1,7 @@
-// Copyright (c) 2015-2024 Jeevanandam M ([email protected]), All rights reserved.
+// Copyright (c) 2015-present Jeevanandam M ([email protected]), All rights reserved.
 // resty source code and usage is governed by a MIT style
 // license that can be found in the LICENSE file.
+// SPDX-License-Identifier: MIT
 
 package resty
 
@@ -9,13 +10,14 @@ import (
 	"compress/gzip"
 	"compress/lzw"
 	"context"
-	"crypto/rand"
+	cryprand "crypto/rand"
 	"crypto/tls"
 	"errors"
 	"fmt"
 	"io"
 	"log"
 	"math"
+	"math/rand"
 	"net"
 	"net/http"
 	"net/url"
@@ -202,19 +204,19 @@ func TestClientTimeout(t *testing.T) {
 	ts := createGetServer(t)
 	defer ts.Close()
 
-	c := dcnl().SetTimeout(time.Second * 3)
+	c := dcnl().SetTimeout(time.Millisecond * 200)
 	_, err := c.R().Get(ts.URL + "/set-timeout-test")
 
-	assertEqual(t, true, strings.Contains(strings.ToLower(err.Error()), "timeout"))
+	assertEqual(t, true, strings.Contains(err.Error(), "Client.Timeout"))
 }
 
 func TestClientTimeoutWithinThreshold(t *testing.T) {
 	ts := createGetServer(t)
 	defer ts.Close()
 
-	c := dcnl().SetTimeout(time.Second * 3)
-	resp, err := c.R().Get(ts.URL + "/set-timeout-test-with-sequence")
+	c := dcnl().SetTimeout(200 * time.Millisecond)
 
+	resp, err := c.R().Get(ts.URL + "/set-timeout-test-with-sequence")
 	assertError(t, err)
 
 	seq1, _ := strconv.ParseInt(resp.String(), 10, 32)
@@ -490,6 +492,12 @@ func TestClientSettingsCoverage(t *testing.T) {
 
 	c.DisableDebug()
 
+	assertEqual(t, true, c.IsRetryDefaultConditions())
+	c.DisableRetryDefaultConditions()
+	assertEqual(t, false, c.IsRetryDefaultConditions())
+	c.EnableRetryDefaultConditions()
+	assertEqual(t, true, c.IsRetryDefaultConditions())
+
 	// [Start] Custom Transport scenario
 	ct := dcnl()
 	ct.SetTransport(&CustomRoundTripper{})
@@ -1204,19 +1212,26 @@ func TestPostRedirectWithBody(t *testing.T) {
 	ts := createPostServer(t)
 	defer ts.Close()
 
-	targetURL, _ := url.Parse(ts.URL)
-	t.Log("ts.URL:", ts.URL)
-	t.Log("targetURL.Host:", targetURL.Host)
+	mu := sync.Mutex{}
+	rnd := rand.New(rand.NewSource(time.Now().UnixNano()))
 
-	c := dcnl()
+	c := dcnl().SetBaseURL(ts.URL)
+
+	totalRequests := 4000
 	wg := sync.WaitGroup{}
-	for i := 0; i < 100; i++ {
-		wg.Add(1)
+	wg.Add(totalRequests)
+	for i := 0; i < totalRequests; i++ {
+		if i%50 == 0 {
+			time.Sleep(20 * time.Millisecond) // to prevent test server socket exhaustion
+		}
 		go func() {
 			defer wg.Done()
+			mu.Lock()
+			randNumber := rnd.Int()
+			mu.Unlock()
 			resp, err := c.R().
-				SetBody([]byte(strconv.Itoa(newRnd().Int()))).
-				Post(targetURL.String() + "/redirect-with-body")
+				SetBody([]byte(strconv.Itoa(randNumber))).
+				Post("/redirect-with-body")
 			assertError(t, err)
 			assertNotNil(t, resp)
 		}()
@@ -1252,14 +1267,6 @@ func TestUnixSocket(t *testing.T) {
 	assertEqual(t, "Hello resty client from a server running on endpoint /hello!", res.String())
 }
 
-var _ RateLimiter = (*testRateLimiter)(nil)
-
-type testRateLimiter struct{}
-
-func (t *testRateLimiter) Allow() bool {
-	return false
-}
-
 func TestClientClone(t *testing.T) {
 	parent := New()
 
@@ -1268,9 +1275,6 @@ func TestClientClone(t *testing.T) {
 	parent.SetBasicAuth("parent", "")
 	parent.SetProxy("http://localhost:8080")
 
-	// set an interface field
-	tr := &testRateLimiter{}
-	parent.SetRateLimiter(tr)
 	parent.SetCookie(&http.Cookie{
 		Name:  "go-resty-1",
 		Value: "This is cookie 1 value",
@@ -1300,12 +1304,11 @@ func TestClientClone(t *testing.T) {
 
 	// assert interface/pointer type
 	assertEqual(t, parent.Client(), clone.Client())
-	assertEqual(t, parent.RateLimiter(), clone.RateLimiter())
 }
 
 func TestResponseBodyLimit(t *testing.T) {
 	ts := createTestServer(func(w http.ResponseWriter, r *http.Request) {
-		io.CopyN(w, rand.Reader, 100*800)
+		io.CopyN(w, cryprand.Reader, 100*800)
 	})
 	defer ts.Close()