From 51a6ef25b8ce98376f95cacda874e910e90c2b35 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Mon, 8 Apr 2024 15:56:28 +0200 Subject: [PATCH] `retry`: Explicitly check context for error The retryable function may exit prematurely due to context errors that shouldn't be retried. Before, we checked the returned error for context errors, i.e. used `errors.Is()` to compare it to `Canceled` and `DeadlineExceeded` which also yields `true` for errors that implement `Is()` accordingly. For example, this applies to some non-exported Go `net` errors. Now we explicitly check the context error instead. --- pkg/retry/retry.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/retry/retry.go b/pkg/retry/retry.go index a76f2c5d..fb3b20b9 100644 --- a/pkg/retry/retry.go +++ b/pkg/retry/retry.go @@ -61,7 +61,12 @@ func WithBackoff( return } - if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { + // Retryable function may have exited prematurely due to context errors. + // We explicitly check the context error here, as the error returned by the retryable function can pass the + // error.Is() checks even though it is not a real context error, e.g. + // https://cs.opensource.google/go/go/+/refs/tags/go1.22.2:src/net/net.go;l=422 + // https://cs.opensource.google/go/go/+/refs/tags/go1.22.2:src/net/net.go;l=601 + if errors.Is(ctx.Err(), context.DeadlineExceeded) || errors.Is(ctx.Err(), context.Canceled) { if prevErr != nil { err = errors.Wrap(err, prevErr.Error()) }