From 3c8f9f5a6231813133d59bc2ccbc369221c0d94e Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Tue, 30 Aug 2016 16:49:52 +0200 Subject: [PATCH] Catch and display mail sending errors (#215) --- src/pretix/base/services/mail.py | 10 ++++-- src/pretix/control/views/auth.py | 25 ++++++++------ src/pretix/control/views/orders.py | 33 +++++++++++-------- src/pretix/locale/de/LC_MESSAGES/django.po | 5 +-- .../locale/de_Informal/LC_MESSAGES/django.po | 5 +-- src/pretix/plugins/banktransfer/views.py | 6 ++++ src/pretix/plugins/paypal/payment.py | 3 ++ src/pretix/plugins/sendmail/views.py | 15 ++++++--- src/pretix/plugins/stripe/payment.py | 4 +++ src/pretix/presale/checkoutflow.py | 3 ++ 10 files changed, 75 insertions(+), 34 deletions(-) diff --git a/src/pretix/base/services/mail.py b/src/pretix/base/services/mail.py index 75555cfefa..a73441b049 100644 --- a/src/pretix/base/services/mail.py +++ b/src/pretix/base/services/mail.py @@ -19,6 +19,10 @@ class TolerantDict(dict): return key +class SendMailException(Exception): + pass + + def mail(email: str, subject: str, template: str, context: Dict[str, Any]=None, event: Event=None, locale: str=None, order: Order=None, headers: dict=None): @@ -46,8 +50,8 @@ def mail(email: str, subject: str, template: str, :param locale: The locale to be used while evaluating the subject and the template - :raises Exception: on obvious, immediate failures. Not raising an exception does not necessarily mean that the - email has been sent, just that it has been queued by the email backend. + :raises MailOrderException: on obvious, immediate failures. Not raising an exception does not necessarily mean + that the email has been sent, just that it has been queued by the email backend. """ with language(locale): if isinstance(template, LazyI18nString): @@ -94,7 +98,7 @@ def mail_send(to: str, subject: str, body: str, sender: str, event: int=None, he backend.send_messages([email]) except Exception: logger.exception('Error sending email') - raise + raise SendMailException('Failed to send an email to {}.'.format(to)) if settings.HAS_CELERY and settings.EMAIL_BACKEND != 'django.core.mail.outbox': diff --git a/src/pretix/control/views/auth.py b/src/pretix/control/views/auth.py index a1a6ccbb70..ceecc3ef9f 100644 --- a/src/pretix/control/views/auth.py +++ b/src/pretix/control/views/auth.py @@ -14,7 +14,7 @@ from pretix.base.forms.auth import ( LoginForm, PasswordForgotForm, PasswordRecoverForm, RegistrationForm, ) from pretix.base.models import User -from pretix.base.services.mail import mail +from pretix.base.services.mail import SendMailException, mail from pretix.helpers.urls import build_absolute_uri @@ -103,15 +103,20 @@ class Forgot(TemplateView): else: rc.setex('pretix_pwreset_%s' % (user.id), 3600 * 24, '1') - mail( - user.email, _('Password recovery'), 'pretixcontrol/email/forgot.txt', - { - 'user': user, - 'url': (build_absolute_uri('control:auth.forgot.recover') - + '?id=%d&token=%s' % (user.id, default_token_generator.make_token(user))) - }, - None, locale=user.locale - ) + try: + mail( + user.email, _('Password recovery'), 'pretixcontrol/email/forgot.txt', + { + 'user': user, + 'url': (build_absolute_uri('control:auth.forgot.recover') + + '?id=%d&token=%s' % (user.id, default_token_generator.make_token(user))) + }, + None, locale=user.locale + ) + except SendMailException: + messages.error(request, _('There was an error sending the mail. Please try again later.')) + return self.get(request, *args, **kwargs) + user.log_action('pretix.control.auth.user.forgot_password.mail_sent') messages.success(request, _('We sent you an e-mail containing further instructions.')) return redirect('control:auth.forgot') diff --git a/src/pretix/control/views/orders.py b/src/pretix/control/views/orders.py index da7d0e86e1..29fa07bc16 100644 --- a/src/pretix/control/views/orders.py +++ b/src/pretix/control/views/orders.py @@ -21,7 +21,7 @@ from pretix.base.services.invoices import ( generate_cancellation, generate_invoice, invoice_pdf, invoice_qualified, regenerate_invoice, ) -from pretix.base.services.mail import mail +from pretix.base.services.mail import SendMailException, mail from pretix.base.services.orders import cancel_order, mark_order_paid from pretix.base.services.stats import order_overview from pretix.base.signals import ( @@ -201,6 +201,8 @@ class OrderTransition(OrderView): mark_order_paid(self.order, manual=True, user=self.request.user) except Quota.QuotaExceededException as e: messages.error(self.request, str(e)) + except SendMailException: + messages.warning(self.request, _('The order has been marked as paid, but we were unable to send a confirmation mail.')) else: messages.success(self.request, _('The order has been marked as paid.')) elif self.order.status == Order.STATUS_PENDING and to == 'c': @@ -311,18 +313,23 @@ class OrderResendLink(OrderView): def post(self, *args, **kwargs): with language(self.order.locale): - mail( - self.order.email, _('Your order: %(code)s') % {'code': self.order.code}, - self.order.event.settings.mail_text_resend_link, - { - 'event': self.order.event.name, - 'url': build_absolute_uri(self.order.event, 'presale:event.order', kwargs={ - 'order': self.order.code, - 'secret': self.order.secret - }), - }, - self.order.event, locale=self.order.locale - ) + try: + mail( + self.order.email, _('Your order: %(code)s') % {'code': self.order.code}, + self.order.event.settings.mail_text_resend_link, + { + 'event': self.order.event.name, + 'url': build_absolute_uri(self.order.event, 'presale:event.order', kwargs={ + 'order': self.order.code, + 'secret': self.order.secret + }), + }, + self.order.event, locale=self.order.locale + ) + except SendMailException: + messages.error(self.request, _('There was an error sending the mail. Please try again later.')) + return redirect(self.get_order_url()) + messages.success(self.request, _('The email has been queued to be sent.')) self.order.log_action('pretix.event.order.resend', user=self.request.user) return redirect(self.get_order_url()) diff --git a/src/pretix/locale/de/LC_MESSAGES/django.po b/src/pretix/locale/de/LC_MESSAGES/django.po index 9b5011a6f1..adea9a1341 100644 --- a/src/pretix/locale/de/LC_MESSAGES/django.po +++ b/src/pretix/locale/de/LC_MESSAGES/django.po @@ -4276,8 +4276,9 @@ msgid "Send" msgstr "Senden" #: pretix/plugins/sendmail/views.py:43 -msgid "Your message will be sent to the selected users." -msgstr "Die Nachricht wird an die ausgewählten Benutzer verschickt." +msgid "Your message has been queued to be sent to the selected users." +msgstr "" +"Die Nachricht wurde zum Versenden an die ausgewählten Benutzer gespeichert." #: pretix/plugins/statistics/__init__.py:10 #: pretix/plugins/statistics/__init__.py:14 diff --git a/src/pretix/locale/de_Informal/LC_MESSAGES/django.po b/src/pretix/locale/de_Informal/LC_MESSAGES/django.po index 429fbabee4..ba4c40a373 100644 --- a/src/pretix/locale/de_Informal/LC_MESSAGES/django.po +++ b/src/pretix/locale/de_Informal/LC_MESSAGES/django.po @@ -4265,8 +4265,9 @@ msgid "Send" msgstr "Senden" #: pretix/plugins/sendmail/views.py:43 -msgid "Your message will be sent to the selected users." -msgstr "Die Nachricht wird an die ausgewählten Benutzer verschickt." +msgid "Your message has been queued to be sent to the selected users." +msgstr "" +"Die Nachricht wurde zum Versenden an die ausgewählten Benutzer gespeichert." #: pretix/plugins/statistics/__init__.py:10 #: pretix/plugins/statistics/__init__.py:14 diff --git a/src/pretix/plugins/banktransfer/views.py b/src/pretix/plugins/banktransfer/views.py index 1e6b7a922b..ebf60ac2f1 100644 --- a/src/pretix/plugins/banktransfer/views.py +++ b/src/pretix/plugins/banktransfer/views.py @@ -15,6 +15,7 @@ from django.utils.translation import ugettext_lazy as _ from django.views.generic import TemplateView from pretix.base.models import Order, Quota +from pretix.base.services.mail import SendMailException from pretix.base.services.orders import mark_order_paid from pretix.base.settings import SettingsSandbox from pretix.control.permissions import EventPermissionRequiredMixin @@ -54,6 +55,7 @@ class ImportView(EventPermissionRequiredMixin, TemplateView): orders = Order.objects.filter(event=self.request.event, code__in=self.request.POST.getlist('mark_paid')) some_failed = False + mail_failures = False for order in orders: try: mark_order_paid(order, provider='banktransfer', info=json.dumps({ @@ -64,6 +66,8 @@ class ImportView(EventPermissionRequiredMixin, TemplateView): })) except Quota.QuotaExceededException: some_failed = True + except SendMailException: + mail_failures = True if some_failed: messages.warning(self.request, _('Not all of the selected orders could be marked as ' @@ -72,6 +76,8 @@ class ImportView(EventPermissionRequiredMixin, TemplateView): else: messages.success(self.request, _('The selected orders have been marked as paid.')) # TODO: Display a list of them! + if mail_failures: + messages.warning(self.request, _('Some confirmation mails could not be sent.')) return self.redirect_back() messages.error(self.request, _('We were unable to detect the file type of this import. Please ' diff --git a/src/pretix/plugins/paypal/payment.py b/src/pretix/plugins/paypal/payment.py index 80b5240ac3..4c157aac20 100644 --- a/src/pretix/plugins/paypal/payment.py +++ b/src/pretix/plugins/paypal/payment.py @@ -10,6 +10,7 @@ from django.utils.translation import ugettext as __, ugettext_lazy as _ from pretix.base.models import Quota from pretix.base.payment import BasePaymentProvider +from pretix.base.services.mail import SendMailException from pretix.base.services.orders import mark_order_paid, mark_order_refunded from pretix.multidomain.urlreverse import build_absolute_uri @@ -182,6 +183,8 @@ class Paypal(BasePaymentProvider): mark_order_paid(order, 'paypal', json.dumps(payment.to_dict())) except Quota.QuotaExceededException as e: messages.error(request, str(e)) + except SendMailException: + messages.warning(request, _('There was an error sending the confirmation mail.')) return None def order_pending_render(self, request, order) -> str: diff --git a/src/pretix/plugins/sendmail/views.py b/src/pretix/plugins/sendmail/views.py index a076d642af..84590f46a2 100644 --- a/src/pretix/plugins/sendmail/views.py +++ b/src/pretix/plugins/sendmail/views.py @@ -8,7 +8,7 @@ from django.utils.translation import ugettext_lazy as _ from django.views.generic import FormView from pretix.base.models import Order -from pretix.base.services.mail import mail +from pretix.base.services.mail import SendMailException, mail from pretix.control.permissions import EventPermissionRequiredMixin from . import forms @@ -36,11 +36,18 @@ class SenderView(EventPermissionRequiredMixin, FormView): self.request.event.log_action('pretix.plugins.sendmail.sent', user=self.request.user, data=dict( form.cleaned_data)) + failures = [] for o in orders: - mail(o.email, form.cleaned_data['subject'], form.cleaned_data['message'], - None, self.request.event, locale=o.locale, order=o) + try: + mail(o.email, form.cleaned_data['subject'], form.cleaned_data['message'], + None, self.request.event, locale=o.locale, order=o) + except SendMailException: + failures.append(o.email) - messages.success(self.request, _('Your message will be sent to the selected users.')) + if failures: + messages.error(self.request, _('Failed to send mails to the following users: {}'.format(' '.join(failures)))) + else: + messages.success(self.request, _('Your message has been queued to be sent to the selected users.')) return redirect( 'plugins:sendmail:send', diff --git a/src/pretix/plugins/stripe/payment.py b/src/pretix/plugins/stripe/payment.py index 656d51deb6..4b98b59f0c 100644 --- a/src/pretix/plugins/stripe/payment.py +++ b/src/pretix/plugins/stripe/payment.py @@ -10,6 +10,7 @@ from django.utils.translation import ugettext_lazy as _ from pretix.base.models import Quota from pretix.base.payment import BasePaymentProvider +from pretix.base.services.mail import SendMailException from pretix.base.services.orders import mark_order_paid, mark_order_refunded from pretix.multidomain.urlreverse import build_absolute_uri @@ -116,6 +117,9 @@ class Stripe(BasePaymentProvider): mark_order_paid(order, 'stripe', str(charge)) except Quota.QuotaExceededException as e: messages.error(request, str(e)) + except SendMailException: + messages.warning(request, _('There was an error sending the confirmation mail.')) + else: messages.warning(request, _('Stripe reported an error: %s' % charge.failure_message)) logger.info('Charge failed: %s' % str(charge)) diff --git a/src/pretix/presale/checkoutflow.py b/src/pretix/presale/checkoutflow.py index fdd1274cd7..50331953f6 100644 --- a/src/pretix/presale/checkoutflow.py +++ b/src/pretix/presale/checkoutflow.py @@ -12,6 +12,7 @@ from django.views.generic.base import TemplateResponseMixin from pretix.base.models import CartPosition, Order from pretix.base.models.orders import InvoiceAddress +from pretix.base.services.mail import SendMailException from pretix.base.services.orders import OrderError, perform_order from pretix.base.signals import register_payment_providers from pretix.multidomain.urlreverse import eventreverse @@ -351,6 +352,8 @@ class ConfirmStep(CartMixin, AsyncAction, TemplateFlowStep): return exception['exc_message'] elif isinstance(exception, OrderError): return str(exception) + elif isinstance(exception, SendMailException): + return _('There was an error sending the confirmation mail. Please try again later.') return super().get_error_message(exception) def get_error_url(self):