feat(enhancement)!: remove HTTP status code check and add new exported retry condition for status codes (#1130)

#1080

Author: jeevatkm

client.go

@@ -1363,7 +1363,9 @@ func (c *Client) IsRetryDefaultConditions() bool {
 }
 
 // SetRetryDefaultConditions method is used to enable/disable the Resty's default
-// retry conditions
+// retry conditions that checks transport, headers and URL errors.
+//
+// By default it is enabled.
 //
 // It can be overridden at request level, see [Request.SetRetryDefaultConditions]
 func (c *Client) SetRetryDefaultConditions(b bool) *Client {

request.go

@@ -1171,7 +1171,9 @@ func (r *Request) SetRetryDelayStrategy(rs RetryDelayStrategyFunc) *Request {
 }
 
 // SetRetryDefaultConditions method is used to enable/disable the Resty's default
-// retry conditions on request level
+// retry conditions on request level, that checks transport, headers and URL errors.
+//
+// By default it is enabled.
 //
 // It overrides value set at the client instance level, see [Client.SetRetryDefaultConditions]
 func (r *Request) SetRetryDefaultConditions(b bool) *Request {
@@ -1509,7 +1511,7 @@ func (r *Request) Execute(method, url string) (res *Response, err error) {
 
 			// apply default retry conditions
 			if r.IsRetryDefaultConditions {
-				needsRetry = applyRetryDefaultConditions(res, err)
+				needsRetry = isDoNotRetryError(err)
 			}
 
 			// apply user-defined retry conditions if default one

retry.go

@@ -48,7 +48,52 @@ var (
 	regexErrInvalidHeader    = regexp.MustCompile("invalid header")
 )
 
-func applyRetryDefaultConditions(res *Response, err error) bool {
+// RetryConditionStatusTooManyRequests is a RetryConditionFunc that returns true
+// if the response status code is 429 Too Many Requests.
+//
+//   - The 429 status code indicates that the user has sent too many requests in a given amount
+//     of time ("rate limiting").
+//   - Retrying after receiving a 429 status code can be effective, especially if the server includes
+//     a Retry-After header indicating when to retry.
+func RetryConditionStatusTooManyRequests(res *Response, _ error) bool {
+	if res == nil {
+		return false
+	}
+	return res.StatusCode() == http.StatusTooManyRequests
+}
+
+// RetryConditionStatus5XX is a RetryConditionFunc that returns true if the response status code is 500 or above,
+// excluding Status 501 - Not Implemented.
+//
+//   - 5XX status codes are generally considered temporary server errors that may be resolved on retry
+//   - The rationale for excluding 501 Not Implemented is that it indicates the server does not support the
+//     functionality required to fulfill the request,
+func RetryConditionStatus5XX(res *Response, _ error) bool {
+	if res == nil {
+		return false
+	}
+	return res.StatusCode() >= 500 && res.StatusCode() != http.StatusNotImplemented
+}
+
+// RetryConditionStatusZero is a RetryConditionFunc that returns true if the response status code is 0.
+//
+//   - A status code of 0 typically indicates that no response was received from the server, which can occur
+//     due to network errors, timeouts, or other issues that prevent the request from being completed.
+//   - Retrying when a status code of 0 is encountered can be effective, as it may allow the request to succeed
+//     on subsequent attempts if the underlying issue is transient.
+func RetryConditionStatusZero(res *Response, _ error) bool {
+	if res == nil {
+		return false
+	}
+	return res.StatusCode() == 0
+}
+
+// isDoNotRetryError checks whether the given request error should trigger a retry.
+//
+// It returns true only for retryable URL/network errors and false for errors that
+// should not be retried, such as TLS certificate errors, invalid scheme, invalid
+// headers, and too many redirects.
+func isDoNotRetryError(err error) bool {
 	// no retry on TLS error
 	if _, ok := err.(*tls.CertificateVerificationError); ok {
 		return false
@@ -68,20 +113,6 @@ func applyRetryDefaultConditions(res *Response, err error) bool {
 		return u.Temporary() // possible retry if it's true
 	}
 
-	if res == nil {
-		return false
-	}
-
-	// certain HTTP status codes are temporary so that we can retry
-	//	- 429 Too Many Requests
-	//	- 500 or above (it's better to ignore 501 Not Implemented)
-	//	- 0 No status code received
-	if res.StatusCode() == http.StatusTooManyRequests ||
-		(res.StatusCode() >= 500 && res.StatusCode() != http.StatusNotImplemented) ||
-		res.StatusCode() == 0 {
-		return true
-	}
-
 	return false
 }
 

retry_test.go

@@ -573,6 +573,7 @@ func TestClientRetryTooManyRequestsAndRecover(t *testing.T) {
 		SetJSONEscapeHTML(false).
 		SetResult(AuthSuccess{}).
 		SetTimeout(10 * time.Millisecond).
+		AddRetryConditions(RetryConditionStatusTooManyRequests).
 		Get(ts.URL + "/set-retry-error-recover")
 
 	assertError(t, err)
@@ -649,7 +650,8 @@ func TestResetMultipartReaderSeekStartError(t *testing.T) {
 
 	c := dcnl().
 		SetRetryCount(2).
-		SetTimeout(200 * time.Millisecond)
+		SetTimeout(200 * time.Millisecond).
+		AddRetryConditions(RetryConditionStatus5XX)
 
 	resp, err := c.R().
 		SetFileReader("name", "filename", testSeeker).
@@ -756,7 +758,8 @@ func TestRequestRetryTooManyRequestsHeaderRetryAfter(t *testing.T) {
 	ts := createGetServer(t)
 	defer ts.Close()
 
-	c := dcnl()
+	c := dcnl().
+		AddRetryConditions(RetryConditionStatusTooManyRequests)
 
 	resp, err := c.R().
 		SetRetryCount(2).
@@ -823,7 +826,7 @@ func TestRetryDefaultConditions(t *testing.T) {
 	})
 
 	t.Run("nil values", func(t *testing.T) {
-		result := applyRetryDefaultConditions(nil, nil)
+		result := isDoNotRetryError(nil)
 		assertFalse(t, result)
 	})
 }
@@ -1105,9 +1108,20 @@ func TestRetryCoverage(t *testing.T) {
 		assertEqual(t, 2*time.Second, dur2)
 	})
 
+	t.Run("retry condition nil response", func(t *testing.T) {
+		result := RetryConditionStatusTooManyRequests(nil, nil)
+		assertFalse(t, result, "expected no retry for nil response")
+
+		result = RetryConditionStatus5XX(nil, nil)
+		assertFalse(t, result, "expected no retry for nil response")
+
+		result = RetryConditionStatusZero(nil, nil)
+		assertFalse(t, result, "expected no retry for nil response")
+	})
+
 	t.Run("mock tls cert error", func(t *testing.T) {
 		certError := tls.CertificateVerificationError{}
-		result1 := applyRetryDefaultConditions(nil, &certError)
+		result1 := isDoNotRetryError(&certError)
 		assertFalse(t, result1, "expected no retry for tls.CertificateVerificationError")
 	})
 }

sse.go

@@ -76,6 +76,7 @@ type (
 		retryCount       int
 		retryWaitTime    time.Duration
 		retryMaxWaitTime time.Duration
+		retryConditions  []RetryConditionFunc
 		serverSentRetry  time.Duration
 		maxBufSize       int
 		onOpen           SSEOpenFunc
@@ -117,13 +118,19 @@ func NewSSESource() *SSESource {
 		retryCount:       3,
 		retryWaitTime:    defaultWaitTime,
 		retryMaxWaitTime: defaultMaxWaitTime,
+		retryConditions:  make([]RetryConditionFunc, 0),
 		maxBufSize:       defaultSseMaxBufSize,
 		onEvent:          make(map[string]*callback),
 		httpClient: &http.Client{
 			Jar:       createCookieJar(),
 			Transport: createTransport(nil, nil),
 		},
 	}
+	sse.retryConditions = append(sse.retryConditions,
+		RetryConditionStatusZero,
+		RetryConditionStatusTooManyRequests,
+		RetryConditionStatus5XX,
+	)
 	return sse
 }
 
@@ -603,7 +610,14 @@ func (sse *SSESource) connect() (*http.Response, error) {
 		}
 
 		rRes := wrapResponse(resp, req)
-		needsRetry := applyRetryDefaultConditions(rRes, doErr)
+		needsRetry := isDoNotRetryError(doErr)
+		if !needsRetry && resp != nil {
+			for _, retryCondition := range sse.retryConditions {
+				if needsRetry = retryCondition(rRes, doErr); needsRetry {
+					break
+				}
+			}
+		}
 
 		// retry not required stop here
 		if !needsRetry {