From a3633cad9e23d0bd936a394837874c7accbe97ce Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Mon, 29 Apr 2024 21:07:17 +0200
Subject: [PATCH] retry: explicitly log failed requests

This simplifies finding the request in the log output that cause an
operation to fail.
---
 cmd/restic/global.go                         |  6 +++-
 internal/backend/retry/backend_retry.go      | 30 +++++++++++++-------
 internal/backend/retry/backend_retry_test.go | 21 ++++++++++++++
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/cmd/restic/global.go b/cmd/restic/global.go
index 6920caa8d..d0facc674 100644
--- a/cmd/restic/global.go
+++ b/cmd/restic/global.go
@@ -416,7 +416,11 @@ func OpenRepository(ctx context.Context, opts GlobalOptions) (*repository.Reposi
 	}
 
 	report := func(msg string, err error, d time.Duration) {
-		Warnf("%v returned error, retrying after %v: %v\n", msg, d, err)
+		if d >= 0 {
+			Warnf("%v returned error, retrying after %v: %v\n", msg, d, err)
+		} else {
+			Warnf("%v failed: %v\n", msg, err)
+		}
 	}
 	success := func(msg string, retries int) {
 		Warnf("%v operation successful after %d retries\n", msg, retries)
diff --git a/internal/backend/retry/backend_retry.go b/internal/backend/retry/backend_retry.go
index 31934ec96..c6815413c 100644
--- a/internal/backend/retry/backend_retry.go
+++ b/internal/backend/retry/backend_retry.go
@@ -44,20 +44,28 @@ func New(be backend.Backend, maxTries int, report func(string, error, time.Durat
 // 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 {
+	var operationWrapper backoff.Operation
 	if success == nil {
-		return backoff.RetryNotify(operation, b, notify)
-	}
-	retries := 0
-	operationWrapper := func() error {
-		err := operation()
-		if err != nil {
-			retries++
-		} else if retries > 0 {
-			success(retries)
+		operationWrapper = operation
+	} else {
+		retries := 0
+		operationWrapper = func() error {
+			err := operation()
+			if err != nil {
+				retries++
+			} else if retries > 0 {
+				success(retries)
+			}
+			return err
 		}
-		return err
 	}
-	return backoff.RetryNotify(operationWrapper, b, notify)
+	err := backoff.RetryNotify(operationWrapper, b, notify)
+
+	if err != nil && notify != nil {
+		// log final error
+		notify(err, -1)
+	}
+	return err
 }
 
 var fastRetries = false
diff --git a/internal/backend/retry/backend_retry_test.go b/internal/backend/retry/backend_retry_test.go
index a515b0b7d..de86e6cf6 100644
--- a/internal/backend/retry/backend_retry_test.go
+++ b/internal/backend/retry/backend_retry_test.go
@@ -497,3 +497,24 @@ func TestNotifyWithSuccessIsCalled(t *testing.T) {
 		t.Fatalf("Success should have been called only once, but was called %d times instead", successCalled)
 	}
 }
+
+func TestNotifyWithSuccessFinalError(t *testing.T) {
+	operation := func() error {
+		return errors.New("expected error in test")
+	}
+
+	notifyCalled := 0
+	notify := func(error, time.Duration) {
+		notifyCalled++
+	}
+
+	successCalled := 0
+	success := func(retries int) {
+		successCalled++
+	}
+
+	err := retryNotifyErrorWithSuccess(operation, backoff.WithMaxRetries(&backoff.ZeroBackOff{}, 5), 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")
+}