Opened 11 years ago
Closed 11 years ago
#20866 closed New feature (invalid)
Password Reset does not catch exception from send_mail
Reported by: | ezra | Owned by: | ezra |
---|---|---|---|
Component: | contrib.auth | Version: | 1.5 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | yes | UI/UX: | no |
Description
PasswordResetForm.save() method calls send_mail to send the password reset token to the user. Per the documentation, send_mail may raise smtplib.SMTPException. PasswordResetForm class does not handle that exception, and neither the views.password_reset method.
It should be straight forward to catch this exception, and report an appropriate error message to the user.
Attachments (1)
Change History (7)
by , 11 years ago
Attachment: | django_20866.patch added |
---|
comment:1 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Attached a potential patch.
comment:2 by , 11 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → New feature |
comment:3 by , 11 years ago
Patch needs improvement: | set |
---|
Thanks for the patch, however, I think it's a bad idea to introduce a dependency of contrib.messages
in contrib.auth
. This would also require a test for the new feature.
comment:4 by , 11 years ago
Thinking of it I'm not sure we should fix this at the form level.
I think that returning a 500 and triggering an admin report (mail_admin, sentry, ...) is the correct way to handle this.
If we choose to display the user a message we must make sure the exception is still triggering an exception report.
comment:5 by , 11 years ago
I don't know Django core code well enough to recommend the correct way to fix this, however I'd love to inform my users if something goes wrong while they were trying to do something.
comment:6 by , 11 years ago
Resolution: | → invalid |
---|---|
Status: | assigned → closed |
@ezra in this case I suggest you read the documentation regarding handling of runtime exceptions. You should be able to inform your users something went wrong this way.
small patch to catch the excpetion