Send mail errors should be handled

rdmo/core/exceptions.py

@@ -1,2 +1,9 @@
 class RDMOException(Exception):
     pass
+
+
+class MailSendError(RDMOException):
+    def __init__(self, reason, original_exception=None):
+        super().__init__(reason)
+        self.reason = reason
+        self.original_exception = original_exception

rdmo/core/mail.py

@@ -1,7 +1,29 @@
+import smtplib
+
 from django.conf import settings
 from django.contrib.sites.models import Site
 from django.core.mail import EmailMessage
 
+from rdmo.core.exceptions import MailSendError
+
+
+def _format_smtp_exception(exception):
+    if isinstance(exception, smtplib.SMTPRecipientsRefused):
+        details = []
+        for _recipient, (_code, response) in exception.recipients.items():
+            if isinstance(response, bytes):
+                response = response.decode(errors='replace')
+            details.append(response)
+        return '; '.join(details) if details else str(exception)
+
+    if isinstance(exception, smtplib.SMTPResponseException):
+        response = exception.smtp_error
+        if isinstance(response, bytes):
+            response = response.decode(errors='replace')
+        return f'{response}' if response else str(exception)
+
+    return str(exception)
+
 
 def send_mail(subject, message, from_email=None, to=None, cc=None, bcc=None, reply_to=None, attachments=None):
     site = Site.objects.get_current()
@@ -11,4 +33,7 @@ def send_mail(subject, message, from_email=None, to=None, cc=None, bcc=None, rep
         from_email = settings.DEFAULT_FROM_EMAIL
 
     mail = EmailMessage(subject, message, from_email, to=to, cc=cc, bcc=bcc, reply_to=reply_to, attachments=attachments)
-    mail.send()
+    try:
+        mail.send()
+    except smtplib.SMTPException as e:
+        raise MailSendError(_format_smtp_exception(e), original_exception=e) from e

rdmo/core/tests/test_mail.py

@@ -1,6 +1,11 @@
+import smtplib
+
+import pytest
+
 from django.conf import settings
 from django.core import mail
 
+from rdmo.core.exceptions import MailSendError
 from rdmo.core.mail import send_mail
 
 
@@ -71,3 +76,23 @@ def test_send_mail_from_attachments(db):
     assert mail.outbox[0].attachments == [
         ('Attachment', b'attachment', 'plain/text')
     ]
+
+
+def test_send_mail_recipient_refused(db, mocker):
+    mocker.patch(
+        'rdmo.core.mail.EmailMessage.send',
+        side_effect=smtplib.SMTPRecipientsRefused({
+            'name@non-existent-domain.abc': (
+                550,
+                b'1.2.3 <name@non-existent-domain.abc>: Recipient address rejected: Domain not found'
+            )
+        })
+    )
+
+    with pytest.raises(MailSendError) as e:
+        send_mail('Subject', 'Message', to=['name@non-existent-domain.abc'])
+
+    assert e.value.reason == (
+        '1.2.3 <name@non-existent-domain.abc>: Recipient address rejected: Domain not found'
+    )
+    assert isinstance(e.value.original_exception, smtplib.SMTPRecipientsRefused)

rdmo/projects/tests/test_view_membership.py

@@ -3,6 +3,7 @@
 from django.core import mail
 from django.urls import reverse
 
+from rdmo.core.exceptions import MailSendError
 from rdmo.projects.models import Invite, Membership
 
 users = (
@@ -164,6 +165,33 @@ def test_membership_create_post_mail_error(db, client, username, password, membe
             assert response.status_code == 302
 
 
+def test_membership_create_post_mail_send_error(db, client, settings, mocker):
+    settings.PROJECT_SEND_INVITE = True
+    client.login(username='owner', password='owner')
+
+    mocker.patch(
+        'rdmo.projects.utils.send_mail',
+        side_effect=MailSendError(
+            '1.2.3 <name@non-existent-domain.abc>: Recipient address rejected: Domain not found'
+        )
+    )
+
+    url = reverse('membership_create', args=[1])
+    response = client.post(url, {
+        'username_or_email': 'someuser@example.com',
+        'role': 'guest'
+    })
+
+    assert response.status_code == 200
+    assert not Invite.objects.filter(project_id=1, user=None,
+                                     email='someuser@example.com').exists()
+    assert len(mail.outbox) == 0
+    assert response.context_data['form'].non_field_errors()[0] == (
+        'Could not send e-mail: 1.2.3 <name@non-existent-domain.abc>: '
+        'Recipient address rejected: Domain not found'
+    )
+
+
 @pytest.mark.parametrize('username,password', users)
 @pytest.mark.parametrize('membership_role', membership_roles)
 def test_membership_create_post_silent(db, client, username, password, membership_role):

rdmo/projects/tests/test_viewset_project_contact.py

@@ -3,6 +3,7 @@
 from django.core import mail
 from django.urls import reverse
 
+from rdmo.core.exceptions import MailSendError
 from rdmo.projects.models import Project, Value
 from rdmo.questions.models import Page, Question
 
@@ -191,3 +192,27 @@ def test_contact_post_error(db, client, username, password, project_id):
             assert response.status_code == 401
 
         assert len(mail.outbox) == 0
+
+
+def test_contact_post_mail_send_error(db, client, mocker):
+    client.login(username='owner', password='owner')
+
+    mocker.patch(
+        'rdmo.projects.utils.send_mail',
+        side_effect=MailSendError(
+            '1.2.3 <name@non-existent-domain.abc>: Recipient address rejected: Domain not found'
+        )
+    )
+
+    url = reverse(urlnames['contact'], args=[1])
+    response = client.post(url, {
+        'subject': 'Test subject',
+        'message': 'Test message'
+    })
+
+    assert response.status_code == 400
+    assert response.json()['non_field_errors'][0] == (
+        'Could not send e-mail: 1.2.3 <name@non-existent-domain.abc>: '
+        'Recipient address rejected: Domain not found'
+    )
+    assert len(mail.outbox) == 0

rdmo/projects/tests/test_viewset_project_invite.py

@@ -3,6 +3,8 @@
 from django.contrib.auth import get_user_model
 from django.urls import reverse
 
+from rdmo.core.exceptions import MailSendError
+
 from ..models import Invite
 
 users = (
@@ -38,6 +40,7 @@
 
 projects = [1, 11]
 invites = [1, 2]
+excluded_invite_ids = [1, 2]
 
 
 @pytest.mark.parametrize('username,password', users)
@@ -121,6 +124,31 @@ def test_create_email(db, client, username, password, project_id):
         assert response.status_code == 404
 
 
+def test_create_email_mail_send_error(db, client, settings, mocker):
+    settings.PROJECT_SEND_INVITE = True
+    client.login(username='owner', password='owner')
+
+    mocker.patch(
+        'rdmo.projects.utils.send_mail',
+        side_effect=MailSendError(
+            '1.2.3 <name@non-existent-domain.abc>: Recipient address rejected: Domain not found'
+        )
+    )
+
+    url = reverse(urlnames['list'], args=[1])
+    response = client.post(url, {
+        'email': 'name@non-existent-domain.abc',
+        'role': 'guest'
+    })
+
+    assert response.status_code == 400
+    assert response.json()['non_field_errors'][0] == (
+        'Could not send e-mail: 1.2.3 <name@non-existent-domain.abc>: '
+        'Recipient address rejected: Domain not found'
+    )
+    assert not Invite.objects.exclude(id__in=excluded_invite_ids).exists()
+
+
 def test_create_error(db, client):
     client.login(username='owner', password='owner')
 

rdmo/projects/views/membership.py

@@ -1,13 +1,16 @@
 import logging
 
 from django.conf import settings
+from django.db import transaction
 from django.http import HttpResponseBadRequest, HttpResponseForbidden, HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
+from django.utils.translation import gettext_lazy as _
 from django.views.generic import DeleteView, UpdateView
 from django.views.generic.edit import FormView
 
 from rdmo.accounts.utils import is_site_manager
+from rdmo.core.exceptions import MailSendError
 from rdmo.core.views import ObjectPermissionMixin, RedirectViewMixin
 
 from ..forms import MembershipCreateForm
@@ -43,9 +46,14 @@ def get_success_url(self):
         return self.project.get_absolute_url()
 
     def form_valid(self, form):
-        invite = form.save()
-        if invite is not None and settings.PROJECT_SEND_INVITE:
-            send_invite_email(self.request, invite)
+        try:
+            with transaction.atomic():
+                invite = form.save()
+                if invite is not None and settings.PROJECT_SEND_INVITE:
+                    send_invite_email(self.request, invite)
+        except MailSendError as e:
+            form.add_error(None, _('Could not send e-mail: %(reason)s') % {'reason': e.reason})
+            return self.form_invalid(form)
         return super().form_valid(form)
 
 

rdmo/projects/viewsets.py

@@ -3,6 +3,7 @@
 from django.conf import settings
 from django.contrib.sites.shortcuts import get_current_site
 from django.core.exceptions import ObjectDoesNotExist
+from django.db import transaction
 from django.db.models import OuterRef, Prefetch, Q, Subquery
 from django.db.models.functions import Coalesce, Greatest
 from django.http import Http404, HttpResponseRedirect
@@ -23,6 +24,7 @@
 from rest_framework_extensions.mixins import NestedViewSetMixin
 
 from rdmo.conditions.models import Condition
+from rdmo.core.exceptions import MailSendError
 from rdmo.core.permissions import HasModelPermission
 from rdmo.core.utils import human2bytes, is_truthy, return_file_response
 from rdmo.options.models import OptionSet
@@ -445,7 +447,12 @@ def contact(self, request, pk):
                 message = request.data.get('message')
 
                 if subject and message:
-                    send_contact_message(request, subject, message)
+                    try:
+                        send_contact_message(request, subject, message)
+                    except MailSendError as e:
+                        raise ValidationError({'non_field_errors': [
+                            _('Could not send e-mail: %(reason)s') % {'reason': e.reason}
+                        ]}) from e
                     return Response(status=status.HTTP_204_NO_CONTENT)
                 else:
                     raise ValidationError({
@@ -562,9 +569,15 @@ def get_serializer_class(self):
             return ProjectInviteSerializer
 
     def perform_create(self, serializer):
-        super().perform_create(serializer)
-        if settings.PROJECT_SEND_INVITE:
-            send_invite_email(self.request, serializer.instance)
+        try:
+            with transaction.atomic():
+                super().perform_create(serializer)
+                if settings.PROJECT_SEND_INVITE:
+                    send_invite_email(self.request, serializer.instance)
+        except MailSendError as e:
+            raise ValidationError({'non_field_errors': [
+                _('Could not send e-mail: %(reason)s') % {'reason': e.reason}
+            ]}) from e
 
 
 class ProjectIssueViewSet(ProjectNestedViewSetMixin, ListModelMixin, RetrieveModelMixin,