From 9a38d4c1c7982e1c675a1b715a55f955263dee2c Mon Sep 17 00:00:00 2001 From: Yuya Okumura Date: Mon, 26 Dec 2022 14:46:41 +0900 Subject: [PATCH 1/2] fix timeout bug Fixed a problem related to timeout config: ctx was being overwritten when the timeout config value was set. As a result, the next retry would immediately timeout. Fixed to not overwrite ctx when timeout config value was set. --- async_retry.go | 4 ++-- async_retry_test.go | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/async_retry.go b/async_retry.go index a20f1eb..d85c177 100644 --- a/async_retry.go +++ b/async_retry.go @@ -94,9 +94,9 @@ func (a *asyncRetry) call(ctx context.Context, f RetryableFunc, config *Config) return retry.Do( func() error { if config.timeout > 0 { - var timeoutCancel context.CancelFunc - ctx, timeoutCancel = context.WithTimeout(ctx, config.timeout) + timeoutCtx, timeoutCancel := context.WithTimeout(ctx, config.timeout) defer timeoutCancel() + return f(timeoutCtx) } return f(ctx) }, diff --git a/async_retry_test.go b/async_retry_test.go index 70ef6c0..979fe36 100644 --- a/async_retry_test.go +++ b/async_retry_test.go @@ -124,9 +124,16 @@ func Test_asyncRetry_Do(t *testing.T) { name: "Timeout set correctly for each try", args: args{ f: func(ctx context.Context) error { + started := time.Now() counter++ select { case <-ctx.Done(): + // Since the timeout for this test case is 10 msec, + // even considering the error of the measurement, + // at least 9 msec should have elapsed by the time `ctx.Done()` is received. + if time.Since(started) < (9 * time.Millisecond) { + return Unrecoverable(fmt.Errorf("timeout is too fast")) + } if counter < 3 { return fmt.Errorf("timeout") } From daa4e7bd4711e3ef267157cde6bfc46cc4d5e87c Mon Sep 17 00:00:00 2001 From: Yuya Okumura Date: Mon, 26 Dec 2022 18:06:01 +0900 Subject: [PATCH 2/2] changed to check ctx is not closed at the start of processing --- async_retry_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/async_retry_test.go b/async_retry_test.go index 979fe36..7611f1a 100644 --- a/async_retry_test.go +++ b/async_retry_test.go @@ -124,16 +124,16 @@ func Test_asyncRetry_Do(t *testing.T) { name: "Timeout set correctly for each try", args: args{ f: func(ctx context.Context) error { - started := time.Now() + select { + case <-ctx.Done(): + // Check ctx passed from async-retry is not closed at the start of f() processing. + return fmt.Errorf("context already closed") + default: + } + counter++ select { case <-ctx.Done(): - // Since the timeout for this test case is 10 msec, - // even considering the error of the measurement, - // at least 9 msec should have elapsed by the time `ctx.Done()` is received. - if time.Since(started) < (9 * time.Millisecond) { - return Unrecoverable(fmt.Errorf("timeout is too fast")) - } if counter < 3 { return fmt.Errorf("timeout") } @@ -147,7 +147,7 @@ func Test_asyncRetry_Do(t *testing.T) { }, opts: []Option{ Delay(1 * time.Millisecond), - Timeout(10 * time.Millisecond), + Timeout(1 * time.Second), Attempts(5), }, },