From c314e8e838aaf4c7e45ea22d8ce0e0c03a7388a4 Mon Sep 17 00:00:00 2001 From: andig Date: Wed, 26 Aug 2026 14:48:33 +0200 Subject: [PATCH] Keep permanent sentinel errors distinguishable (#33194) --- api/error.go | 19 ++++++++++++------- api/error_test.go | 47 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/api/error.go b/api/error.go index 2b5ffe961..f71d16c34 100644 --- a/api/error.go +++ b/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 diff --git a/api/error_test.go b/api/error_test.go index 4dfceebe1..3f8231789 100644 --- a/api/error_test.go +++ b/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) +}