Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StopValidation not caught during process_formdata #61

Open
davidism opened this issue Mar 6, 2014 · 4 comments
Open

StopValidation not caught during process_formdata #61

davidism opened this issue Mar 6, 2014 · 4 comments
Labels
enhancement New feature, or existing feature improvement
Milestone

Comments

@davidism
Copy link
Member

davidism commented Mar 6, 2014

If ValidationError is raised during process_formdata, it is correctly caught and added to .errors. StopValidation is not caught; therefore there is no way to say "I couldn't process the form data, don't run any validators." Right now I need to have a custom validator check if field.data is None: raise StopValidation('couldn't process data').

This is somewhat demonstrated in your own code for dateutil DateTimeField. The field raises a ValidationError if it couldn't parse the data during processing. A form using this field with validators will need to verify that the data is not None before doing anything else that assumes a datetime. This could be handled better if the field raised StopValidation and wtforms handled it correctly.

@crast
Copy link
Contributor

crast commented Mar 24, 2014

The reason that StopValidation isn't honored in process_formdata is that at this point, you're not actually validating, you're in Form.process() which calls Field.process().

The validation always starts when you call Form.validate(), regardless of whether there were processing errors (and those are stored in field.process_errors).

While I could see a reason for potentially setting a flag which stops validation from happening due to process errors, I can also see how it would limit the capabilities of validators to actually influence or be influenced by processing errors.

@crast crast added the waiting label Mar 24, 2014
@davidism
Copy link
Member Author

at this point, you're not actually validating

Why is ValidationError handled at this point then? Why have a way to essentially pre-validate without being able to "pre-stop".

I understand from your explanation that a validator could check if field.process_errors is empty or data is not None before continuing. This should probably be added to the documentation on writing custom or in-line validators.

It's not even mentioned that the built-in validators do this check (data is not None). Looking at the source, some of them seem to think None means don't validate, while others assume a default value. And they all check and default in a different way. Not consistent.

I can't think of a situation where you would want to validate data that didn't even process correctly, so this just seems like unnecessary boilerplate, especially in-line validators that should be quick and easy to write.

Ultimately, it's not that big a deal, as I've managed to work around it and understand why it works the way it does. I just feel like this is all a little unclear and could be handled better.

@crast
Copy link
Contributor

crast commented May 20, 2014

I concur.

In an attempt to add the features we wanted to ad without creating major API breakage, we haven't actually changed the paradigm yet in WTForms 2.0. WTForms 2.0 (released today) is a transition version, allowing a peek of some of the new design ideas while still having the Field contract be mostly unchanged. We're going to go stomping around now in our fields and breaking stuff for the next major release, but this allowed us to significantly reduce our code liability (we deprecated a lot of things, so that the next release will clearly be a breaking release as far as this is concerned.)

@crast
Copy link
Contributor

crast commented Jul 6, 2014

Why is ValidationError handled at this point then? Why have a way to essentially pre-validate without being able to "pre-stop".

ValidationError specifically isn't handled, it's ValueError which is handled, of which ValidationError is a subclass. This is to catch any process-time errors that would have occurred during coercion, and they are kept on .process_errors of the field. By default, the validation adds the process errors to the errors.

@crast crast added enhancement New feature, or existing feature improvement and removed waiting labels Feb 19, 2016
@crast crast added this to the 3.0 milestone Feb 19, 2016
@azmeuk azmeuk mentioned this issue Oct 6, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, or existing feature improvement
Development

No branches or pull requests

2 participants