More info in ValidationErrors

[gazpachoking]

In my project, I want to create user facing error messages which are friendlier than the default ones. I'm thinking we could attach the instance value, as well as the value for the schema keyword that failed to the ValidationError to facilitate this.

The instance could already be grabbed by walking the path through the instance, but it would be a bit easier if it is already attached.

[gazpachoking]

Not exactly sure what should be included if it was allOf that failed. It would be nice to be able to tell which schemas were the failing ones in that case.

[Julian]

as well as the value for the schema keyword

Is this ValidationError.validator?

And as for the instance, reduce(operator.methodcaller("__getitem__"), error.path, instance) or thereabouts should do it, though sure I wouldn't be opposed to adding that as a property on the error.

[gazpachoking]

Is this ValidationError.validator?

That just stores the keyword, no? In addition to knowing minItems failed, I'd like to also know that minItems was 3, for example.

[gazpachoking]

And yes, knowing the path is certainly enough to get the value of the instance back, was just thinking it'd be nice to have directly in the error.

[Julian]

Ah. So yeah that's schema["minItems"] unless you have to deal with nested schemas or whatever in which case you have to look at path too. So yeah that's probably a good candidate for another property I guess.

[gazpachoking]

Yeah, it's much tougher to trace the path on the schema side for nested schemas, since a path of a/0/1 might trace to properties/a/items/0/additionalItems

[Julian]

Indeed. OK, sounds good to me if we can do it reasonably.

[gazpachoking]

The more complicated case is similar to #51, when the types validator (in draft 3), or the anyOf, oneOf, or not keywords might have child errors that you are interested in. Currently the anyOf validator is the worst in my case, where the default error message of 'The instance is not valid under any of the given schemas.' is not very helpful at all.

[gazpachoking]

Perhaps we can do something with the casue attribute for these cases. The issue is that there may be more than one cause.

[gazpachoking]

This is not fully related, but perhaps we could also redo the error class hierarchy such that all of the errors were subclasses of a JsonschemaError. We could handle the message attribute in there, as well as __str__ stuff. This would enable somebody to try validation and catch JsonschemaErrors no matter whether they were from validation, metavalidaton or ref resolution.

[Julian]

I avoid doing that to avoid encouraging people to be overzealous and catch more than they should. There's nothing wrong with except (ValidationError, SchemaError) if that makes sense (catching those along with much more than that though would be silly most likely).

[gazpachoking]

Sounds reasonable. I'll write up something for the simpler cases I mentioned here later tonight, those more complex cases I need to think about what information is most useful and how to best deliver it more.

[Julian]

Sounds good to me.

[gazpachoking]

Just trying to collect my thoughts here.

I think you are right that it is easy enough to get at the instance using the path, so perhaps attaching that is not needed. Along those lines, maybe providing something like validator_path would be better than just providing the value given to the validator. The value could be easily looked up in the schema using that path.

As for anyOf/oneOf/not... What I would like, is to be able to get the list of errors from each of the possible subschemas that were validated. How those should be packaged and attach them to the error, I'm still not sure. allOf is currently handled differently, in that the allOf validator will never cause a validation error itself, the subschema errors are already the ones returned. If we come up with a clean solution to package the child errors, maybe that should be converted to use it as well.

[gazpachoking]

Perhaps if we just have a list of child errors that would be fine. If each of them had the validator path attached to them they could then be separated into the child schemas they came from if desired.

[gazpachoking]

I'm liking that idea. I'll put something together tomorrow unless you have any objections or suggestions.

[Julian]

Sounds reasonable as long as we find a nice way to attach that to the errors.

[gazpachoking]

Hmm. $ref sort of messes up being able to give a tracable validator path.

[Julian]

Well, we'd need to store the final-reffed schema on the error and then a path within that.

All this bookkeeping though suggests that we might want to have something like a ValidationContext object or something that is responsible for keeping all this information and then finalizing the validation errors as they go out.

[gazpachoking]

Yeah, might be okay, the issue I have is that if the following schema fails, I'd like to be able to sort out which child errors came from which of the 3 schemas. If the validator path is just relative to the last ref context that information gets lost.

{
    "anyOf": [
        {"$ref": "a"},
        {"$ref": "b"},
        {"$ref": "c"}
    ]
}

Not quite sure how a ValidationContext might work, you just mean we'd set these paths on the context, rather than on the errors themselves?

[Julian]

All I meant with a context object is that it'd be nice to not shove tons of junk on an exception object. I probably shouldn't have even started with doing that, but whoops! So if we're gonna start with even more detail, it'd be nice if we move towards"this is an error. If you want to know how the hell we got here, here's some other object".

As for how exactly that other object should work to support what you want I haven't thought about yet.

[Julian]

Essentially I think we are (re) inventing tracebacks, where stack frames are schemas and function calls are ref resolutions and we want a nice way to allow traversing them.

Though I'm not sure if thinking about itlike that helps at all.

[gazpachoking]

So, if we attach the value for the keyword directly to the error (or context object) I think having the path just contain $ref is fine. The path wouldn't be directly traversable, but you wouldn't really need that if you already have the value.

[gazpachoking]

Okay, started thinking about this again. Going to try to whip something up after I finish some coursework today. As for having extra details in a context object, you are just thinking another class with all the details which gets attached as an attribute to ValidationError?

[Julian]

Yeah. Not a big deal, we can work that out after we get the interface down.
On Mar 17, 2013 5:24 PM, "Chase Sterling" [email protected] wrote:

Okay, started thinking about this again. Going to try to whip something up
after I finish some coursework today. As for having extra details in a
context object, you are just thinking another class with all the details
which gets attached as an attribute to ValidationError?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/79#issuecomment-15031219
.

[gazpachoking]

Okay, started a bit of work. I added in a schema_path so far, didn't separate it out into a separate object yet, just put it on ValidationError for now. It ignores refs in the path, as if the json reference object was replaced with the actual schema that it points to. Didn't write any tests yet. I'll work on it more tomorrow.
https://github.com/gazpachoking/jsonschema/tree/validationerror_context

[gazpachoking]

I think the schema path is a good thing, I'm no longer too worried about not being able to walk it out without redoing ref resolution. I'm thinking add the validator configuration in there as well, and I'm also tempted to attach the whole subschema. This would mean that all 3 arguments that were passed in to the validate_ method that failed were available on the error. Once all that stuff is in, I think attaching a list of suberrors for validators like anyOf and oneOf(and type for draft 3) will be useful, and they will have enough context to understand where they came from. Thoughts?

[Julian]

Sounds good to me, I have to either see or work out exactly how many and what attributes will have what in a slightly more complicated case to get a clearer picture but it sounds like we're headed in a reasonable direction if I'm imagining it right.

[gazpachoking]

I added all the attributes I was thinking of directly to ValidationError for now. Here's what I have:

• validator_arg
• instance
• subschema

Those are the three agruments that get passed into the validate function

• context

This is a list of errors that caused the current validation error (used in anyOf, oneOf, and draft 3 style type validators)

• schema_path

This is the path to the validator keyword in the schema that failed. In the case of errors in the context list, the path and schema_path are relative to the path and schema_path of the parent error.

gazpachoking@741c275

[gazpachoking]

So, what properties do you think should stay directly in ValidationError other than the message?

[Julian]

I changed my mind maybe. Not sure I care about separating these off.

I think in the tiny bit I've looked at these seem OK. Not sure I love all the names but I don't have any suggestions yet.

[gazpachoking]

Yeah, I don't like all the names either. I'll work on some tests in the mean time, let me know if you have any good ideas for better names.

[gazpachoking]

Opened a pull for easier tracking of progress on the branch. Added a unit test and fixed the implementation when used in nested schemas.

[Julian]

Looking good.

Only thing I'm concerned about is keeping it easy for people writing validators to manage this properly. E.g. I think all the attributes you're setting should only be set if they haven't been already. And we need to update the docs.

And maybe just schema instead of subschema is good enough? And validator_value instead of arg?

[gazpachoking]

Yeah, those names are probably better. Maybe rename validator to validator_keyword too? It seems like that would fit better as a pair with validator_value, although it might not be worth changing what was already there. And I'll fix it up so the conditional is per attribute, although they should all be getting set at the same time, and none of them should be getting set from the validator except for context (and the paths).

[Julian]

Sounds good to me along with a deprecation warning on a property.

[Julian]

Maybe giving ValidationError (`_Error) a method to do all this default setting is reasonable.

[Julian]

Closing this, we're close to finishing the PR so rest of the discussion can go there.