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.

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

Useful code in Validation errors? #4

Closed
ethanfrey opened this issue Aug 25, 2016 · 3 comments
Closed

Useful code in Validation errors? #4

ethanfrey opened this issue Aug 25, 2016 · 3 comments

Comments

@ethanfrey
Copy link

ethanfrey commented Aug 25, 2016

Hi, I came across this package, as I am auto-generating my input and output models using go-swagger. This is very nice, as I am able to generate my parsing and validation logic from the same source as my documentation, making the design -> documentation <-> development workflow very streamlined. So, first off, thanks for all the hard work you all have done to make such a useful toolkit.

However, I came across a usability issue with the errors, I would like to address, but would like feedback before making a fork. In short, the error code for all validation errors is exactly the same (422), which is problematic, when I want to generate custom validation errors in our api. Our api returns validation erros for each field with an error code, which is translated in the frontend (javascript SPA, iOS app). We use one error code for required, one for duplicate, one for must be true, one for invalid length, one for invalid data, etc... So, I was writing some code to translate the go-openapi/errors/Validation type into our custom type. But there is no easy way to introspect an error to find its cause.

Basically, my input validation code (auto-generated by go-swagger) looks something like this:

// Validate validates this create link params
func (m *CreateLinkParams) Validate(formats strfmt.Registry) error {
    var res []error
    if err := m.validateDestID(formats); err != nil {
        // prop
        res = append(res, err)
    }
        // ... validate other fields ....
    if len(res) > 0 {
        return errors.CompositeValidationError(res...)
    }
    return nil
}

func (m *CreateLinkParams) validateDestID(formats strfmt.Registry) error {
    if err := validate.Required("dest_id", "body", m.DestID); err != nil {
        return err
    }
    return nil
}

validate.Required (from go-openapi/validate) is this:

// Required validates an interface for requiredness
func Required(path, in string, data interface{}) *errors.Validation {
    val := reflect.ValueOf(data)
    if val.IsValid() {
        if reflect.DeepEqual(reflect.Zero(val.Type()).Interface(), val.Interface()) {
            return errors.Required(path, in)
        }
        return nil
    }
    return errors.Required(path, in)
}

and errors.Required (from go-openapi/errors) is this:

// Required error for when a value is missing
func Required(name, in string) *Validation {
    var msg string
    if in == "" {
        msg = fmt.Sprintf(requiredFailNoIn, name)
    } else {
        msg = fmt.Sprintf(requiredFail, name, in)
    }
    return &Validation{
        code:    422,
        Name:    name,
        In:      in,
        message: msg,
    }
}

Name gives me the field name that has an error (very nice), In is always body for parsing POST/PUT body, message (returned by Error()) is some text that does depend on the error type, and code (returned by Code()) is always 422 for every error.

I now have three approaches.

  1. Just refer to any field that fails validation as invalid, giving the error message as extra context, which makes our response not so useful and not translatable.
  2. Try to parse out the Error() text to figure out if it is a required, too long, etc. error. Adn then set the proper error code. This is possible but ugly and error-prone.
  3. Attach some useful code variable to each different type of Validation error that could be used by a switch statement to determine the type (ideally as a set of exportyed named constants form the errors package.

The third choice seems the most stable, extensible, and useful for me and others, but I don't know if this would break some other package. Or if there was a conscious decision to use this http error code here?

If you like the approach, I would be happy to prepare a pull request with approach number 3

Cheers,
Ethan

@casualjim
Copy link
Member

I like option 3, and I'd welcome a PR like that. It's a good enhancement.

And then just fyi, I'm not sure if you've you seen this hook?

https://github.com/go-swagger/go-swagger/blob/master/examples/todo-list/restapi/configure_todo_list.go#L25

This ultimately ends up going here:
https://github.com/go-openapi/errors/blob/master/api.go#L115

Which gives you a place to translate swagger errors into something more acceptable for your app.

@ethanfrey
Copy link
Author

Great, I'll work on the PR soon.

The hook is interesting, but I only generate the models from swagger. We have an existing code base with lots of custom code and I am adding swagger elements to a new feature we develop. The models are easy to import, the rest requires a rework of the whole app....

@ethanfrey
Copy link
Author

go get -u github.com/go-openapi/errors and updated my vendors directory. Switching based on Validation.Code() works well in my project. I'll call this closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants