Skip to content

Commit

Permalink
feat(enhancement): enhance retry flow
Browse files Browse the repository at this point in the history
- add SetAllowNonIdempotentRetry method
- enable multiple reads and seek start io.Reader flow during a retry if its *bytes.Buffer
- use struct{} instead of bool
- godoc update
  • Loading branch information
jeevatkm committed Nov 23, 2024
1 parent dce8de9 commit 805c3d3
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 11 deletions.
33 changes: 33 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ type Client struct {
retryHooks []RetryHookFunc
retryStrategy RetryStrategyFunc
isRetryDefaultConditions bool
allowNonIdempotentRetry bool
headerAuthorizationKey string
responseBodyLimit int64
resBodyUnlimitedReads bool
Expand Down Expand Up @@ -631,6 +632,7 @@ func (c *Client) R() *Request {
ResponseBodyUnlimitedReads: c.resBodyUnlimitedReads,
AllowMethodGetPayload: c.allowMethodGetPayload,
AllowMethodDeletePayload: c.allowMethodDeletePayload,
AllowNonIdempotentRetry: c.allowNonIdempotentRetry,

client: c,
baseURL: c.baseURL,
Expand Down Expand Up @@ -1199,6 +1201,12 @@ func (c *Client) RetryCount() int {
// first attempt + retry count = total attempts
//
// See [Request.SetRetryStrategy]
//
// NOTE:
// - By default, Resty only does retry on idempotent HTTP methods, [RFC 9110 Section 9.2.2], [RFC 9110 Section 18.2]
//
// [RFC 9110 Section 9.2.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-idempotent-methods
// [RFC 9110 Section 18.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-method-registration
func (c *Client) SetRetryCount(count int) *Client {
c.lock.Lock()
defer c.lock.Unlock()
Expand Down Expand Up @@ -1296,6 +1304,31 @@ func (c *Client) SetRetryDefaultConditions(b bool) *Client {
return c
}

// AllowNonIdempotentRetry method returns true if the client is enabled to allow
// non-idempotent HTTP methods retry; otherwise, it is `false`
//
// Default value is `false`
func (c *Client) AllowNonIdempotentRetry() bool {
c.lock.RLock()
defer c.lock.RUnlock()
return c.allowNonIdempotentRetry
}

// SetAllowNonIdempotentRetry method is used to enable/disable non-idempotent HTTP
// methods retry. By default, Resty only allows idempotent HTTP methods, see
// [RFC 9110 Section 9.2.2], [RFC 9110 Section 18.2]
//
// It can be overridden at request level, see [Request.SetAllowNonIdempotentRetry]
//
// [RFC 9110 Section 9.2.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-idempotent-methods
// [RFC 9110 Section 18.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-method-registration
func (c *Client) SetAllowNonIdempotentRetry(b bool) *Client {
c.lock.Lock()
defer c.lock.Unlock()
c.allowNonIdempotentRetry = b
return c
}

// RetryConditions method returns all the retry condition functions.
func (c *Client) RetryConditions() []RetryConditionFunc {
c.lock.RLock()
Expand Down
21 changes: 20 additions & 1 deletion middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package resty

import (
"bytes"
"fmt"
"io"
"mime/multipart"
Expand Down Expand Up @@ -454,9 +455,27 @@ func handleRequestBody(c *Client, r *Request) error {
r.bodyBuf = acquireBuffer()

switch body := r.Body.(type) {
case io.Reader: // Resty v3 onwards io.Reader used as-is with the request body
case io.Reader:
// Resty v3 onwards io.Reader used as-is with the request body.
releaseBuffer(r.bodyBuf)
r.bodyBuf = nil

// enable multiple reads if retry enabled
// and body type is *bytes.Buffer
if r.RetryCount > 0 {
if b, ok := r.Body.(*bytes.Buffer); ok {
v := b.Bytes()
r.Body = bytes.NewReader(v)
}
}

// do seek start for retry attempt if io.ReadSeeker
// interface supported
if r.Attempt > 1 {
if rs, ok := r.Body.(io.ReadSeeker); ok {
_, _ = rs.Seek(0, io.SeekStart)
}
}
return nil
case []byte:
r.bodyBuf.Write(body)
Expand Down
45 changes: 35 additions & 10 deletions request.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type Request struct {
RetryMaxWaitTime time.Duration
RetryStrategy RetryStrategyFunc
IsRetryDefaultConditions bool
AllowNonIdempotentRetry bool

// RetryTraceID provides GUID for retry count > 0
RetryTraceID string
Expand Down Expand Up @@ -980,6 +981,12 @@ func (r *Request) AddRetryCondition(condition RetryConditionFunc) *Request {
// first attempt + retry count = total attempts
//
// See [Request.SetRetryStrategy]
//
// NOTE:
// - By default, Resty only does retry on idempotent HTTP methods, [RFC 9110 Section 9.2.2], [RFC 9110 Section 18.2]
//
// [RFC 9110 Section 9.2.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-idempotent-methods
// [RFC 9110 Section 18.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-method-registration
func (r *Request) SetRetryCount(count int) *Request {
r.RetryCount = count
return r
Expand Down Expand Up @@ -1034,6 +1041,19 @@ func (r *Request) SetRetryDefaultConditions(b bool) *Request {
return r
}

// SetAllowNonIdempotentRetry method is used to enable/disable non-idempotent HTTP
// methods retry. By default, Resty only allows idempotent HTTP methods, see
// [RFC 9110 Section 9.2.2], [RFC 9110 Section 18.2]
//
// It overrides value set at the client instance level, see [Client.SetAllowNonIdempotentRetry]
//
// [RFC 9110 Section 9.2.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-idempotent-methods
// [RFC 9110 Section 18.2]: https://datatracker.ietf.org/doc/html/rfc9110.html#name-method-registration
func (r *Request) SetAllowNonIdempotentRetry(b bool) *Request {
r.AllowNonIdempotentRetry = b
return r
}

//‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾
// HTTP request tracing
//_______________________________________________________________________
Expand Down Expand Up @@ -1295,7 +1315,7 @@ func (r *Request) Execute(method, url string) (res *Response, err error) {
}

if backoff != nil {
needsRetry := false
needsRetry, isCtxDone := false, false

// apply default retry conditions
if r.IsRetryDefaultConditions {
Expand Down Expand Up @@ -1343,10 +1363,15 @@ func (r *Request) Execute(method, url string) (res *Response, err error) {
timer := time.NewTimer(waitDuration)
select {
case <-r.Context().Done():
isCtxDone = true
timer.Stop()
return nil, wrapErrors(r.Context().Err(), err)
err = wrapErrors(r.Context().Err(), err)
break
case <-timer.C:
}
if isCtxDone {
break
}
}
}

Expand Down Expand Up @@ -1625,18 +1650,18 @@ func (r *Request) resetFileReaders() error {

// https://datatracker.ietf.org/doc/html/rfc9110.html#name-idempotent-methods
// https://datatracker.ietf.org/doc/html/rfc9110.html#name-method-registration
var idempotentMethods = map[string]bool{
MethodDelete: true,
MethodGet: true,
MethodHead: true,
MethodOptions: true,
MethodPut: true,
MethodTrace: true,
var idempotentMethods = map[string]struct{}{
MethodDelete: {},
MethodGet: {},
MethodHead: {},
MethodOptions: {},
MethodPut: {},
MethodTrace: {},
}

func (r *Request) isIdempotent() bool {
_, found := idempotentMethods[r.Method]
return found
return found || r.AllowNonIdempotentRetry
}

func jsonIndent(v []byte) []byte {
Expand Down
64 changes: 64 additions & 0 deletions retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"fmt"
"io"
"net/http"
"net/http/httptest"
"reflect"
"strconv"
"strings"
Expand Down Expand Up @@ -849,6 +850,69 @@ func TestRetryDefaultConditions(t *testing.T) {
})
}

func TestRetryRequestPutIoReadSeekerForBuffer(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
assertError(t, err)
assertEqual(t, 12, len(b))
assertEqual(t, "body content", string(b))
w.WriteHeader(http.StatusInternalServerError)
}))

c := dcnl().
AddRetryCondition(
func(r *Response, err error) bool {
return err != nil || r.StatusCode() > 499
},
).
SetRetryCount(3).
SetAllowNonIdempotentRetry(true)

assertEqual(t, true, c.AllowNonIdempotentRetry())

buf := bytes.NewBuffer([]byte("body content"))
resp, err := c.R().
SetBody(buf).
SetAllowMethodGetPayload(false).
Put(srv.URL)

assertNil(t, err)
assertEqual(t, 4, resp.Request.Attempt)
assertEqual(t, http.StatusInternalServerError, resp.StatusCode())
assertEqual(t, "", resp.String())
}

func TestRetryRequestPostIoReadSeeker(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
assertError(t, err)
assertEqual(t, 12, len(b))
assertEqual(t, "body content", string(b))
w.WriteHeader(http.StatusInternalServerError)
}))

c := dcnl().
AddRetryCondition(
func(r *Response, err error) bool {
return err != nil || r.StatusCode() > 499
},
).
SetRetryCount(3).
SetAllowNonIdempotentRetry(false)

assertEqual(t, false, c.AllowNonIdempotentRetry())

resp, err := c.R().
SetBody([]byte("body content")).
SetAllowNonIdempotentRetry(true).
Post(srv.URL)

assertNil(t, err)
assertEqual(t, 4, resp.Request.Attempt)
assertEqual(t, http.StatusInternalServerError, resp.StatusCode())
assertEqual(t, "", resp.String())
}

func TestRetryCoverage(t *testing.T) {
t.Run("apply retry default min and max value", func(t *testing.T) {
backoff := newBackoffWithJitter(0, 0)
Expand Down

0 comments on commit 805c3d3

Please sign in to comment.