Custom errors from format checkers?

[python-jsonschema]

Custom errors from format checkers?

[Julian]

On the train so I'll be terse and come back to this later with a full response, but I don't really want to encourage client code to be directly raising ValidationErrors. They should wrap ValidationErrors in their own exception (possibly also named ValidationError) but just passing them through to users isn't what I had in mind for these. So if you have ideas on something that would make that easier (wrapping them, pulling out information from them that can help you construct your message) I'd love to hear those but otherwise I'm hesitant unless you can make a case. There's another issue which asked for something similar to this that I was also hesitant on if you want to dig it up too.

[Julian]

I guess for format the issue is that each checker has its own notions about what things are invalid. So maybe this does make sense since up until now all the possible reasons an instance could be invalid were relatively known, so we could design ValidationError to provide all the information someone would want. That won't work here though so I see what you mean.

Need to think more later.

[gazpachoking]

Yeah, I'm not certain the best way to go about it yet, that was just the first thought that came to mind.

[gazpachoking]

Hmm, how should we pass the error back to the validator class from the format checker class? It seems the try .. except catching the registered exception should be in the conforms method to me, but validate_format is just expecting a bool from that. Should the FormatChecker.conforms be the one to convert to a ValidationError and then keep a similar try block in validate_format as was in my first PR?

[Julian]

Ech. Doing is True is such an antipattern I think I would get stoned if that appears in jsonschema, even with all the nasty crap jsonschema forces us to do. Have to think about it, unless you come up with something first, otherwise I'll try to give it some thought in an hour or two.

[gazpachoking]

Sounds good. I've got some other work to do, I'll check back after, see if you have a good idea, or maybe I'll come up with something by then.

[gazpachoking]

So, I'm thinking that validate_format, rather than expecting a bool from FormatChecker.conforms either expects nothing, or a ValueError to be raised (or if you prefer, create a FormatError). validate_format then yields a ValidationError based on the error it catches. Two options at this point:

• We keep your api idea for FormatChecker functions, such that a specific checking function registers what error it might throw, and FormatChecker.conforms catches the registered error and creates the ValueError (or FormatError) and raises it.
• FormatChecker.conforms just lets whatever errors pass back through to validate_format, and the specific format checking functions are responsible for raising the appropriate error type when the format checking fails.

Either way, I'd say have FormatChecker.conforms still allow for the simple case of bool returns from the checking function and raise the appropriate error with no extra message when the return is False.
...
And, now that I've typed it, I'm not sure how happy I am with this. Maybe I need to try implementing to see. Still got some other stuff to do, I'll check back later.

[gazpachoking]

I dunno, I'm not fully happy with any of my ideas. I guess the root of the issue is there are 3 possible outcomes.

• The instance matches the format
• The instance doesn't match, no special error message
• The instance doesn't match, error message given

This has to be relayed across 2 levels, from the checker function to FormatChecker and from FormatChecker to validate_format

I guess my favorite solution so far is to catch the error specified when registering the checker in FormatChecker and create a ValidationError. If the checker just returns false with no exception, the generic ValidationError will be created by FormatChecker. Then validate_format just considers the instance conforming if no ValidationError is raised, otherwise yields the error it caught.
ValidationErrors will be created by FormatChecker instead of the actual validator this way, but at least they don't need to be created by the actual format checking function.

[Julian]

How about a method called check that returns None if the instance conforms
to the format, False if it doesn't but no exception was raised, otherwise
if one was it catches it and returns it (we could make conform do that but
I don't think conform is a good name for that anymore. So we could just
change the implementation of conform to just do return self.check(instance,
format) is None since a function that returns a Boolean here still seems
like a thing people might find useful plus who the hell wants to deal with
deprecation if we don't need to).

I think I'm OK with doing:

exception = self.checker.check(...)
if exception is None:
return
elif exception is False:
make validation error with default
else:
make validation error using the returned error

Inside the validator. I just don't really like things outside validators
raising (or constructing) validation errors, especially given all the extra
information they are supposed to have, which makes things really fragile in
case they happen to accidentally leak out without being fully properly
initialized.
On Feb 27, 2013 11:03 PM, "Chase Sterling" [email protected] wrote:

I dunno, I'm not fully happy with any of my ideas. I guess the root of the
issue is there are 3 possible outcomes.

• The instance matches the format
• The instance doesn't match, no special error message
• The instance doesn't match, error message given

This has to be relayed across 2 levels, from the checker function to
FormatChecker and from FormatChecker to validate_format

I guess my favorite solution so far is to catch the error specified when
registering the checker in FormatChecker and create a ValidationError. If
the checker just returns false with no exception, the generic
ValidationError will be created by FormatChecker. Then validate_formatjust considers the instance conforming if no
ValidationError is raised, otherwise yields the error it caught.
ValidationErrors will be created by FormatChecker instead of the actual
validator this way, but at least they don't need to be created by the
actual format checking function.

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

[gazpachoking]

Yeah, I figured you wanted to keep the ValidationError inside the validator, I agree there. I'm not so wild about the False vs None returns meaning the opposite thing though. Guess it's better than the alternatives.

[gazpachoking]

My other thinking was reverse the conforms method such that either True or an error return meant it was invalid, I can't think of any names that don't make a method like that seem awkward though.

[gazpachoking]

How do you feel about leaving the true/false returns like it is and just having a FormatError that could be raised and folded into a ValdationError from validate_format for custom errors?

[Julian]

I think implementation wise it will be simpler to always deal with one type
of return: a value. With that you'd have a return and a possible exception
where one return value and one exception do pretty similar things but in
different code paths.

But both ways kinda suck so if you're gonna compare them I'm OK with seeing
whichever you think looks better.
On Feb 28, 2013 1:46 AM, "Chase Sterling" [email protected] wrote:

How do you feel about leaving the true/false returns like it is and just
having a FormatError that could be raised and folded into a ValdationErrorfrom
validate_format for custom errors?

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

[gazpachoking]

Slight variation: validate_format expects a FormatError, which it turns into a ValidationError, or it considers the check successful. FormatChecker lets those exceptions through, and if it gets a False return, raises a generic FormatError.

[Julian]

FormatChecker lets those exceptions through

You mean catches them and re-raises a FormatError right?

[gazpachoking]

Nope, I meant the checking function raises FormatError directly and FormatChecker just lets it through. But it could also be with your suggestion of registering a custom error to catch that gets turned into a FormatError by FormatChecker

[Julian]

I don't mind the latter.

[gazpachoking]

Added in #76