Make emails throw timeout errors, and gracefully handle them

cotisations/models.py

@@ -330,9 +330,14 @@ class Facture(BaseInvoice):
 
     def save(self, *args, **kwargs):
         super(Facture, self).save(*args, **kwargs)
+
+        request = None
+        if "request" in kwargs:
+            request = kwargs["request"]
+
         if not self.__original_valid and self.valid:
             self.reorder_purchases()
-            send_mail_invoice(self)
+            send_mail_invoice(self, request)
         if (
             self.is_subscription()
             and not self.__original_control
@@ -340,7 +345,7 @@ class Facture(BaseInvoice):
             and CotisationsOption.get_cached_value("send_voucher_mail")
             and self.user.is_adherent()
         ):
-            send_mail_voucher(self)
+            send_mail_voucher(self, request)
 
     def __str__(self):
         return str(self.user) + " " + str(self.date)
@@ -870,7 +875,7 @@ class Paiement(RevMixin, AclMixin, models.Model):
 
         # So make this invoice valid, trigger send mail
         invoice.valid = True
-        invoice.save()
+        invoice.save(request)
 
         # In case a cotisation was bought, inform the user, the
         # cotisation time has been extended too

cotisations/utils.py

@@ -23,6 +23,9 @@ import os
 
 from django.template.loader import get_template
 from django.core.mail import EmailMessage
+from django.utils.translation import ugettext_lazy as _
+from django.contrib import messages
+from smtplib import SMTPException
 
 from .tex import create_pdf
 from preferences.models import AssoOption, GeneralOption, CotisationsOption, Mandate
@@ -43,7 +46,21 @@ def find_payment_method(payment):
     return None
 
 
-def send_mail_invoice(invoice):
+def send_mail(mail, request):
+    """Wrapper for Django's EmailMessage.send which handles errors"""
+    try:
+        mail.send()
+    except SMTPException as e:
+        if request:
+            messages.error(
+                request,
+                _("Failed to send email: %(error)s.") % {
+                    "error": e,
+                },
+            )
+
+
+def send_mail_invoice(invoice, request=None):
     """Creates the pdf of the invoice and sends it by email to the client"""
     purchases_info = []
     for purchase in invoice.vente_set.all():
@@ -90,10 +107,11 @@ def send_mail_invoice(invoice):
         [invoice.user.get_mail],
         attachments=[("invoice.pdf", pdf, "application/pdf")],
     )
-    mail.send()
+
+    send_mail(mail, request)
 
 
-def send_mail_voucher(invoice):
+def send_mail_voucher(invoice, request=None):
     """Creates a voucher from an invoice and sends it by email to the client"""
     president = Mandate.get_mandate(invoice.date).president
     ctx = {
@@ -126,4 +144,5 @@ def send_mail_voucher(invoice):
         [invoice.user.get_mail],
         attachments=[("voucher.pdf", pdf, "application/pdf")],
     )
-    mail.send()
+
+    send_mail(mail, request)

cotisations/views.py

@@ -1008,7 +1008,7 @@ def credit_solde(request, user, **_kwargs):
         else:
             price_ok = True
         if price_ok:
-            invoice.save()
+            invoice.save(request)
             Vente.objects.create(
                 facture=invoice,
                 name="solde",

re2o/settings.py

@@ -181,6 +181,9 @@ MEDIA_URL = os.path.join(BASE_DIR, "/media/")
 # Models to use for graphs
 GRAPH_MODELS = {"all_applications": True, "group_models": True}
 
+# Timeout when sending emails through Django (in seconds)
+EMAIL_TIMEOUT = 10
+
 # Activate API
 if "api" in INSTALLED_APPS:
     from api.settings import *

re2o/utils.py

@@ -39,6 +39,10 @@ from __future__ import unicode_literals
 from django.utils import timezone
 from django.db.models import Q
 from django.contrib.auth.models import Permission
+from django.utils.translation import ugettext_lazy as _
+from django.core.mail import send_mail as django_send_mail
+from django.contrib import messages
+from smtplib import SMTPException
 
 from cotisations.models import Cotisation, Facture, Vente
 from machines.models import Interface, Machine
@@ -213,3 +217,17 @@ def remove_user_room(room, force=True):
     if force or not user.has_access():
         user.room = None
         user.save()
+
+
+def send_mail(request, *args, **kwargs):
+    """Wrapper for Django's send_mail which handles errors"""
+    try:
+        kwargs["fail_silently"] = request is None
+        django_send_mail(*args, **kwargs)
+    except SMTPException as e:
+        messages.error(
+            request,
+            _("Failed to send email: %(error)s.") % {
+                "error": e,
+            },
+        )

tickets/models.py

@@ -1,11 +1,11 @@
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
-from django.core.mail import send_mail
 from django.template import loader
 from django.db.models.signals import post_save
 from django.dispatch import receiver
 
 from re2o.mixins import AclMixin
+from re2o.utils import send_mail
 
 from preferences.models import GeneralOption
 
@@ -38,6 +38,7 @@ class Ticket(AclMixin, models.Model):
         help_text=_("An email address to get back to you."), max_length=100, null=True
     )
     solved = models.BooleanField(default=False)
+    request = None
 
     class Meta:
         permissions = (("view_tickets", _("Can view a ticket object")),)
@@ -50,7 +51,7 @@ class Ticket(AclMixin, models.Model):
         else:
             return _("Anonymous ticket. Date: %s.") % (self.date)
 
-    def publish_mail(self):
+    def publish_mail(self, request=None):
         site_url = GeneralOption.objects.first().main_site_url
         to_addr = Preferences.objects.first().publish_address
         context = {"ticket": self, "site_url": site_url}
@@ -62,7 +63,9 @@ class Ticket(AclMixin, models.Model):
         else:
             obj = "New ticket opened"
             template = loader.get_template("tickets/publication_mail_en")
+
         send_mail(
+            request,
             obj,
             template.render(context),
             GeneralOption.get_cached_value("email_from"),
@@ -108,4 +111,4 @@ def ticket_post_save(**kwargs):
     if kwargs["created"]:
         if Preferences.objects.first().publish_address:
             ticket = kwargs["instance"]
-            ticket.publish_mail()
+            ticket.publish_mail(ticket.request)

tickets/views.py

@@ -60,6 +60,8 @@ def new_ticket(request):
         if ticketform.is_valid():
             email = ticketform.cleaned_data.get("email")
             ticket = ticketform.save(commit=False)
+            ticket.request = request
+
             if request.user.is_authenticated:
                 ticket.user = request.user
                 ticket.save()

users/models.py

@@ -60,7 +60,6 @@ from django.db.models.signals import post_save, post_delete, m2m_changed
 from django.dispatch import receiver
 from django.utils.functional import cached_property
 from django.template import loader
-from django.core.mail import send_mail
 from django.core.urlresolvers import reverse
 from django.db import transaction
 from django.utils import timezone
@@ -84,6 +83,7 @@ from re2o.settings import LDAP, GID_RANGES, UID_RANGES
 from re2o.field_permissions import FieldPermissionModelMixin
 from re2o.mixins import AclMixin, RevMixin
 from re2o.base import smtp_check
+from re2o.utils import send_mail
 
 from cotisations.models import Cotisation, Facture, Paiement, Vente
 from machines.models import Domain, Interface, Machine, regen
@@ -244,6 +244,7 @@ class User(
     REQUIRED_FIELDS = ["surname", "email"]
 
     objects = UserManager()
+    request = None
 
     class Meta:
         permissions = (
@@ -749,7 +750,7 @@ class User(
             name__in=list(queryset_users.values_list("pseudo", flat=True))
         )
 
-    def notif_inscription(self):
+    def notif_inscription(self, request=None):
         """ Prend en argument un objet user, envoie un mail de bienvenue """
         template = loader.get_template("users/email_welcome")
         mailmessageoptions, _created = MailMessageOption.objects.get_or_create()
@@ -761,7 +762,9 @@ class User(
             "welcome_mail_en": mailmessageoptions.welcome_mail_en,
             "pseudo": self.pseudo,
         }
+
         send_mail(
+            request,
             "Bienvenue au %(name)s / Welcome to %(name)s"
             % {"name": AssoOption.get_cached_value("name")},
             "",
@@ -769,7 +772,6 @@ class User(
             [self.email],
             html_message=template.render(context),
         )
-        return
 
     def reset_passwd_mail(self, request):
         """ Prend en argument un request, envoie un mail de
@@ -789,7 +791,9 @@ class User(
             ),
             "expire_in": str(GeneralOption.get_cached_value("req_expire_hrs")),
         }
+
         send_mail(
+            request,
             "Changement de mot de passe de %(name)s / Password change for "
             "%(name)s" % {"name": AssoOption.get_cached_value("name")},
             template.render(context),
@@ -797,7 +801,6 @@ class User(
             [req.user.email],
             fail_silently=False,
         )
-        return
 
     def send_confirm_email_if_necessary(self, request):
         """Update the user's email state:
@@ -873,7 +876,9 @@ class User(
             "confirm_before_fr": self.confirm_email_before_date().strftime("%d/%m/%Y"),
             "confirm_before_en": self.confirm_email_before_date().strftime("%Y-%m-%d"),
         }
+
         send_mail(
+            request,
             "Confirmation du mail de %(name)s / Email confirmation for "
             "%(name)s" % {"name": AssoOption.get_cached_value("name")},
             template.render(context),
@@ -883,7 +888,7 @@ class User(
         )
         return
 
-    def autoregister_machine(self, mac_address, nas_type):
+    def autoregister_machine(self, mac_address, nas_type, request=None):
         """ Fonction appellée par freeradius. Enregistre la mac pour
         une machine inconnue sur le compte de l'user"""
         allowed, _message, _rights = Machine.can_create(self, self.id)
@@ -927,7 +932,9 @@ class User(
             "asso_email": AssoOption.get_cached_value("contact"),
             "pseudo": self.pseudo,
         }
+
         send_mail(
+            None,
             "Ajout automatique d'une machine / New machine autoregistered",
             "",
             GeneralOption.get_cached_value("email_from"),
@@ -936,7 +943,7 @@ class User(
         )
         return
 
-    def notif_disable(self):
+    def notif_disable(self, request=None):
         """Envoi un mail de notification informant que l'adresse mail n'a pas été confirmée"""
         template = loader.get_template("users/email_disable_notif")
         context = {
@@ -945,7 +952,9 @@ class User(
             "asso_email": AssoOption.get_cached_value("contact"),
             "site_name": GeneralOption.get_cached_value("site_name"),
         }
+
         send_mail(
+            request,
             "Suspension automatique / Automatic suspension",
             template.render(context),
             GeneralOption.get_cached_value("email_from"),
@@ -1509,8 +1518,10 @@ def user_post_save(**kwargs):
     is_created = kwargs["created"]
     user = kwargs["instance"]
     EMailAddress.objects.get_or_create(local_part=user.pseudo.lower(), user=user)
+
     if is_created:
-        user.notif_inscription()
+        user.notif_inscription(user.request)
+
     user.state_sync()
     user.ldap_sync(
         base=True, access_refresh=True, mac_refresh=False, group_refresh=True
@@ -1737,13 +1748,14 @@ class Ban(RevMixin, AclMixin, models.Model):
     date_start = models.DateTimeField(auto_now_add=True)
     date_end = models.DateTimeField()
     state = models.IntegerField(choices=STATES, default=STATE_HARD)
+    request = None
 
     class Meta:
         permissions = (("view_ban", _("Can view a ban object")),)
         verbose_name = _("ban")
         verbose_name_plural = _("bans")
 
-    def notif_ban(self):
+    def notif_ban(self, request=None):
         """ Prend en argument un objet ban, envoie un mail de notification """
         template = loader.get_template("users/email_ban_notif")
         context = {
@@ -1752,7 +1764,9 @@ class Ban(RevMixin, AclMixin, models.Model):
             "date_end": self.date_end,
             "asso_name": AssoOption.get_cached_value("name"),
         }
+
         send_mail(
+            request,
             "Déconnexion disciplinaire / Disciplinary disconnection",
             template.render(context),
             GeneralOption.get_cached_value("email_from"),
@@ -1795,7 +1809,7 @@ def ban_post_save(**kwargs):
     user.ldap_sync(base=False, access_refresh=True, mac_refresh=False)
     regen("mailing")
     if is_created:
-        ban.notif_ban()
+        ban.notif_ban(ban.request)
         regen("dhcp")
         regen("mac_ip_list")
     if user.has_access():

users/views.py

@@ -119,6 +119,8 @@ def new_user(request):
     """ Vue de création d'un nouvel utilisateur,
     envoie un mail pour le mot de passe"""
     user = AdherentCreationForm(request.POST or None, user=request.user)
+    user.request = request
+
     GTU_sum_up = GeneralOption.get_cached_value("GTU_sum_up")
     GTU = GeneralOption.get_cached_value("GTU")
     is_set_password_allowed = OptionalUser.get_cached_value("allow_set_password_during_user_creation")
@@ -165,6 +167,8 @@ def new_club(request):
     """ Vue de création d'un nouveau club,
     envoie un mail pour le mot de passe"""
     club = ClubForm(request.POST or None, user=request.user)
+    club.request = request
+
     if club.is_valid():
         club = club.save(commit=False)
         club.save()
@@ -368,6 +372,8 @@ def add_ban(request, user, userid):
     Syntaxe : JJ/MM/AAAA , heure optionnelle, prend effet immédiatement"""
     ban_instance = Ban(user=user)
     ban = BanForm(request.POST or None, instance=ban_instance)
+    ban.request = request
+
     if ban.is_valid():
         ban.save()
         messages.success(request, _("The ban was added."))
@@ -386,6 +392,8 @@ def edit_ban(request, ban_instance, **_kwargs):
     (a fortiori bureau)
     Syntaxe : JJ/MM/AAAA , heure optionnelle, prend effet immédiatement"""
     ban = BanForm(request.POST or None, instance=ban_instance)
+    ban.request = request
+
     if ban.is_valid():
         if ban.changed_data:
             ban.save()