Merge branch 'master' into merge_129

Author: tstromberg

options.go

@@ -158,6 +158,35 @@ func CombineDelay(delays ...DelayTypeFunc) DelayTypeFunc {
 	}
 }
 
+// FullJitterBackoffDelay is a DelayTypeFunc that calculates delay using exponential backoff
+// with full jitter. The delay is a random value between 0 and the current backoff ceiling.
+// Formula: sleep = random_between(0, min(cap, base * 2^attempt))
+// It uses config.Delay as the base delay and config.MaxDelay as the cap.
+func FullJitterBackoffDelay(n uint, err error, config *Config) time.Duration {
+	// Calculate the exponential backoff ceiling for the current attempt
+	backoffCeiling := float64(config.delay) * math.Pow(2, float64(n))
+	currentCap := float64(config.maxDelay)
+
+	// If MaxDelay is set and backoffCeiling exceeds it, cap at MaxDelay
+	if currentCap > 0 && backoffCeiling > currentCap {
+		backoffCeiling = currentCap
+	}
+
+	// Ensure backoffCeiling is at least 0
+	if backoffCeiling < 0 {
+		backoffCeiling = 0
+	}
+
+	// Add jitter: random value between 0 and backoffCeiling
+	// rand.Int63n panics if argument is <= 0
+	if backoffCeiling <= 0 {
+		return 0 // No delay if ceiling is zero or negative
+	}
+
+	jitter := rand.Int63n(int64(backoffCeiling)) // #nosec G404 -- Using math/rand is acceptable for non-security critical jitter.
+	return time.Duration(jitter)
+}
+
 // OnRetry function callback are called each retry
 //
 // log each retry example:

retry.go

@@ -188,8 +188,6 @@ shouldRetry:
 			break
 		}
 
-		config.onRetry(n, err)
-
 		for errToCheck, attempts := range attemptsForError {
 			if errors.Is(err, errToCheck) {
 				attempts--
@@ -204,6 +202,9 @@ shouldRetry:
 		if n == config.attempts-1 {
 			break shouldRetry
 		}
+
+		config.onRetry(n, err)
+
 		n++
 		select {
 		case <-config.timer.After(delay(config, n, err)):

retry_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"os"
 	"testing"
 	"time"
@@ -35,7 +36,7 @@ func TestDoWithDataAllFailed(t *testing.T) {
 	assert.Len(t, err, 10)
 	fmt.Println(err.Error())
 	assert.Equal(t, expectedErrorFormat, err.Error(), "retry error format")
-	assert.Equal(t, uint(45), retrySum, "right count of retry")
+	assert.Equal(t, uint(36), retrySum, "right count of retry")
 }
 
 func TestDoFirstOk(t *testing.T) {
@@ -678,6 +679,28 @@ func TestAttemptsForErrorNoDelayAfterFinalAttempt(t *testing.T) {
 	assert.Less(t, totalTime, 300*time.Millisecond, "should not delay after final attempt")
 }
 
+func TestOnRetryNotCalledOnLastAttempt(t *testing.T) {
+	callCount := 0
+	onRetryCalls := make([]uint, 0)
+	
+	err := Do(
+		func() error {
+			callCount++
+			return errors.New("test error")
+		},
+		Attempts(3),
+		OnRetry(func(n uint, err error) {
+			onRetryCalls = append(onRetryCalls, n)
+		}),
+		Delay(time.Nanosecond),
+	)
+	
+	assert.Error(t, err)
+	assert.Equal(t, 3, callCount, "function should be called 3 times")
+	assert.Equal(t, []uint{0, 1}, onRetryCalls, "onRetry should only be called for first 2 attempts, not the final one")
+	assert.Len(t, onRetryCalls, 2, "onRetry should be called exactly 2 times (not on last attempt)")
+}
+
 func TestIsRecoverable(t *testing.T) {
 	err := errors.New("err")
 	assert.True(t, IsRecoverable(err))
@@ -688,3 +711,71 @@ func TestIsRecoverable(t *testing.T) {
 	err = fmt.Errorf("wrapping: %w", err)
 	assert.False(t, IsRecoverable(err))
 }
+
+func TestFullJitterBackoffDelay(t *testing.T) {
+	// Seed for predictable randomness in tests
+	// In real usage, math/rand is auto-seeded in Go 1.20+ or should be seeded once at program start.
+	// For library test predictability, local seeding is fine.
+	// However, retry-go's RandomDelay uses global math/rand without explicit seeding in tests.
+	// Let's follow the existing pattern of not explicitly seeding in each test for now,
+	// assuming test runs are isolated enough or that exact delay values aren't asserted,
+	// but rather ranges or properties.
+
+	baseDelay := 50 * time.Millisecond
+	maxDelay := 500 * time.Millisecond
+
+	config := &Config{
+		delay:    baseDelay,
+		maxDelay: maxDelay,
+		// other fields can be zero/default for this test
+	}
+
+	attempts := []uint{0, 1, 2, 3, 4, 5, 6, 10}
+
+	for _, n := range attempts {
+		delay := FullJitterBackoffDelay(n, errors.New("test error"), config)
+
+		expectedMaxCeiling := float64(baseDelay) * math.Pow(2, float64(n))
+		if expectedMaxCeiling > float64(maxDelay) {
+			expectedMaxCeiling = float64(maxDelay)
+		}
+
+		assert.True(t, delay >= 0, "Delay should be non-negative. Got: %v for attempt %d", delay, n)
+		assert.True(t, delay <= time.Duration(expectedMaxCeiling),
+			"Delay %v should be less than or equal to current backoff ceiling %v for attempt %d", delay, time.Duration(expectedMaxCeiling), n)
+
+		t.Logf("Attempt %d: BaseDelay=%v, MaxDelay=%v, Calculated Ceiling=~%v, Actual Delay=%v",
+			n, baseDelay, maxDelay, time.Duration(expectedMaxCeiling), delay)
+
+		// Test with MaxDelay disabled (0)
+		configNoMax := &Config{delay: baseDelay, maxDelay: 0}
+		delayNoMax := FullJitterBackoffDelay(n, errors.New("test error"), configNoMax)
+		expectedCeilingNoMax := float64(baseDelay) * math.Pow(2, float64(n))
+		if expectedCeilingNoMax > float64(10*time.Minute) { // Avoid overflow for very large N
+			expectedCeilingNoMax = float64(10 * time.Minute)
+		}
+		assert.True(t, delayNoMax >= 0, "Delay (no max) should be non-negative. Got: %v for attempt %d", delayNoMax, n)
+		assert.True(t, delayNoMax <= time.Duration(expectedCeilingNoMax),
+			"Delay (no max) %v should be less than or equal to current backoff ceiling %v for attempt %d", delayNoMax, time.Duration(expectedCeilingNoMax), n)
+	}
+
+	// Test case where baseDelay might be zero
+	configZeroBase := &Config{delay: 0, maxDelay: maxDelay}
+	delayZeroBase := FullJitterBackoffDelay(0, errors.New("test error"), configZeroBase)
+	assert.Equal(t, time.Duration(0), delayZeroBase, "Delay with zero base delay should be 0")
+
+	delayZeroBaseAttempt1 := FullJitterBackoffDelay(1, errors.New("test error"), configZeroBase)
+	assert.Equal(t, time.Duration(0), delayZeroBaseAttempt1, "Delay with zero base delay (attempt > 0) should be 0")
+
+	// Test with very small base delay
+	smallBaseDelay := 1 * time.Nanosecond
+	configSmallBase := &Config{delay: smallBaseDelay, maxDelay: 100 * time.Nanosecond}
+	for i := uint(0); i < 5; i++ {
+		d := FullJitterBackoffDelay(i, errors.New("test"), configSmallBase)
+		ceil := float64(smallBaseDelay) * math.Pow(2, float64(i))
+		if ceil > 100 {
+			ceil = 100
+		}
+		assert.True(t, d <= time.Duration(ceil))
+	}
+}