cenkalti/backoff

Error stack inconsistency when using PermanentError

VinceDeslo opened this issue · 0 comments

Good day!

While using the library, I noticed that wrapped errors get swallowed by the Permanent Error check. This impedes any other checks against nested errors from a higher level.

Just looking to validate if this is expected behaviour or if I should open a PR to update the return to err instead of permanent.Err in the check. This feels inconsistent with how retries handle non-permanent errors, WDYT?

Example program:

package main

import (
	"errors"
	"fmt"

	"github.com/cenkalti/backoff/v4"
)

func main() {
	expBackoff := backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 1)
	wrapErr := errors.New("Operation error")

	// First retry to illustrate without a permanent error in the error stack.
	operation1 := func() error {
		opErr := ErrStack(false)
		if opErr != nil {
			return errors.Join(wrapErr, opErr)
		}
		return nil
	}
	err := backoff.Retry(operation1, expBackoff)
	fmt.Printf("Error stack without permanent error:\n %s\n\n", err)

	// Second retry to illustrate with a permanent error in the error stack.
	operation2 := func() error {
		opErr := ErrStack(true)
		if opErr != nil {
			return errors.Join(wrapErr, opErr)
		}
		return nil
	}
	err = backoff.Retry(operation2, expBackoff)
	fmt.Printf("Error stack with permanent error:\n %s\n", err)
}

// Main call in the operation
func ErrStack(withPermanent bool) error {
	wrapErr := errors.New("Routine 1 failed")
	err := ErrStackDeep(withPermanent)

	return errors.Join(wrapErr, err)
}

// Nested calls with wrapped errors
func ErrStackDeep(withPermanent bool) error {
	wrapErr := errors.New("Routine 2 failed")
	upstreamErr := errors.New("Some failure")

	if withPermanent {
		return errors.Join(
			wrapErr,
			backoff.Permanent(upstreamErr),
		)
	}
	return errors.Join(wrapErr, upstreamErr)
}

Program output

Error stack without permanent error:
 Operation error
Routine 1 failed
Routine 2 failed
Some failure

Error stack with permanent error:
 Some failure

In the above snippet, you can see that only the deeply nested error is returned when using PermanentError.

Playground link:
https://goplay.tools/snippet/wIj6yFprbId