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

Unique validator causes 500 errors when type coercion fails. #139

Open
TrilceAC opened this issue Jan 16, 2019 · 0 comments
Open

Unique validator causes 500 errors when type coercion fails. #139

TrilceAC opened this issue Jan 16, 2019 · 0 comments

Comments

@TrilceAC
Copy link
Contributor

Unique validator checks whether a given value is already in the database:

The way this check is performed might end in internal server errors when some type coercion is required for the query to be performed. A simplified example:

Suppose that I'm doing an app to manage the phone numbers of a company, and therefore, I don't want duplicated phone numbers:

class Phone(Model):
    """Represents a phone number of the company."""

    phone_number = Column(
        PhoneNumberType(region='ES', max_length=15),
        nullable=False,
        unique=True,
        index=True
    )
    # There might be other fields...

This Model requires a valid phone number which should be unique. When the form data is being validated, if the received data is not a valid phone number, like for example the string abcd, it raises an exception that ends on an Internal Server Error. If no uniqueness were required (unique=False), this problem would not happened and the propper message "Not a valid phone number value" would have been shown. Somehow the Unique validator should be prepared and protected against data that could not be coerced. Three differents approaches come to my mind:

The first one, the permissive one, is to catch any exception and ignore it in the hope that other validators will fail, or at a later state the insertion/update will fail:

        try:
            obj = query.first()
        except:
            obj = None

In the above case, if the validator is unable to retrieve an object from the database, it will just continue as if there were none. The problem with this approach is that all exceptions are catched, which is in general a bad practice and can hide other issues like the database not being available at a given point of time.

The second approach would be the most restrictive one:

        try:
            obj = query.first()
        except:
            raise ValidationError(u'Unable to check uniqueness.')

This does not cause internal server errors and prevents the form to be accepted when uniqueness could not be checked for any reason, including cases when there should be real internal server errors, like valid data but connection timeouts. I have used a custom message for showing the exact cause of the problem, but self.message could also have been used instead, if any.

The third approach is to catch each possible exception caused by types that require coercion, like PhoneNumberType or I guess CountryType, ColorType,... the problems with this approach is that it won't work with custom types. And even with know types, catching all possible exception would be tedious.

Which should be the recommended way to go? I bet for the first or the second one, but none of them is perfect.

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

No branches or pull requests

1 participant