User-friendly error messages for type validation

[Ch00k]

Let's say I have a PUT method that must be given a JSON of this structure:

{ "imdb_data": { "id": 1234567 } }

My code then would look like this:

def validate_imdb_data_json(val):
    if not isinstance(val, dict):
        raise ValidationError('imdb_data is not a dict')
    # further validations of inner dict

imdb_data_json = {'imdb_data': Arg(dict, required=True, validate=validate_imdb_data_json)}

class MovieIMDBDataAPI(Resource):
    def __init__(self):
        super(MovieIMDBDataAPI, self).__init__()

    @use_args(imdb_data_json, targets=('json',))
    def put(self, args, movie_id):
        print args

The problem I have is with webargs' type coversion, namely this part of code. So if I provide this kind of JSON: { "imdb_data": 1 }, my custom validation function validate_imdb_data_json will not even get a chance to work because validation will fail before it gets called, on line 172. And I will result in a response like this:

HTTP/1.0 400 BAD REQUEST
Content-Type: application/json
Content-Length: 50
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: Content-Type
Server: Werkzeug/0.9.6 Python/2.7.8
Date: Sat, 29 Nov 2014 11:37:29 GMT

{
    "message": "'int' object is not iterable"
}

which is actually the result of an attempt to do dict(1) (btw this is again weird because the code catches ValueError but this kind of error is a TypeError, but this is a different conversation).
The only way I can think of to work around this is to not pass type_ at all. Then all the validation will be done by validate_imdb_data_json function. But that does not look right to me. Is there something I am missing?
This is somewhat similar to #18.

[Ch00k]

Actually it's not only the complex types really. You will likely have the same problem if you try int("foo")

[sloria]

Indeed, type validation occurs before passing the value to the validator(s).

The rationale behind this that user validators are expected to receive a value of type type_ as input. If the passed value cannot be cast to a value of type type_, the user validators will not make sense and the validation should fail fast.

re: The TypeError being caught implicitly: that is the expected behavior, but I'm realizing now that it is no optimal. I will open a separate issue for it.

Thanks for reporting!

[Ch00k]

The motivation of this kind of validation is clear but that breaks the whole idea of user-friendly error messages. The simplest example - the user providing a string where an integer is required - already "breaks" the error handling.
A quick solution I just came up with could be something like the following:

try:
    ret = self.type(ret)
except:
    msg = 'Expected type {}, got {}'.format(JSON_TYPES[self.type.__name__],
                                            JSON_TYPES[type(ret).__name__])
    raise ValidationError(self.error or msg)

with a basic map of JSON types:

JSON_TYPES = dict(
    int='Integer',
    str='String',
    dict='Object',
    list='List'
)

[sloria]

Yes, I think the current error messages for type validation can definitely use improvement. I think you have the start of a good solution. I would suggest using the terminology used in JSON Schema for the primitive types. Also, a default error message should be used if the value is not in the JSON_TYPES mapping.

If you would like to send a pull request, I would gladly review and merge it!

[Ch00k]

Thanks! I'll work on a pull request.

[sloria]

@Ch00k Thank you for offering to work on this. I have updated the title of the issue to more accurately reflect the problem being solved.

[Ch00k]

@sloria Sure. It looks like the idea of having proper type validation is a lot more difficult to implement than I initially thought. Specifically because of the need to support different targets and also different Python versions. I already started experimenting and created a PR #24. It's still very far from ready, lots of existing tests fail but still please take a look if you have any feedback on this.

[nickretallack]

I'm coming here from flask-restful / reqparse. I like how it doesn't differentiate between type coercion and validation. For example, I can do this:

def to_bool(value, name):
    if isinstance(value, bool):
        return value
    if isinstance(value, basestring):
        if value.lower() in ['true','yes','on']:
            return True
        if value.lower() in ['false','no','off']:
            return False
    raise ValueError("The parameter '{}' should be true/yes/on or false/no/off, but it was {}".format(name, value))

from flask.ext.restful import reqparse
parser = reqparse.RequestParser()
parser.add_argument('cool', type=to_bool, required=True)

In most situations it'd probably be easiest to just write a little function like this that handles both type coercion and validation all at once.

[sloria]

Giving validators a single point of responsibility is beneficial for maintainability and reuseability. It also makes validators easier to write because they only have to worry about one type of input.

If you really want to do both type coercion and validation in the same function, you can still pass the use argument.

from webargs import Arg, ValidationError

def to_bool(value):
    if isinstance(value, bool):
        return value
    if isinstance(value, basestring):
        if value.lower() in ['true','yes','on']:
            return True
        if value.lower() in ['false','no','off']:
            return False
    raise ValidationError("The parameter '{}' should be true/yes/on or false/no/off, but it was {}".format(name, value))

args = {
    'cool': Arg(use=to_bool, required=True)
}

[sloria]

Closed by #24.