Skip to content

Commit

Permalink
Don't wait after last retry (#444)
Browse files Browse the repository at this point in the history
* Don't wait after last retry
* Fix tests

Signed-off-by: Pavel <kositsyn.pa@phystech.edu>
  • Loading branch information
pkositsyn committed Sep 13, 2021
1 parent c1fa358 commit b8f7c11
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 6 deletions.
6 changes: 6 additions & 0 deletions retry.go
Expand Up @@ -129,6 +129,12 @@ func Backoff(operation func() (*Response, error), options ...Option) error {
hook(resp, err)
}

// Don't need to wait when no retries left.
// Still run retry hooks even on last retry to keep compatibility.
if attempt == opts.maxRetries {
return err
}

waitTime, err2 := sleepDuration(resp, opts.waitTime, opts.maxWaitTime, attempt)
if err2 != nil {
if err == nil {
Expand Down
45 changes: 39 additions & 6 deletions retry_test.go
Expand Up @@ -32,6 +32,39 @@ func TestBackoffSuccess(t *testing.T) {
assertEqual(t, externalCounter, attempts)
}

func TestBackoffNoWaitForLastRetry(t *testing.T) {
attempts := 1
externalCounter := 0
numRetries := 1

canceledCtx, cancel := context.WithCancel(context.Background())
defer cancel()

resp := &Response{
Request: &Request{
ctx: canceledCtx,
client: &Client{
RetryAfter: func(*Client, *Response) (time.Duration, error) {
return 6, nil
},
},
},
}

retryErr := Backoff(func() (*Response, error) {
externalCounter++
return resp, nil
}, RetryConditions([]RetryConditionFunc{func(response *Response, err error) bool {
if externalCounter == attempts + numRetries {
// Backoff returns context canceled if goes to sleep after last retry.
cancel()
}
return true
}}), Retries(numRetries))

assertNil(t, retryErr)
}

func TestBackoffTenAttemptsSuccess(t *testing.T) {
attempts := 10
externalCounter := 0
Expand Down Expand Up @@ -169,8 +202,8 @@ func TestClientRetryGet(t *testing.T) {
assertNotNil(t, resp.Body())
assertEqual(t, 0, len(resp.Header()))

assertEqual(t, true, (strings.HasPrefix(err.Error(), "Get "+ts.URL+"/set-retrycount-test") ||
strings.HasPrefix(err.Error(), "Get \""+ts.URL+"/set-retrycount-test\"")))
assertEqual(t, true, strings.HasPrefix(err.Error(), "Get "+ts.URL+"/set-retrycount-test") ||
strings.HasPrefix(err.Error(), "Get \""+ts.URL+"/set-retrycount-test\""))
}

func TestClientRetryWait(t *testing.T) {
Expand Down Expand Up @@ -639,8 +672,8 @@ func TestClientRetryCount(t *testing.T) {
// 2 attempts were made
assertEqual(t, attempt, 2)

assertEqual(t, true, (strings.HasPrefix(err.Error(), "Get "+ts.URL+"/set-retrycount-test") ||
strings.HasPrefix(err.Error(), "Get \""+ts.URL+"/set-retrycount-test\"")))
assertEqual(t, true, strings.HasPrefix(err.Error(), "Get "+ts.URL+"/set-retrycount-test") ||
strings.HasPrefix(err.Error(), "Get \""+ts.URL+"/set-retrycount-test\""))
}

func TestClientErrorRetry(t *testing.T) {
Expand Down Expand Up @@ -693,8 +726,8 @@ func TestClientRetryHook(t *testing.T) {

assertEqual(t, 3, attempt)

assertEqual(t, true, (strings.HasPrefix(err.Error(), "Get "+ts.URL+"/set-retrycount-test") ||
strings.HasPrefix(err.Error(), "Get \""+ts.URL+"/set-retrycount-test\"")))
assertEqual(t, true, strings.HasPrefix(err.Error(), "Get "+ts.URL+"/set-retrycount-test") ||
strings.HasPrefix(err.Error(), "Get \""+ts.URL+"/set-retrycount-test\""))
}

func filler(*Response, error) bool {
Expand Down

0 comments on commit b8f7c11

Please sign in to comment.