From 2a1bafe95a4f4c9c0902cb1e316cb72cb3777c05 Mon Sep 17 00:00:00 2001 From: Alvar Penning Date: Tue, 26 Mar 2024 15:31:02 +0100 Subject: [PATCH] retry: distinguish non-retryable errors from timeouts The WithBackoff function returns a wrapped error with the message "can't retry" both for errors marked as non-retryable by IsRetryable as well as when the context is done. The latter might happen when a positive settings.Timeout is specified, as this results in a wrapped context with a timeout. This behavior resulted in me debugging the IsRetryable function, when just the Timeout has exceeded. To save others from having this experience, I have adapted the messages. --- pkg/retry/retry.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/retry/retry.go b/pkg/retry/retry.go index 32d3a615e..63c9e65db 100644 --- a/pkg/retry/retry.go +++ b/pkg/retry/retry.go @@ -3,6 +3,7 @@ package retry import ( "context" "database/sql/driver" + "fmt" "github.com/go-sql-driver/mysql" "github.com/icinga/icingadb/pkg/backoff" "github.com/lib/pq" @@ -65,7 +66,7 @@ func WithBackoff( } if !isRetryable { - err = errors.Wrap(err, "can't retry") + err = errors.Wrap(err, "can't retry non-retryable error") return } @@ -76,10 +77,19 @@ func WithBackoff( if outerErr := parentCtx.Err(); outerErr != nil { err = errors.Wrap(outerErr, "outer context canceled") } else { + var errMsg string + if errors.Is(ctx.Err(), context.DeadlineExceeded) && settings.Timeout > 0 { + errMsg = fmt.Sprintf("can't retry as timeout of %v has been exceeded", settings.Timeout) + } else { + // This shouldn't happen as the context should either be the parentCtx or be + // wrapped as context.WithTimeout. However, just to be future-proof... + errMsg = "can't retry as subcontext is canceled" + } + if err == nil { err = ctx.Err() } - err = errors.Wrap(err, "can't retry") + err = errors.Wrap(err, errMsg) } return