feat(errors): ErrorModel implement errors.Unwrap#777
feat(errors): ErrorModel implement errors.Unwrap#777burgesQ wants to merge 1 commit intodanielgtaylor:mainfrom
Conversation
Allow compatibility for errors returned by huma.NewError* with the std' errors.As/errors.Is
18555a5 to
814bc11
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the ErrorModel to support error unwrapping in order to be compatible with std errors.As/errors.Is.
- Adds an unexported field "errors []error" to hold wrapped errors.
- Implements the Unwrap() method to return the slice of errors.
- Updates the Add() and NewError() functions to populate this new field.
Comments suppressed due to low confidence (1)
error.go:97
- Consider using a pointer receiver for the Unwrap method (i.e., func (e *ErrorModel) Unwrap() []error) to maintain consistency with other methods like Error() and Add(), ensuring that modifications to the ErrorModel are consistently observed.
func (e ErrorModel) Unwrap() []error { return e.errors }
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #777 +/- ##
=======================================
Coverage 93.10% 93.11%
=======================================
Files 23 23
Lines 5296 5301 +5
=======================================
+ Hits 4931 4936 +5
Misses 313 313
Partials 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // Value: 5 | ||
| // }) | ||
| func (e *ErrorModel) Add(err error) { | ||
| e.errors = append(e.errors, err) |
There was a problem hiding this comment.
This is great, but huma.NewError doesn't actually call Add on the error when constructing the details, and the same is potentially the case when manually returning error models in other code (and handlers in your service). I wonder if there is a better way to support most uses instead of requiring Add to get called 🤔
There was a problem hiding this comment.
Maybe smth like errors.Joins on already existing .Errors field ?
There was a problem hiding this comment.
Maybe a simple edit to the nvm it's already published with such changeNewError method would already help a bit
There was a problem hiding this comment.
Hehe I think we're good !
My changes were for some code that did some stuff like return nil, huma.Error500...("error", ErrorSpecialType{err}). Turn out my "middleware" tests pass if I change it to smth like return nil, ErrorSpecialType{huma.Error500...("error", err)
Allow compatibility for errors returned by huma.NewError* with the std' errors.As/errors.Is