Improvement: Schema build to report what custom validator failed

[JohanLorenzo]

Thank you very much for this library 😃 Mozilla uses it to validate structured dictionaries in several projects. For instance this and that ones.

I noticed an improvement we could make when custom validators are failing. As of now, schema builders don't tell what function failed. This cost me about half a day to understand that task_provisionerId_test() (in link 2) was causing the error:

    def validate_with_humanized_errors(data, schema, max_sub_error_length=MAX_VALIDATION_ERROR_ITEM_LENGTH):
        try:
            return schema(data)
        except (Invalid, MultipleInvalid) as e:
>           raise Error(humanize_error(data, e, max_sub_error_length))
E           Error: not a valid value for dictionary value @ data['tasks'][10]['task']. Got {'priority': 'high', 'provisionerId': 'aws-provisioner-v1', 'expires': '3017-04-10T17:55:13.477520+02:00', 'metadata': {'source': 'https://github.com/mozilla/releasetasks', 'name': 'firefox email release-drivers foo-None', 'description': 'Sends email to release-drivers telling updates are ready on foo-None\n', 'owner': 'release@mozilla.com'}, 'routes': ['index.releases.v1.foo.abcdef123456.firefox.42_0b2.build3.email', 'index.releases.v1.foo.latest.firefox.latest.email'], 'payload': {'maxRunTi...

(task being a fairly-sized dict). The bug was definitely on our side. Nonetheless, I think a simpler error would have been:

  Error: not a valid value for dictionary value @ data['tasks'][10]['task']. task_provisionerId_test() is not True. Got: ...

Long story short, I made a reduced test case which fits in a few lines.

[tusharmakkar08]

Hey @JohanLorenzo

Thanks for reaching out to us. We are happy that Voluptuous is being used at Mozilla.

We also aim to make the error more human-readable/friendly. I guess the issue is with the message displayed when custom validators are added in Voluptuous. Can you please give a small concrete schema example which tells what the required inputs and expected outputs are?

Thanks.

[alecthomas]

This feels like it would be useful for debugging, but not so much for users? Perhaps adding context to the exception would be sufficient, such as a reference to the validator that failed, though it would be quite a lot of work to add it everywhere.

What do you think @tusharmakkar08 ?

[JohanLorenzo]

Hey guys!

Yeah, this will only be useful for debugging. I think if the function name is just added to the error message, that's gonna be helpful enough.

For example, this small schema highlights the issue. The expected output to me is: the same error message which contains the validator name.

[tusharmakkar08]

@alecthomas Yes, this will be only useful for debugging purpose but as @JohanLorenzo mentioned it can reduce the debugging time phenomenally (Half a day to maybe few minutes). I am not however sure about the effort which would incur in incorporating the change. Would need to look into it.