Keep permanent sentinel errors distinguishable (#33197)

api/error.go

@@ -7,23 +7,28 @@ import (
 	"github.com/cenkalti/backoff/v4"
 )
 
-// permanentError is a sentinel error that keeps matching errors.Is after
-// backoff has stripped the backoff.Permanent wrapper.
+// permanentError is a sentinel error that signals permanence to backoff while
+// remaining distinguishable from the other permanent sentinels.
 type permanentError struct {
 	msg string
 }
 
 func (e *permanentError) Error() string { return e.msg }
 
-// Is matches the wrapped sentinel, too
-func (e *permanentError) Is(target error) bool {
-	var t *permanentError
-	return errors.As(target, &t) && t == e
+// As signals permanence to backoff. Wrapping the sentinel in backoff.Permanent
+// instead would make errors.Is match any other permanent error, since
+// backoff.PermanentError.Is matches by type rather than identity.
+func (e *permanentError) As(target any) bool {
+	if p, ok := target.(**backoff.PermanentError); ok {
+		*p = &backoff.PermanentError{Err: e}
+		return true
+	}
+	return false
 }
 
 // permanent creates a permanent sentinel error
 func permanent(msg string) error {
-	return backoff.Permanent(&permanentError{msg})
+	return &permanentError{msg}
 }
 
 // ErrNotAvailable indicates that a feature is not available

api/error_test.go

@@ -8,20 +8,47 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+// permanentSentinels are all errors created by permanent()
+var permanentSentinels = []error{
+	ErrNotAvailable,
+	ErrUnsupportedPlatform,
+	ErrSponsorRequired,
+	ErrMissingCredentials,
+	ErrMissingToken,
+}
+
 // Backoff returns permanent errors unwrapped. These must still match the sentinel.
 func TestPermanentSentinels(t *testing.T) {
-	for _, tc := range []struct{ err, other error }{
-		{ErrNotAvailable, ErrUnsupportedPlatform},
-		{ErrUnsupportedPlatform, ErrNotAvailable},
-		{ErrMissingCredentials, ErrMissingToken},
-		{ErrMissingToken, ErrMissingCredentials},
-	} {
+	for _, err := range permanentSentinels {
 		_, unwrapped := backoff.RetryWithData(func() (int, error) {
-			return 0, tc.err
+			return 0, err
 		}, &backoff.StopBackOff{})
 
-		assert.ErrorIs(t, unwrapped, tc.err)
-		assert.ErrorIs(t, fmt.Errorf("wrapped: %w", unwrapped), tc.err)
-		assert.NotErrorIs(t, unwrapped, tc.other)
+		assert.ErrorIs(t, unwrapped, err)
+		assert.ErrorIs(t, fmt.Errorf("wrapped: %w", unwrapped), err)
+		assert.ErrorIs(t, fmt.Errorf("wrapped: %w", err), err)
 	}
 }
+
+// Permanent sentinels must not match each other, whether returned directly or
+// unwrapped by backoff.
+func TestPermanentSentinelIdentity(t *testing.T) {
+	for _, err := range permanentSentinels {
+		_, unwrapped := backoff.RetryWithData(func() (int, error) {
+			return 0, err
+		}, &backoff.StopBackOff{})
+
+		for _, other := range permanentSentinels {
+			if other == err {
+				continue
+			}
+
+			assert.NotErrorIs(t, err, other)
+			assert.NotErrorIs(t, unwrapped, other)
+			assert.NotErrorIs(t, fmt.Errorf("wrapped: %w", err), other)
+		}
+	}
+
+	// login required is permanent, too
+	assert.NotErrorIs(t, LoginRequiredError("foo"), ErrNotAvailable)
+}