create core/errors package#128
Draft
justinwon777 wants to merge 3 commits into
Draft
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
requested changes
Jun 30, 2026
sbalabanov
left a comment
sbalabanov
left a comment
Contributor
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" | ||
| ) |
Contributor
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() } |
Contributor
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. |
Contributor
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")) |
Contributor
There was a problem hiding this comment.
why need for niche sentinels to be in a global package?
Contributor
Author
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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