Wrapping unrecoverable error makes it impossible to detect
Nitjsefni7 opened this issue · 2 comments
Nitjsefni7 commented
When wrapping error already wrapped with retry.Unrevocerable()
it is not going to be properly detected by retry.IsRecoverable()
.
Example:
package main
import (
"errors"
"fmt"
"github.com/avast/retry-go/v4"
)
func main() {
err := retry.Unrecoverable(errors.New("unrecoverable err"))
errWr := fmt.Errorf("wrapping: %w", err)
// This is going to be printed.
if !retry.IsRecoverable(err) {
fmt.Println("unrecoverable")
}
// This is not going to be printed.
if !retry.IsRecoverable(errWr) {
fmt.Println("wrapped unrecoverable")
}
}
Suggestion - while checking if error is recoverable, continue to unwrap error if possible:
// IsRecoverable checks if error is an instance of `unrecoverableError`
func IsRecoverable(err error) bool {
for {
if _, isUnrecoverable := err.(unrecoverableError); isUnrecoverable {
return false
}
if err = errors.Unwrap(err); err == nil {
return true
}
}
}
JaSei commented
This should be fixed by 4.3.2 properly - can you please check it? Thanks
jamescostian commented
@JaSei it's still broken - the issue is this code, which should be replaced with:
// IsRecoverable checks if error is an instance of `unrecoverableError`
func IsRecoverable(err error) bool {
return !errors.Is(err, unrecoverableError{})
}
// Adds support for errors.Is usage on unrecoverableError
func (unrecoverableError) Is(err error) bool {
_, isUnrecoverable := err.(unrecoverableError)
return isUnrecoverable
}
This way, go will be able to continuously call .Unwrap
on errors to find the real error, and on the unrecoverableError
, it can call the Is
method to see that it is indeed an unrecoverableError
.
To see this in action, compare the updated code to the existing code.