From 38654a3bd7a740fb363eff68223f10072af035c6 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 30 May 2024 18:48:52 +0200 Subject: [PATCH] backend/retry: do not log final error if context was canceled Calls to `List(ctx, ...)` are usually stopped by canceling the context once no further entries are required by the caller. Thus, don't log the final error if the used context was canceled. --- internal/backend/retry/backend_retry.go | 6 ++--- internal/backend/retry/backend_retry_test.go | 25 +++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/internal/backend/retry/backend_retry.go b/internal/backend/retry/backend_retry.go index d5134d433..8d0f42bfd 100644 --- a/internal/backend/retry/backend_retry.go +++ b/internal/backend/retry/backend_retry.go @@ -43,7 +43,7 @@ func New(be backend.Backend, maxElapsedTime time.Duration, report func(string, e // retryNotifyErrorWithSuccess is an extension of backoff.RetryNotify with notification of success after an error. // success is NOT notified on the first run of operation (only after an error). -func retryNotifyErrorWithSuccess(operation backoff.Operation, b backoff.BackOff, notify backoff.Notify, success func(retries int)) error { +func retryNotifyErrorWithSuccess(operation backoff.Operation, b backoff.BackOffContext, notify backoff.Notify, success func(retries int)) error { var operationWrapper backoff.Operation if success == nil { operationWrapper = operation @@ -61,8 +61,8 @@ func retryNotifyErrorWithSuccess(operation backoff.Operation, b backoff.BackOff, } err := backoff.RetryNotify(operationWrapper, b, notify) - if err != nil && notify != nil { - // log final error + if err != nil && notify != nil && b.Context().Err() == nil { + // log final error, unless the context was canceled notify(err, -1) } return err diff --git a/internal/backend/retry/backend_retry_test.go b/internal/backend/retry/backend_retry_test.go index cd0c4d48b..fd76200d4 100644 --- a/internal/backend/retry/backend_retry_test.go +++ b/internal/backend/retry/backend_retry_test.go @@ -460,7 +460,7 @@ func TestNotifyWithSuccessIsNotCalled(t *testing.T) { t.Fatal("Success should not have been called") } - err := retryNotifyErrorWithSuccess(operation, &backoff.ZeroBackOff{}, notify, success) + err := retryNotifyErrorWithSuccess(operation, backoff.WithContext(&backoff.ZeroBackOff{}, context.Background()), notify, success) if err != nil { t.Fatal("retry should not have returned an error") } @@ -486,7 +486,7 @@ func TestNotifyWithSuccessIsCalled(t *testing.T) { successCalled++ } - err := retryNotifyErrorWithSuccess(operation, &backoff.ZeroBackOff{}, notify, success) + err := retryNotifyErrorWithSuccess(operation, backoff.WithContext(&backoff.ZeroBackOff{}, context.Background()), notify, success) if err != nil { t.Fatal("retry should not have returned an error") } @@ -515,12 +515,31 @@ func TestNotifyWithSuccessFinalError(t *testing.T) { successCalled++ } - err := retryNotifyErrorWithSuccess(operation, backoff.WithMaxRetries(&backoff.ZeroBackOff{}, 5), notify, success) + err := retryNotifyErrorWithSuccess(operation, backoff.WithContext(backoff.WithMaxRetries(&backoff.ZeroBackOff{}, 5), context.Background()), notify, success) test.Assert(t, err.Error() == "expected error in test", "wrong error message %v", err) test.Equals(t, 6, notifyCalled, "notify should have been called 6 times") test.Equals(t, 0, successCalled, "success should not have been called") } +func TestNotifyWithCancelError(t *testing.T) { + operation := func() error { + return errors.New("expected error in test") + } + + notify := func(error, time.Duration) { + t.Error("unexpected call to notify") + } + + success := func(retries int) { + t.Error("unexpected call to success") + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := retryNotifyErrorWithSuccess(operation, backoff.WithContext(&backoff.ZeroBackOff{}, ctx), notify, success) + test.Assert(t, err == context.Canceled, "wrong error message %v", err) +} + type testClock struct { Time time.Time }