Merge pull request #12 from Rican7/bugfix/attempt-number-accuracy

Bugfix - Attempt Number Accuracy

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
 }

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

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
 

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)
 	}
 }
 

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")
 	}
 }