Skip to content

Commit

Permalink
retry: Explicitly check context for error
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lippserd committed Apr 8, 2024
1 parent 2344e0a commit b2fd89f
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion pkg/retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down

0 comments on commit b2fd89f

Please sign in to comment.