ValidationError don't raise if in try block - Custom formfield validators

Question

What was done

I have a form that receives a code that is given to the user so he can verify his email address. To check if the form is valid I created a custom field validator, see forms.py

Problem

It looks like if raise ValidationError("...") would always fail in the try-block. Is this normal behavior or did I something wrong?

forms.py

class SignUpVerificationForm(forms.Form):

    def is_valid_verification_code(code):
        #settings
        time_to_verify_in_minutes = 5

        try:
            tmp = SignUpUser.objects.get(signup_verification=code)
            email = tmp.signup_email
            time = tmp.signup_time
            if time >= timezone.now() - datetime.timedelta(minutes=time_to_verify_in_minutes):
                try:
                    check = User.objects.get(email=email)
                    raise ValidationError("This verification code was already used.")
                except:
                    return code
            else:
                raise ValidationError("This verification code has expired.")
        except:
            raise ValidationError("Invalid verification code.")




    verification_code = forms.CharField(max_length=50,
                                        label='',
                                        validators=[is_valid_verification_code]
                                        )

Edit with ugly solution

I ended up with the code below. I think it's not the right way of doing this but i don't know better at the moment and it's working

def is_valid_verification_code(code):
    #settings
    time_to_register_in_minutes = 5

    try:
        tmp = SignUpUser.objects.get(signup_verification=code)
        email = tmp.signup_email
        time = tmp.signup_time
        if time >= timezone.now() - datetime.timedelta(minutes=time_to_register_in_minutes):
            try:
                user_already_active = User.objects.get(email=email)
                user_already_active = 1
            except:
                return code
        else:
            try:
                user_already_active = User.objects.get(email=email)
                user_already_active = 1
            except:
                user_already_active = 0
    except:
        raise ValidationError("Invalid verification code.")

    if user_already_active:
        raise ValidationError("This verification code was already used.")
    if not user_already_active:
        raise ValidationError("This verification code has expired.")

You always raise an Exception so the try block will always fail. Not sure if you understand ``try/except``. Normally ``User.objects.get(...)`` should raise an Exception if it fails (because the code is already used) and delete the following line. — MSeifert, Feb 24 '16 at 13:36

idjaw · Accepted Answer · 2016-02-24T13:52:59.850

2

You are not using raise the way you should be using it. The way your code is written right now, for every successful call to:

check = User.objects.get(email=email)

You will 100% of the time raise an exception:

raise ValidationError("This verification code was already used.")

You should be putting it in your except block.

try:
    check = User.objects.get(email=email)
except:
    raise ValidationError("This verification code was already used.")

However, based on how you are calling a method called get. You will probably need to add some logic around to see if you in fact need to even raise your "already used" exception. Something like:

if get:
    ValidationError("This verification code was already used.")

How try/except works

edited Feb 24 '16 at 13:52

answered Feb 24 '16 at 13:39

idjaw

25,487
7
64
83

I want to raise an error if a `User` can be gotten. `User != SignUpUser`. A `SignUpUser` has to verify his mail address to become a user. This form gets the verification code and should proof if the code is valid. And so also if a `User` was already activated by the given activation code. – rwx Feb 24 '16 at 13:56
1

@rwx Then what you need to do is similar to what I suggested near the end of my answer with my condition. When you call the `get` method. I assume that if you are able to get something, then that means you should probably raise something. So, you should check to see if the `get` method returns the "thing" you are looking to conclude: "Yes, this user has been activated". Hence, why I suggested a conditional statement. – idjaw Feb 24 '16 at 13:58
It works with `if` as i want it; but now i have to look for some other validation to check if the object exists, because i will get an Internal Error `Exception Value: SignUpUser matching query does not exist` when i pass in a verification code that's not existing. That was the reason why i used the `try`-block Exception Value: SignUpUser matching query does not exist. SO: [what is the right way to validate if an object exists in a django](http://stackoverflow.com/a/640131)/4946454 – rwx Feb 24 '16 at 14:13
1

This is where you need to decide how you want your exception handling to work in your code. Clearly now with an exception that the user does not exist, this means that wherever that exception is being thrown you want to catch it around a try/except either inside that method, or catch the `Exception Value: SignUpUser matching query does not exist.` in the code you posted somewhere and add the necessary logic. – idjaw Feb 24 '16 at 14:20
1

In short. This exception being thrown indicates to you that the user was not activated, so you don't actually want to err. So you need to now think of logic to work around this. – idjaw Feb 24 '16 at 14:22

ValidationError don't raise if in try block - Custom formfield validators

What was done

Problem

forms.py

Edit with ugly solution

1 Answers1