diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index 5c9b955..10bdd91 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -23,7 +23,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: ['1.20', '1.21', '1.22', '1.23', '1.24'] + go-version: ['1.20', '1.21', '1.22', '1.23', '1.24', '1.25'] os: [ubuntu-latest, macos-latest, windows-latest] env: OS: ${{ matrix.os }} diff --git a/VERSION b/VERSION index 8ac28bf..f6cdf40 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.6.1 +4.7.0 diff --git a/go.mod b/go.mod index bf1c650..af995f0 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,8 @@ module github.com/avast/retry-go/v4 -go 1.18 +go 1.20 -require github.com/stretchr/testify v1.10.0 +require github.com/stretchr/testify v1.11.1 require ( github.com/davecgh/go-spew v1.1.1 // indirect diff --git a/go.sum b/go.sum index 713a0b4..c4c1710 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= -github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/options.go b/options.go index 5577ee7..3ae28bb 100644 --- a/options.go +++ b/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: diff --git a/retry.go b/retry.go index 1812a1b..62d6392 100644 --- a/retry.go +++ b/retry.go @@ -132,7 +132,7 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) ( opt(config) } - if err := config.context.Err(); err != nil { + if err := context.Cause(config.context); err != nil { return emptyT, err } @@ -161,9 +161,9 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) ( case <-config.timer.After(delay(config, n, err)): case <-config.context.Done(): if config.wrapContextErrorWithLastError { - return emptyT, Error{config.context.Err(), lastErr} + return emptyT, Error{context.Cause(config.context), lastErr} } - return emptyT, config.context.Err() + return emptyT, context.Cause(config.context) } } } @@ -175,8 +175,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 @@ -194,26 +194,26 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) ( if errors.Is(err, errToCheck) { attempts-- attemptsForError[errToCheck] = attempts - shouldRetry = shouldRetry && attempts > 0 + if attempts <= 0 { + break shouldRetry + } } } // if this is last attempt - don't wait if n == config.attempts-1 { - break + break shouldRetry } n++ select { case <-config.timer.After(delay(config, n, err)): case <-config.context.Done(): if config.lastErrorOnly { - return emptyT, config.context.Err() + return emptyT, context.Cause(config.context) } - return emptyT, append(errorLog, config.context.Err()) + return emptyT, append(errorLog, context.Cause(config.context)) } - - shouldRetry = shouldRetry && n < config.attempts } if config.lastErrorOnly { diff --git a/retry_test.go b/retry_test.go index 1ee3739..b71010a 100644 --- a/retry_test.go +++ b/retry_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math" "os" "testing" "time" @@ -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, @@ -489,7 +497,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 @@ -642,3 +650,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)) + } +}