diff --git a/example_test.go b/example_test.go index 2071d58..b34ba1b 100644 --- a/example_test.go +++ b/example_test.go @@ -69,6 +69,8 @@ func Example_httpGetWithStrategies() { func Example_withBackoffJitter() { action := func(attempt uint) error { + fmt.Println("attempt", attempt) + return errors.New("something happened") } @@ -83,4 +85,11 @@ func Example_withBackoffJitter() { jitter.Deviation(random, 0.5), ), ) + + // Output: + // attempt 1 + // attempt 2 + // attempt 3 + // attempt 4 + // attempt 5 } diff --git a/retry.go b/retry.go index 57aed6c..307cf53 100644 --- a/retry.go +++ b/retry.go @@ -17,7 +17,7 @@ func Retry(action Action, strategies ...strategy.Strategy) error { var err error for attempt := uint(0); (attempt == 0 || err != nil) && shouldAttempt(attempt, strategies...); attempt++ { - err = action(attempt) + err = action(attempt + 1) } return err diff --git a/retry_test.go b/retry_test.go index 8340a15..b3b886b 100644 --- a/retry_test.go +++ b/retry_test.go @@ -17,6 +17,43 @@ func TestRetry(t *testing.T) { } } +func TestRetryAttemptNumberIsAccurate(t *testing.T) { + var strategyAttemptNumber uint + var actionAttemptNumber uint + + strategy := func(attempt uint) bool { + strategyAttemptNumber = attempt + + return true + } + + action := func(attempt uint) error { + actionAttemptNumber = attempt + + return nil + } + + err := Retry(action, strategy) + + if err != nil { + t.Error("expected a nil error") + } + + if strategyAttemptNumber != 0 { + t.Errorf( + "expected strategy to receive 0, received %v instead", + strategyAttemptNumber, + ) + } + + if actionAttemptNumber != 1 { + t.Errorf( + "expected action to receive 1, received %v instead", + actionAttemptNumber, + ) + } +} + func TestRetryRetriesUntilNoErrorReturned(t *testing.T) { const errorUntilAttemptNumber = 5 diff --git a/strategy/strategy.go b/strategy/strategy.go index 0a5148c..8d2f0ec 100644 --- a/strategy/strategy.go +++ b/strategy/strategy.go @@ -15,16 +15,16 @@ import ( // allows for the next attempt to be made. Returning `false` halts the retrying // process and returns the last error returned by the called Action. // -// The strategy will be passed an "attempt" number on each successive retry +// The strategy will be passed an "attempt" number before each successive retry // iteration, starting with a `0` value before the first attempt is actually -// made. This allows for a pre-action delay, etc. +// made. This allows for a pre-action, such as a delay, etc. type Strategy func(attempt uint) bool // Limit creates a Strategy that limits the number of attempts that Retry will // make. func Limit(attemptLimit uint) Strategy { return func(attempt uint) bool { - return (attempt <= attemptLimit) + return (attempt < attemptLimit) } } diff --git a/strategy/strategy_test.go b/strategy/strategy_test.go index 8c525b6..0c13bd7 100644 --- a/strategy/strategy_test.go +++ b/strategy/strategy_test.go @@ -10,23 +10,25 @@ import ( const timeMarginOfError = time.Millisecond func TestLimit(t *testing.T) { + // Strategy attempts are 0-based. + // Treat this functionally as n+1. const attemptLimit = 3 strategy := Limit(attemptLimit) - if !strategy(1) { + if !strategy(0) { t.Error("strategy expected to return true") } - if !strategy(2) { + if !strategy(1) { t.Error("strategy expected to return true") } - if !strategy(3) { + if !strategy(2) { t.Error("strategy expected to return true") } - if strategy(4) { + if strategy(3) { t.Error("strategy expected to return false") } }