From 27bc46172aae612c6ab718bbffbcf5771a9ed15d Mon Sep 17 00:00:00 2001 From: Will Date: Wed, 2 Aug 2023 21:32:38 +0100 Subject: [PATCH 1/4] Allow last error to be returned with context error --- README.md | 24 ++++++++++++++++++++++++ options.go | 46 +++++++++++++++++++++++++++++++++++----------- retry.go | 8 +++++++- retry_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index dcda9a1..141f68b 100644 --- a/README.md +++ b/README.md @@ -369,6 +369,30 @@ retry.Do( ) +#### func WrapContextErrorWithLastError + +```go +func WrapContextErrorWithLastError(wrapContextErrorWithLastError bool) Option +``` +WrapContextErrorWithLastError allows the context error to be returned wrapped +with the last error that the retried function returned. This is only applicable +when Attempts is set to 0 to retry indefinitly and when using a context to +cancel / timeout + +default is false + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + retry.Do( + func() error { + ... + }, + retry.Context(ctx), + retry.Attempts(0), + retry.WrapContextErrorWithLastError(true), + ) + #### type RetryIfFunc ```go diff --git a/options.go b/options.go index ea9687f..e931b3a 100644 --- a/options.go +++ b/options.go @@ -23,17 +23,18 @@ type Timer interface { } type Config struct { - attempts uint - attemptsForError map[error]uint - delay time.Duration - maxDelay time.Duration - maxJitter time.Duration - onRetry OnRetryFunc - retryIf RetryIfFunc - delayType DelayTypeFunc - lastErrorOnly bool - context context.Context - timer Timer + attempts uint + attemptsForError map[error]uint + delay time.Duration + maxDelay time.Duration + maxJitter time.Duration + onRetry OnRetryFunc + retryIf RetryIfFunc + delayType DelayTypeFunc + lastErrorOnly bool + context context.Context + timer Timer + wrapContextErrorWithLastError bool maxBackOffN uint } @@ -250,3 +251,26 @@ func WithTimer(t Timer) Option { c.timer = t } } + +// WrapContextErrorWithLastError allows the context error to be returned wrapped with the last error that the +// retried function returned. This is only applicable when Attempts is set to 0 to retry indefinitly and when +// using a context to cancel / timeout +// +// default is false +// +// ctx, cancel := context.WithCancel(context.Background()) +// defer cancel() +// +// retry.Do( +// func() error { +// ... +// }, +// retry.Context(ctx), +// retry.Attempts(0), +// retry.WrapContextErrorWithLastError(true), +// ) +func WrapContextErrorWithLastError(wrapContextErrorWithLastError bool) Option { + return func(c *Config) { + c.wrapContextErrorWithLastError = wrapContextErrorWithLastError + } +} diff --git a/retry.go b/retry.go index 8e40cf2..8c1c692 100644 --- a/retry.go +++ b/retry.go @@ -95,6 +95,7 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { } // Setting attempts to 0 means we'll retry until we succeed + var lastErr error if config.attempts == 0 { for err := retryableFunc(); err != nil; err = retryableFunc() { if !IsRecoverable(err) { @@ -105,12 +106,17 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { return err } + lastErr = err + n++ config.onRetry(n, err) select { case <-config.timer.After(delay(config, n, err)): case <-config.context.Done(): - return config.context.Err() + if !config.wrapContextErrorWithLastError { + return config.context.Err() + } + return fmt.Errorf("%w: %w", config.context.Err(), lastErr) } } diff --git a/retry_test.go b/retry_test.go index 5b03462..c62579e 100644 --- a/retry_test.go +++ b/retry_test.go @@ -451,6 +451,45 @@ func TestContext(t *testing.T) { assert.Equal(t, 2, retrySum, "called at most once") }() }) + + t.Run("cancelled on retry infinte attempts - wraps context error with last retried function error", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + retrySum := 0 + err := Do( + func() error { return fooErr{str: fmt.Sprintf("error %d", retrySum+1)} }, + OnRetry(func(n uint, err error) { + retrySum += 1 + if retrySum == 2 { + cancel() + } + }), + Context(ctx), + Attempts(0), + WrapContextErrorWithLastError(true), + ) + assert.ErrorIs(t, err, context.Canceled) + assert.ErrorIs(t, err, fooErr{str: "error 2"}) + }) + + 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) + defer cancel() + + retrySum := 0 + err := Do( + func() error { return fooErr{str: fmt.Sprintf("error %d", retrySum+1)} }, + OnRetry(func(n uint, err error) { + retrySum += 1 + }), + Context(ctx), + Attempts(0), + WrapContextErrorWithLastError(true), + ) + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.ErrorIs(t, err, fooErr{str: "error 2"}) + }) } type testTimer struct { From a1e1efa9e81a4e725a353770cedc0446b598b613 Mon Sep 17 00:00:00 2001 From: Will Date: Wed, 2 Aug 2023 21:40:01 +0100 Subject: [PATCH 2/4] switch the if statement --- retry.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/retry.go b/retry.go index 8c1c692..4a44d07 100644 --- a/retry.go +++ b/retry.go @@ -113,10 +113,10 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { select { case <-config.timer.After(delay(config, n, err)): case <-config.context.Done(): - if !config.wrapContextErrorWithLastError { - return config.context.Err() + if config.wrapContextErrorWithLastError { + return fmt.Errorf("%w: %w", config.context.Err(), lastErr) } - return fmt.Errorf("%w: %w", config.context.Err(), lastErr) + return config.context.Err() } } From e31035e12f2b92d4c58dd9538e4ad4ad60fe6b89 Mon Sep 17 00:00:00 2001 From: Will Date: Wed, 2 Aug 2023 22:05:04 +0100 Subject: [PATCH 3/4] ci on PR --- .github/workflows/workflow.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/workflow.yaml b/.github/workflows/workflow.yaml index a48fe87..35d0d39 100644 --- a/.github/workflows/workflow.yaml +++ b/.github/workflows/workflow.yaml @@ -1,6 +1,10 @@ name: Go -on: [push] +on: + push: + pull_request: + branches: + - master jobs: golangci-lint: From 9b38e2e054ce82c90763cdbea0a06ada8189764c Mon Sep 17 00:00:00 2001 From: Will Date: Thu, 3 Aug 2023 19:33:29 +0100 Subject: [PATCH 4/4] make use of retry library multi error --- retry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/retry.go b/retry.go index 4a44d07..68a9f72 100644 --- a/retry.go +++ b/retry.go @@ -114,7 +114,7 @@ func Do(retryableFunc RetryableFunc, opts ...Option) error { case <-config.timer.After(delay(config, n, err)): case <-config.context.Done(): if config.wrapContextErrorWithLastError { - return fmt.Errorf("%w: %w", config.context.Err(), lastErr) + return Error{config.context.Err(), lastErr} } return config.context.Err() }