Merge pull request restic#4835 from MichaelEischer/fix-list-cancel

backend/retry: do not log final error if context was canceled
rawtaz · Jun 1, 2024 · 660679c · 660679c
2 parents 456ebfb + 38654a3
commit 660679c
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 6 deletions.
diff --git 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
@@ -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
 }