From fbf051ddfa2198062f9d88d97def157426771de3 Mon Sep 17 00:00:00 2001 From: sergeyb Date: Fri, 27 Feb 2026 17:45:53 +0000 Subject: [PATCH] feat(errors): No retryable user error --- core/errs/README.md | 14 ++++++++------ core/errs/errs.go | 27 +++++++++------------------ core/errs/errs_test.go | 17 +++-------------- 3 files changed, 20 insertions(+), 38 deletions(-) diff --git a/core/errs/README.md b/core/errs/README.md index 1c138b58..84ff3f47 100644 --- a/core/errs/README.md +++ b/core/errs/README.md @@ -8,13 +8,15 @@ Errors are classified along two axes: | | Non-retryable (default) | Retryable | |---------------|-------------------------|----------------------------------| -| **User** | `NewUserError` | `NewRetryableUserError` | +| **User** | `NewUserError` | *(not supported)* | | **Infra** | *(any unclassified error)* | `NewRetryableError` | -| **Dependency**| `NewDependencyError` | `NewRetryableDependencyError` | +| **Infra dep** | `NewDependencyError` | `NewRetryableDependencyError` | -**Non-retryable by default.** A plain `fmt.Errorf(...)` is treated as a non-retryable infra error. Retryability must be explicitly opted into by wrapping with `NewRetryableError` or `NewRetryableUserError`. This prevents accidental infinite retry loops from unclassified errors. +**Non-retryable by default.** A plain `fmt.Errorf(...)` is treated as a non-retryable infra error. Retryability must be explicitly opted into by wrapping with `NewRetryableError`. This prevents accidental infinite retry loops from unclassified errors. -**Infra by default.** Any error that is not explicitly wrapped with `NewUserError` or `NewRetryableUserError` is an infra error. There is no `NewInfraError` constructor — infra is the default classification. +**Only infra errors can be retryable.** User errors are never retryable — if a user action caused the failure, retrying the same operation will produce the same result. If an error is retryable, it is by definition an infrastructure issue. + +**Infra by default.** Any error that is not explicitly wrapped with `NewUserError` is an infra error. There is no `NewInfraError` constructor — infra is the default classification. ## Who Classifies Errors @@ -58,7 +60,7 @@ return errs.NewUserError(fmt.Errorf("lookup failed: %w", extensionErr)) // All of these work on the resulting error: errs.IsUserError(err) // true — framework classification -errs.IsRetryable(err) // false — non-retryable user error +errs.IsRetryable(err) // false — user errors are never retryable errors.Is(err, ErrNotFound) // true — cause is in the chain ``` @@ -67,5 +69,5 @@ errors.Is(err, ErrNotFound) // true — cause is in the chain | Helper | Returns `true` when | |---------------------|--------------------------------------------------------------| | `IsUserError(err)` | `err` is or wraps a `userError` | -| `IsRetryable(err)` | `err` is or wraps an error with the retryable flag set | +| `IsRetryable(err)` | `err` is or wraps an infra error with the retryable flag set | | `IsDependencyError(err)` | `err` is or wraps an infra error marked as dependency | diff --git a/core/errs/errs.go b/core/errs/errs.go index e8208bb1..1cf65668 100644 --- a/core/errs/errs.go +++ b/core/errs/errs.go @@ -5,24 +5,19 @@ import ( ) // userError represents an error caused by invalid user input or actions. -// Use NewUserError or NewRetryableUserError to wrap an underlying cause. +// User errors are never retryable — only infrastructure errors can be retryable. +// Use NewUserError to wrap an underlying cause. type userError struct { // cause is the underlying error. cause error - // retryable indicates whether the operation can be retried. - retryable bool } -// NewUserError creates a non-retryable user error wrapping the given cause. -// A user error is an error that is caused by the user's action or input, for example an invalid input or a merge conflict. +// NewUserError creates a user error wrapping the given cause. +// A user error is an error that is caused by the user's action or input, +// for example an invalid input or a merge conflict. User errors are never +// retryable — only infrastructure errors can be retryable. func NewUserError(cause error) error { - return &userError{cause: cause, retryable: false} -} - -// NewRetryableUserError creates a retryable user error wrapping the given cause. It is very rare for the user -// error to be retryable and often a result of misclassification. Please know what you are doing when you use this. -func NewRetryableUserError(cause error) error { - return &userError{cause: cause, retryable: true} + return &userError{cause: cause} } // Error returns the error message. @@ -104,14 +99,10 @@ func IsUserError(err error) bool { } // IsRetryable checks if err is retryable. Returns true only when err is or -// wraps an error whose retryable flag is set. A generic -// error (not wrapped) returns false, consistent +// wraps an infrastructure error whose retryable flag is set. User errors are +// never retryable. A generic error (not wrapped) returns false, consistent // with the convention that unclassified errors are non-retryable. func IsRetryable(err error) bool { - var ue *userError - if errors.As(err, &ue) { - return ue.retryable - } var ie *infraError if errors.As(err, &ie) { return ie.retryable diff --git a/core/errs/errs_test.go b/core/errs/errs_test.go index ae28c26a..cb351e4f 100644 --- a/core/errs/errs_test.go +++ b/core/errs/errs_test.go @@ -46,11 +46,6 @@ func TestIsUserError(t *testing.T) { err: NewUserError(errors.New("bad input")), want: true, }, - { - name: "retryable user error", - err: NewRetryableUserError(errors.New("rate limited")), - want: true, - }, { name: "retryable infra error", err: NewRetryableError(errors.New("db down")), @@ -87,15 +82,10 @@ func TestIsRetryable(t *testing.T) { want: false, }, { - name: "non-retryable user error", + name: "user error is not retryable", err: NewUserError(errors.New("bad input")), want: false, }, - { - name: "retryable user error", - err: NewRetryableUserError(errors.New("rate limited")), - want: true, - }, { name: "retryable infra error", err: NewRetryableError(errors.New("connection reset")), @@ -107,7 +97,7 @@ func TestIsRetryable(t *testing.T) { want: true, }, { - name: "wrapped non-retryable user error", + name: "wrapped user error is not retryable", err: fmt.Errorf("handler: %w", NewUserError(errors.New("invalid"))), want: false, }, @@ -183,11 +173,10 @@ func TestIsDependencyError(t *testing.T) { func TestErrorsAs(t *testing.T) { t.Run("extract user error from chain", func(t *testing.T) { cause := errors.New("invalid email") - wrapped := fmt.Errorf("validation: %w", NewRetryableUserError(cause)) + wrapped := fmt.Errorf("validation: %w", NewUserError(cause)) var ue *userError require.True(t, errors.As(wrapped, &ue)) - assert.True(t, ue.retryable) assert.True(t, errors.Is(ue, cause)) })