Merge branch 'master' into merge_131

Author: tstromberg

options.go

@@ -124,6 +124,8 @@ func BackOffDelay(n uint, _ error, config *Config) time.Duration {
 		config.maxBackOffN = max - uint(math.Floor(math.Log2(float64(config.delay))))
 	}
 
+	n--
+
 	if n > config.maxBackOffN {
 		n = config.maxBackOffN
 	}
@@ -158,6 +160,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

@@ -143,8 +143,8 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (
 		attemptsForError[err] = attempts
 	}
 
-	shouldRetry := true
-	for shouldRetry {
+shouldRetry:
+	for {
 		t, err := retryableFunc()
 		if err == nil {
 			return t, nil
@@ -160,21 +160,24 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (
 			break
 		}
 
-		config.onRetry(n, err)
-
 		for errToCheck, attempts := range attemptsForError {
 			if errors.Is(err, errToCheck) {
 				attempts--
 				attemptsForError[errToCheck] = attempts
-				shouldRetry = shouldRetry && attempts > 0
+				if attempts <= 0 {
+					break shouldRetry
+				}
 			}
 		}
 
 		// Setting attempts to 0 means we'll retry until we succeed
 		// if this is last attempt - don't wait
-		if config.attempts != 0 && n == config.attempts-1 {
-			break
+		if n == config.attempts-1 {
+			break shouldRetry
 		}
+
+		config.onRetry(n, err)
+
 		n++
 		select {
 		case <-config.timer.After(delay(config, n, err)):
@@ -185,7 +188,6 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (
 
 			return emptyT, append(errorLog, context.Cause(config.context))
 		}
-
 	}
 
 	if config.lastErrorOnly {

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) {
@@ -294,7 +295,7 @@ func TestBackOffDelay(t *testing.T) {
 			delay:         -1,
 			expectedMaxN:  62,
 			n:             2,
-			expectedDelay: 4,
+			expectedDelay: 2,
 		},
 		{
 			label:         "zero-delay",
@@ -310,6 +311,13 @@ func TestBackOffDelay(t *testing.T) {
 			n:             62,
 			expectedDelay: time.Second << 33,
 		},
+		{
+			label:         "one-second-n",
+			delay:         time.Second,
+			expectedMaxN:  33,
+			n:             1,
+			expectedDelay: time.Second,
+		},
 	} {
 		t.Run(
 			c.label,
@@ -490,7 +498,7 @@ func TestContext(t *testing.T) {
 	})
 
 	t.Run("timed out on retry infinte attempts - wraps context error with last retried function error", func(t *testing.T) {
-		ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
+		ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*200)
 		defer cancel()
 
 		retrySum := 0
@@ -633,6 +641,74 @@ func BenchmarkDoWithDataNoErrors(b *testing.B) {
 	}
 }
 
+type attemptsForErrorTestError struct{}
+
+func (attemptsForErrorTestError) Error() string { return "test error" }
+
+func TestAttemptsForErrorNoDelayAfterFinalAttempt(t *testing.T) {
+	var count uint64
+	var timestamps []time.Time
+	
+	startTime := time.Now()
+	
+	err := Do(
+		func() error {
+			count++
+			timestamps = append(timestamps, time.Now())
+			return attemptsForErrorTestError{}
+		},
+		Attempts(3),
+		Delay(200*time.Millisecond),
+		DelayType(FixedDelay),
+		AttemptsForError(2, attemptsForErrorTestError{}),
+		LastErrorOnly(true),
+		Context(context.Background()),
+	)
+	
+	endTime := time.Now()
+	
+	assert.Error(t, err)
+	assert.Equal(t, uint64(2), count, "should attempt exactly 2 times")
+	assert.Len(t, timestamps, 2, "should have 2 timestamps")
+	
+	// Verify timing: first attempt at ~0ms, second at ~200ms, end immediately after second attempt
+	firstAttemptTime := timestamps[0].Sub(startTime)
+	secondAttemptTime := timestamps[1].Sub(startTime)
+	totalTime := endTime.Sub(startTime)
+	
+	// First attempt should be immediate
+	assert.Less(t, firstAttemptTime, 50*time.Millisecond, "first attempt should be immediate")
+	
+	// Second attempt should be after delay
+	assert.Greater(t, secondAttemptTime, 150*time.Millisecond, "second attempt should be after delay")
+	assert.Less(t, secondAttemptTime, 250*time.Millisecond, "second attempt should not be too delayed")
+	
+	// Total time should not include delay after final attempt
+	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))
@@ -643,3 +719,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))
+	}
+}