From 912b0c5559fb3149bd9e7ba3dc6fac5f46eb0449 Mon Sep 17 00:00:00 2001 From: Niv Keidan Date: Mon, 10 Jun 2024 12:23:50 +0300 Subject: [PATCH 1/8] extract context errors with context.Cause and not ctx.Err() to support users using CancelCause --- go.mod | 2 +- retry.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index bf1c650..2e2071b 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/avast/retry-go/v4 -go 1.18 +go 1.20 require github.com/stretchr/testify v1.10.0 diff --git a/retry.go b/retry.go index 1812a1b..5338985 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) } } } @@ -207,10 +207,10 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) ( 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 From 10df0c15507f53a58c22876846d15c711afe70a9 Mon Sep 17 00:00:00 2001 From: "amirreza.fahimidero" Date: Fri, 30 May 2025 14:15:15 +0330 Subject: [PATCH 2/8] feature: add FullJitterBackoffDelay --- .idea/workspace.xml | 168 ++++++++++++++++++++++++++++++++++++++++++++ options.go | 29 ++++++++ retry_test.go | 69 ++++++++++++++++++ 3 files changed, 266 insertions(+) create mode 100644 .idea/workspace.xml diff --git a/.idea/workspace.xml b/.idea/workspace.xml new file mode 100644 index 0000000..894b82c --- /dev/null +++ b/.idea/workspace.xml @@ -0,0 +1,168 @@ + + + + + + + + + + + + + + + + + {} + { + "isMigrated": true +} + { + "associatedIndex": 6 +} + + + + { + "keyToString": { + "DefaultGoTemplateProperty": "Go File", + "Go Test.ExampleFullJitterBackoffDelay in github.com/avast/retry-go/v4.executor": "Run", + "Go Test.TestCustomRetryFunction in github.com/avast/retry-go/v4/examples.executor": "Run", + "Go Test.TestFullJitterBackoffDelay in github.com/avast/retry-go/v4.executor": "Run", + "ModuleVcsDetector.initialDetectionPerformed": "true", + "RunOnceActivity.GoLinterPluginOnboarding": "true", + "RunOnceActivity.GoLinterPluginStorageMigration": "true", + "RunOnceActivity.ShowReadmeOnStart": "true", + "RunOnceActivity.git.unshallow": "true", + "RunOnceActivity.go.formatter.settings.were.checked": "true", + "RunOnceActivity.go.migrated.go.modules.settings": "true", + "RunOnceActivity.go.modules.go.list.on.any.changes.was.set": "true", + "git-widget-placeholder": "feature/full-jitter-delay", + "go.import.settings.migrated": "true", + "go.sdk.automatically.set": "true", + "last_opened_file_path": "/Users/homayoun/dev/retry-go", + "node.js.detected.package.eslint": "true", + "node.js.selected.package.eslint": "(autodetect)", + "nodejs_package_manager_path": "npm" + } +} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 1748600159971 + + + + + + + true + + \ No newline at end of file diff --git a/options.go b/options.go index 5577ee7..81aed1f 100644 --- a/options.go +++ b/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: diff --git a/retry_test.go b/retry_test.go index 1ee3739..97364a3 100644 --- a/retry_test.go +++ b/retry_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math" "os" "testing" "time" @@ -642,3 +643,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)) + } +} From 3b6a3184ae9f5a60ef49aa00417b5c5a4bc8437c Mon Sep 17 00:00:00 2001 From: "amirreza.fahimidero" Date: Fri, 30 May 2025 14:15:55 +0330 Subject: [PATCH 3/8] ops: remove unnecessary files --- .idea/workspace.xml | 168 -------------------------------------------- 1 file changed, 168 deletions(-) delete mode 100644 .idea/workspace.xml diff --git a/.idea/workspace.xml b/.idea/workspace.xml deleted file mode 100644 index 894b82c..0000000 --- a/.idea/workspace.xml +++ /dev/null @@ -1,168 +0,0 @@ - - - - - - - - - - - - - - - - - {} - { - "isMigrated": true -} - { - "associatedIndex": 6 -} - - - - { - "keyToString": { - "DefaultGoTemplateProperty": "Go File", - "Go Test.ExampleFullJitterBackoffDelay in github.com/avast/retry-go/v4.executor": "Run", - "Go Test.TestCustomRetryFunction in github.com/avast/retry-go/v4/examples.executor": "Run", - "Go Test.TestFullJitterBackoffDelay in github.com/avast/retry-go/v4.executor": "Run", - "ModuleVcsDetector.initialDetectionPerformed": "true", - "RunOnceActivity.GoLinterPluginOnboarding": "true", - "RunOnceActivity.GoLinterPluginStorageMigration": "true", - "RunOnceActivity.ShowReadmeOnStart": "true", - "RunOnceActivity.git.unshallow": "true", - "RunOnceActivity.go.formatter.settings.were.checked": "true", - "RunOnceActivity.go.migrated.go.modules.settings": "true", - "RunOnceActivity.go.modules.go.list.on.any.changes.was.set": "true", - "git-widget-placeholder": "feature/full-jitter-delay", - "go.import.settings.migrated": "true", - "go.sdk.automatically.set": "true", - "last_opened_file_path": "/Users/homayoun/dev/retry-go", - "node.js.detected.package.eslint": "true", - "node.js.selected.package.eslint": "(autodetect)", - "nodejs_package_manager_path": "npm" - } -} - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 1748600159971 - - - - - - - true - - \ No newline at end of file From e330bceda0635d367d49c324ae28bda6df3bd1d1 Mon Sep 17 00:00:00 2001 From: StounhandJ Date: Fri, 6 Jun 2025 17:17:30 +0300 Subject: [PATCH 4/8] no delay after final retry on max attempts --- retry.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/retry.go b/retry.go index 5338985..62d6392 100644 --- a/retry.go +++ b/retry.go @@ -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,13 +194,15 @@ 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 { @@ -212,8 +214,6 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) ( return emptyT, append(errorLog, context.Cause(config.context)) } - - shouldRetry = shouldRetry && n < config.attempts } if config.lastErrorOnly { From 6c62c209b5af94fb000269cd3ea985b25e788552 Mon Sep 17 00:00:00 2001 From: StounhandJ Date: Fri, 6 Jun 2025 18:19:16 +0300 Subject: [PATCH 5/8] BackOffDelay multiplies attempts from zero --- options.go | 2 ++ retry_test.go | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/options.go b/options.go index 5577ee7..f79fa98 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 } diff --git a/retry_test.go b/retry_test.go index 1ee3739..2a1ec11 100644 --- a/retry_test.go +++ b/retry_test.go @@ -294,7 +294,7 @@ func TestBackOffDelay(t *testing.T) { delay: -1, expectedMaxN: 62, n: 2, - expectedDelay: 4, + expectedDelay: 2, }, { label: "zero-delay", @@ -310,6 +310,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 +496,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 From 459fade57178b54cf274e5113eff8f4133b1f048 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 28 Aug 2025 05:27:49 +0000 Subject: [PATCH 6/8] Bump github.com/stretchr/testify from 1.10.0 to 1.11.1 Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.10.0 to 1.11.1. - [Release notes](https://github.com/stretchr/testify/releases) - [Commits](https://github.com/stretchr/testify/compare/v1.10.0...v1.11.1) --- updated-dependencies: - dependency-name: github.com/stretchr/testify dependency-version: 1.11.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 2e2071b..af995f0 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/avast/retry-go/v4 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= From 0bdef9ca6be8ab1a946ebbfbb3203e7e7ca27e5d Mon Sep 17 00:00:00 2001 From: Jan Seidl Date: Tue, 14 Oct 2025 20:36:12 +0200 Subject: [PATCH 7/8] ci(workflow): add Go version 1.25 to test matrix for expanded compatibility testing --- .github/workflows/workflow.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 }} From 375037b55dceec39815fd3d0c5bfbccf30255a38 Mon Sep 17 00:00:00 2001 From: Jan Seidl Date: Tue, 14 Oct 2025 20:38:31 +0200 Subject: [PATCH 8/8] bump version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 8ac28bf..f6cdf40 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.6.1 +4.7.0