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

Validators correctly raise exceptions #551

Merged
merged 3 commits into from
Apr 21, 2020
Merged

Conversation

azmeuk
Copy link
Member

@azmeuk azmeuk commented Apr 17, 2020

This PR should fix #445
Because ValidationError inherits from ValueError, A ValueError raised inside a validator is just considered as the validator refusing the data. This behavior felt unexpected to me, and is fixed in this PR.

Here is the piece of code from #445 illustrating the bug:

>>> from wtforms import StringField, Form
>>> from werkzeug.datastructures import MultiDict
...
>>> def division_error_validator(form, field): raise ZeroDivisionError
>>> class DivisionErrorForm(Form):
...     foo = StringField(validators=[division_error_validator])
...
>>> def value_error_validator(form, field): raise ValueError
>>> class ValueErrorForm(Form):
...    foo = StringField(validators=[value_error_validator])
...
>>> DivisionErrorForm(MultiDict({"bar": "baz"})).validate()
Traceback (most recent call last):
...
ZeroDivisionError

>>> ValueErrorForm(MultiDict({"bar": "baz"})).validate()
Traceback (most recent call last):
  File "/home/azmeuk/dev/wtforms/src/wtforms/fields/core.py", line 275, in _run_validation_chain
    validator(form, self)
  File "<stdin>", line 1, in value_error_validator
ValueError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/azmeuk/dev/wtforms/src/wtforms/form.py", line 300, in validate
    return super(Form, self).validate(extra)
  File "/home/azmeuk/dev/wtforms/src/wtforms/form.py", line 136, in validate
    if not field.validate(self, extra):
  File "/home/azmeuk/dev/wtforms/src/wtforms/fields/core.py", line 255, in validate
    stop_validation = self._run_validation_chain(form, chain)
  File "/home/azmeuk/dev/wtforms/src/wtforms/fields/core.py", line 281, in _run_validation_chain
    self.errors.append(e.args[0])
IndexError: tuple index out of range

@davidism
Copy link
Member

#61 is related

@davidism
Copy link
Member

I'm not 100% sure on this one, I feel like there was a reason ValueError was accepted in certain places. Are there previous discussions about this?

@azmeuk
Copy link
Member Author

azmeuk commented Apr 21, 2020

A comment on the thread you mentioned, says that during the processing phase, ValidationError are unrelevant, so only ValueError are handled.

But I am not sure this is directly related to this patch, because this line it changes to except ValidationError, is in _run_validation_chain, so basically in the validation phase.
https://github.com/wtforms/wtforms/blob/129040ab4d5af83a05ddc7b63f854d48be3d55aa/src/wtforms/fields/core.py#L280

My feeling is that we should trigger special wtforms actions (such as making the validation fail (i.e. return False)) on wtforms exceptions, and let the other exception types reach the user so they can be debugged.
Currently, there is no way to differentiate a ValueError that is raised to make the validation fail and a classical ValueError.

edit: I edited the changelog, and made the behavior consistent with pre_validate and post_validate.

@azmeuk azmeuk merged commit a3f87bc into pallets-eco:master Apr 21, 2020
@azmeuk
Copy link
Member Author

azmeuk commented Apr 21, 2020

Thank you. I think I won't backport this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Validators do not raise IndexError when it catches a ValueError
2 participants