Use ozzo-validation type for validation errors

[smacker]

My use case:

in validators I use repositories and services, the simplest example of such validator "email already exists". This validator goes to check email in db/redis/whatever through repository.

Repository returns user&error.

Error == NotFound means validation passed
Error == nil means email already exists
Error == anythingElse I want to stop validation and handle it in my handler.

And now it's impossible.

I implemented this feature for myself and I would love to bring it to your lib. But it will break all existing validators. Do you have any ideas how to implement this feature without breaking changes?

Thanks!

P.S. You can take a look at my implementation here: https://github.com/smacker/ozzo-validation

[qiangxue]

I noticed you introduced validation.Error and used it to differentiate it from non-validation errors. I think this is too strong - it is possible that a validator may want to return more information in the validation error.

Instead, introducing an interface (e.g. NonValidationError) might be better. That is, in the 3rd case of your example, you would create a new error implementing this interface.

Still, I'm not fully sure if this is the right approach. When you fail to validate Email due to a database error for example, shouldn't you still treat this as a validation error (something like "we are unable to check email uniqueness at this moment)? Internally you may log the error for debugging purpose.

[smacker]

I think we have quite different use cases, so I'll try to explain what I'm doing and why. Sorry for many words.

Let's start with the end. Non validation error means something went wrong. If we are building API it's important to differentiate errors because client will treat them differently. In the simplest example 422 validation error shouldn't be retried, but 500 internal error should.

Assume we are building POST api. Few possible errors which validator might return and should be mapped to different http code: 400 Bad Request - we might parse input json in validator. Incorrect json is Bad Request, not validation error. 401 Unauthorized - validator might need to make request to different service to validate input. If authorization needed for that server client should send request with auth headers. 408 Request time out - again in case of another service validation request can take too much time. We should return correct error to client. 422 Unprocessable Entity - regular validation error 500 Internal Server Error - it's archivable through panic, but I strongly believe validators shouldn't panic. If you open list of http codes (or grpc codes) you will find many other cases.

Why I need validation.Error instead of NonValidationError interface? When validator calls Repo/Service it can return thousands different errors because in Go it's very common to just return unknown error by a stack up and you never know what is underneath of that service. It is possible to wrap them in validator, but then I hide real error. I can't use common error handler anymore or it will require very strange code of "unwrapping" error.

Consider this code:

func (s *service) Handle(req *Req) error {
    if err := ozzo.Validate(req,
        ozzo.Field(&req.Field1, ozzo.Required),
        ozzo.Field(&req.Field2, ozzo.Required, ozzo.By(func (obj interface{}) error {
            err := s.externalService.ValidateField2(obj.(string))
            if _, ok := err.(externalService.ValidationError); ok {
                return validation.Error(err)
            }
            return err
        })),
    ); err != nil {
        return err
    }
    // some business logic here can cause some errors

    // or we can use external service again
    if err := s.externalService.CreateEntry(req.Field2); err != nil {
        return err
    }
    return nil
}

func (s *service) Response(err error) http.HandlerFunc {
    return func(w http.ResponseWriter, r *request.Request) {
        var statusCode = 200
        // handler known errors from any handler
        switch e := err.(type) {
            case validation.Errors:
                statusCode = 422
                break
            case repository.NotFound:
                statusCode = 404
                break
            case externalService.PaymentRequired:
                statusCode = 402
                break
            case error:
                statusCode = 500 // any unknown errors
                log(e)
        }
        w.WriteHeader(statusCode)
    }
}

this is simplified example of what I want.

Of course if such cases are out of your scope, it's absolutely okay, you can just close this issue.

[qiangxue]

I'm convinced that "422 validation error shouldn't be retried, but 500 internal error should".

I think the problem can still be solved nicely without requiring the introduction of validation.Error. Your validation may look like the following, while your error handler remains the same:

ozzo.Field(&req.Field2, ozzo.Required, ozzo.By(func (obj interface{}) error {
            err := s.externalService.ValidateField2(obj.(string))
            if _, ok := err.(externalService.ValidationError); ok {
                return err
            }
            return validation.NewNonValidationError(err)
        }))

ozzo-validation can be modified to check if a validator returns a NonValidationError or not. If so, it will unwrap it and stop further validations. What do you think?

[smacker]

Yeah. Thank you. It will work. But still will require manual unwrapping in case I use validator with a value, not a struct. (like validation.Validate(data, Validator)). Let me think few days is it okay for me or not. If it's okay, I'll make PR.