-
-
Notifications
You must be signed in to change notification settings - Fork 206
feat: implement UnmarshalerError #705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #705 +/- ##
==========================================
+ Coverage 77.94% 78.01% +0.06%
==========================================
Files 22 22
Lines 7998 7996 -2
==========================================
+ Hits 6234 6238 +4
+ Misses 1349 1345 -4
+ Partials 415 413 -2 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new error type, UnmarshalerError, to wrap errors produced by the decodeByUnmarshaler function and prevent double-wrapping of errors.
- Adds the UnmarshalerError struct and its associated methods in internal/errors/error.go
- Exposes UnmarshalerError via the root error.go file
- Updates decode.go to wrap errors from decodeByUnmarshaler (and map-key decoding) using ErrUnmarshal
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/errors/error.go | Added new UnmarshalerError type and error formatting |
| error.go | Exposed UnmarshalerError via an alias |
| decode.go | Updated error returns to wrap decoding errors with ErrUnmarshal |
goccy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the test case for this changes
a1d59f0 to
3a13581
Compare
| } | ||
| var te *errors.TypeError | ||
| if errors.As(err, &te) { | ||
| if te, ok := err.(*errors.TypeError); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will stop working if the err is wrapped somewhere. Why did you change the sequence using As ?
| }) | ||
| } | ||
|
|
||
| func TestUnmarshalableErrors(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is only a test case for UnmarshalYAML([]byte) error, so please add tests for other cases as well. In the tests, ensure that you can obtain an UnmarshalerError and verify that the content of its Error() matches the expected message.
Adds a new error type,
UnmarshalerErrorwhich is used to wrap any errors returned fromdecodeByUnmarshalerCloses #704