create core/errors package#128
Conversation
Introduces a new core/errors package that defines: - ClassifiedError struct wrapping any error with ErrorType and Reason fields for metrics - New(errorType, reason string, err error) constructor - Pre-constructed User and Infra sentinel vars (no-arg messages) - Named struct types for all Handleable-Structured errors (User and Infra) so callers can use errors.As to extract specific fields This package will replace core/common's ClassifiedError interface and WithReason function, and all bare fmt.Errorf/errors.New call sites, in a follow-up change. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Restore ErrParseTimestamp and ErrPRCommitHistory as typed structs with Cause field so callers can type-discriminate via errors.As - Add errors_test.go covering: New constructor, errors.As/Is traversal through ClassifiedError, sentinel identity, and Error() strings for all structured types Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
|
justinwon777
force-pushed
the
justin.won/core-errors
branch
from
d0e4bfe to
17f5794
Compare
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
justinwon777
force-pushed
the
justin.won/core-errors
branch
from
17f5794 to
bc5ac9c
Compare
sbalabanov
left a comment
sbalabanov
left a comment
There was a problem hiding this comment.
plz schedule Design Review for errors processing/classification before implementation.
alternatively, come up with an rfc-only PR
| FailureReasonUnknown = "unknown" | ||
| FailureReasonStorage = "storage" | ||
| FailureReasonValidation = "validation" | ||
| ) |
There was a problem hiding this comment.
should type and reason be custom types (enums) then
| Err error | ||
| } | ||
|
|
||
| func (e *ClassifiedError) Error() string { return e.Err.Error() } |
There was a problem hiding this comment.
anything exported should have a doc
| func (e *ClassifiedError) Error() string { return e.Err.Error() } | ||
| func (e *ClassifiedError) Unwrap() error { return e.Err } | ||
|
|
||
| // New constructs a ClassifiedError. |
There was a problem hiding this comment.
explain usage i.e. args
| ErrFirstRevisionRequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first revision is required")) | ||
| ErrSecondRevisionRequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("second revision is required")) | ||
| ErrFirstRevisionRemoteRequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first revision remote is required")) | ||
| ErrFirstRevisionBaseSHARequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first revision base_sha is required")) |
There was a problem hiding this comment.
why need for niche sentinels to be in a global package?
This is WIP and I'm reviewing the design with Sung today. Changed it to draft state. I can let you know once it's ready. |
3 participants
Introduces a new core/errors package that defines:
so callers can use errors.As to extract specific fields
This package will replace core/common's ClassifiedError interface and
WithReason function, and all bare fmt.Errorf/errors.New call sites,
in a follow-up change.
Current failure reasons are copied from common package. Follow up will expand the list so we don't have unknown failures.
Test Plan
Unit tests
Issue
https://linear.app/uber/issue/TANGO-4/improve-tango-error-reporting