Skip to content
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.

Sign up for GitHub

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

Add support for error aggregation for request/response validation #259

Merged
merged 5 commits into from
Oct 26, 2020
Merged

Add support for error aggregation for request/response validation #259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2ef4682
WIP for multi part errors
zlozano Oct 14, 2020
2e0ac59
add fail fast option
zlozano Oct 20, 2020
270f058
merge in upstream changes
zlozano Oct 21, 2020
f62b88b
complete multerrors for request/response validation; add tests
zlozano Oct 26, 2020
3255cbc
move allocation; remove TODO
zlozano Oct 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions openapi3/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package openapi3

import (
"bytes"
"errors"
)

// MultiError is a collection of errors, intended for when
// multiple issues need to be reported upstream
type MultiError []error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type MultiError []error
type MultiError []error
//Is allows you to determine if a generic error is in fact a MultiError using `errors.Is()`
//It will also return true if any of the contained errors match target
func (me MultiError) Is(target error) bool {
if _, ok := target.(MultiError); ok {
return true
}
for _, e := range me {
if errors.Is(e, target) {
return true
}
}
return false
}
//As allows you to use `errors.As()` to set target to the first error within the multi error that matches the target type
func (me MultiError) As(target interface{}) bool {
for _, e := range me {
if errors.As(e, target) {
return true
}
}
return false
}
// StatusCode Calculates the most specific http status code for the list of potential error codes
func (me MultiError) StatusCode() int {
code4xx := 0
code5xx := 0
for _, err := range me {
code := 0
if sc, ok := err.(StatusCoder); ok {
code = sc.StatusCode()
}
if code != 0 {
if 400 >= code && code <=499 {
if code4xx == 0 {
code4xx = code
} else if code4xx != code {
code4xx = 400
}
} else if 500 >= code && code <=599 {
if code5xx == 0 {
code5xx = code
} else if code5xx != code {
code5xx = 500
}
}
}
}
if code5xx != 0 {
return code5xx
}
return code4xx
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the input! I am about to push up the rest of my changes, and I will add this in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the Is and the As.

The StatusCoder is defined in the openapi3filter package, so including here would introduce an import cycle. I think this is fine for now since the rules used above for calculating the status code are not universal. Also, it looks like the StatusCoder is only implemented by the ValidationError.

I'm not super familiar with the ValidationError/ErrorEncoder, so I am not quite sure how it would look, but I think what would be needed is a way to convert a MultiError into a ValidationError (similar to what is done for SchemaErrors and RequestErrors. It seems like this work could be done separately/as a follow up to this since the error will get passed-through to the encoder, allowing the end user to control this behavior. Related to my previous comment, it's not clear how exactly we would calculate error precedence if there are many errors. So this may be the best course of action for now. I'll defer to those who have more experience with using this feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given go's preference for interfaces being defined where they are consumed it would have been acceptable to redefine StatusCoder within this package or use an anonymous interface like some of the core packages. Ex: if sc, ok := err.(interface{StatusCode() string}); ok {

It is fine to leave the StatusCode function out for now. I actually had it as a standalone function in the proof of concept that I was working on.

I agree we should have a separate discussion about conversion to ValidationError. I had a hard time implementing my own ErrorEncoder.


func (me MultiError) Error() string {
buff := &bytes.Buffer{}
for _, e := range me {
buff.WriteString(e.Error())
buff.WriteString(" | ")
}
return buff.String()
}

//Is allows you to determine if a generic error is in fact a MultiError using `errors.Is()`
//It will also return true if any of the contained errors match target
func (me MultiError) Is(target error) bool {
if _, ok := target.(MultiError); ok {
return true
}
for _, e := range me {
if errors.Is(e, target) {
return true
}
}
return false
}

//As allows you to use `errors.As()` to set target to the first error within the multi error that matches the target type
func (me MultiError) As(target interface{}) bool {
for _, e := range me {
if errors.As(e, target) {
return true
}
}
return false
}
Loading