From 5e9ebb44a2715f105f87da5ca0a60e5a18ec94b9 Mon Sep 17 00:00:00 2001 From: Claudia Malzer Date: Wed, 8 Jul 2026 14:25:15 +0200 Subject: [PATCH 1/4] Catch SMTP mail errors and surface validation messages --- rdmo/core/mail.py | 32 ++++++++++++++++++++++++++++++- rdmo/projects/views/membership.py | 14 +++++++++++--- rdmo/projects/viewsets.py | 21 ++++++++++++++++---- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/rdmo/core/mail.py b/rdmo/core/mail.py index 641935923a..b0e83b33bd 100644 --- a/rdmo/core/mail.py +++ b/rdmo/core/mail.py @@ -1,8 +1,35 @@ +import smtplib + from django.conf import settings from django.contrib.sites.models import Site from django.core.mail import EmailMessage +class MailSendError(Exception): + def __init__(self, reason, original_exception=None): + super().__init__(reason) + self.reason = reason + self.original_exception = original_exception + + +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() subject = f'[{site.name}] ' + subject @@ -11,4 +38,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 diff --git a/rdmo/projects/views/membership.py b/rdmo/projects/views/membership.py index 54242b57e9..7e6c2fd73a 100644 --- a/rdmo/projects/views/membership.py +++ b/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.mail 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) diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index 84782db09f..f592d4e806 100644 --- a/rdmo/projects/viewsets.py +++ b/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.mail 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, From 8da0cf47d41624aed4b62953d0e320589ba0b9f2 Mon Sep 17 00:00:00 2001 From: Claudia Malzer Date: Wed, 8 Jul 2026 16:02:19 +0200 Subject: [PATCH 2/4] Make MailSendError a custom exception --- rdmo/core/exceptions.py | 7 +++++++ rdmo/core/mail.py | 7 +------ rdmo/projects/views/membership.py | 2 +- rdmo/projects/viewsets.py | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/rdmo/core/exceptions.py b/rdmo/core/exceptions.py index cde361b78b..84fedabe76 100644 --- a/rdmo/core/exceptions.py +++ b/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 diff --git a/rdmo/core/mail.py b/rdmo/core/mail.py index b0e83b33bd..07cfa12b40 100644 --- a/rdmo/core/mail.py +++ b/rdmo/core/mail.py @@ -4,12 +4,7 @@ from django.contrib.sites.models import Site from django.core.mail import EmailMessage - -class MailSendError(Exception): - def __init__(self, reason, original_exception=None): - super().__init__(reason) - self.reason = reason - self.original_exception = original_exception +from rdmo.core.exceptions import MailSendError def _format_smtp_exception(exception): diff --git a/rdmo/projects/views/membership.py b/rdmo/projects/views/membership.py index 7e6c2fd73a..3d0d0b254a 100644 --- a/rdmo/projects/views/membership.py +++ b/rdmo/projects/views/membership.py @@ -10,7 +10,7 @@ from django.views.generic.edit import FormView from rdmo.accounts.utils import is_site_manager -from rdmo.core.mail import MailSendError +from rdmo.core.exceptions import MailSendError from rdmo.core.views import ObjectPermissionMixin, RedirectViewMixin from ..forms import MembershipCreateForm diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index f592d4e806..60651b56b8 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -24,7 +24,7 @@ from rest_framework_extensions.mixins import NestedViewSetMixin from rdmo.conditions.models import Condition -from rdmo.core.mail import MailSendError +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 From aaa04c94fe0cfa466e50db09cd7cbf90ba4036b7 Mon Sep 17 00:00:00 2001 From: Claudia Malzer Date: Wed, 8 Jul 2026 20:08:09 +0200 Subject: [PATCH 3/4] Add tests --- rdmo/core/tests/test_mail.py | 25 +++++++++++++++++ rdmo/projects/tests/test_view_membership.py | 28 +++++++++++++++++++ .../tests/test_viewset_project_contact.py | 25 +++++++++++++++++ .../tests/test_viewset_project_invite.py | 28 +++++++++++++++++++ 4 files changed, 106 insertions(+) diff --git a/rdmo/core/tests/test_mail.py b/rdmo/core/tests/test_mail.py index 939a4d12aa..ad92b6c535 100644 --- a/rdmo/core/tests/test_mail.py +++ b/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 : 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 : Recipient address rejected: Domain not found' + ) + assert isinstance(e.value.original_exception, smtplib.SMTPRecipientsRefused) diff --git a/rdmo/projects/tests/test_view_membership.py b/rdmo/projects/tests/test_view_membership.py index 4eef8c5c98..46d6a40b09 100644 --- a/rdmo/projects/tests/test_view_membership.py +++ b/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 : 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 : ' + '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): diff --git a/rdmo/projects/tests/test_viewset_project_contact.py b/rdmo/projects/tests/test_viewset_project_contact.py index 06851c21cf..e82985d8cd 100644 --- a/rdmo/projects/tests/test_viewset_project_contact.py +++ b/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 : 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 : ' + 'Recipient address rejected: Domain not found' + ) + assert len(mail.outbox) == 0 diff --git a/rdmo/projects/tests/test_viewset_project_invite.py b/rdmo/projects/tests/test_viewset_project_invite.py index a99fc9fd97..26b3cf020d 100644 --- a/rdmo/projects/tests/test_viewset_project_invite.py +++ b/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 : 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 : ' + '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') From a71bd1e5f181b9f0be87f756d3d7fbf0e86b301f Mon Sep 17 00:00:00 2001 From: Claudia Malzer Date: Thu, 9 Jul 2026 13:57:32 +0200 Subject: [PATCH 4/4] Make mail error handling more general --- rdmo/core/exceptions.py | 3 +-- rdmo/core/mail.py | 20 +------------------ rdmo/core/tests/test_mail.py | 19 +++++++++--------- rdmo/projects/tests/test_view_membership.py | 12 ++++++----- .../tests/test_viewset_project_contact.py | 14 ++++++------- .../tests/test_viewset_project_invite.py | 14 ++++++------- 6 files changed, 32 insertions(+), 50 deletions(-) diff --git a/rdmo/core/exceptions.py b/rdmo/core/exceptions.py index 84fedabe76..b53138a361 100644 --- a/rdmo/core/exceptions.py +++ b/rdmo/core/exceptions.py @@ -2,8 +2,7 @@ class RDMOException(Exception): pass -class MailSendError(RDMOException): +class MailSendError(Exception): def __init__(self, reason, original_exception=None): super().__init__(reason) self.reason = reason - self.original_exception = original_exception diff --git a/rdmo/core/mail.py b/rdmo/core/mail.py index 07cfa12b40..5cf553b371 100644 --- a/rdmo/core/mail.py +++ b/rdmo/core/mail.py @@ -7,24 +7,6 @@ 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() subject = f'[{site.name}] ' + subject @@ -36,4 +18,4 @@ def send_mail(subject, message, from_email=None, to=None, cc=None, bcc=None, rep try: mail.send() except smtplib.SMTPException as e: - raise MailSendError(_format_smtp_exception(e), original_exception=e) from e + raise MailSendError(str(e)) from e diff --git a/rdmo/core/tests/test_mail.py b/rdmo/core/tests/test_mail.py index ad92b6c535..dc6efd895b 100644 --- a/rdmo/core/tests/test_mail.py +++ b/rdmo/core/tests/test_mail.py @@ -79,20 +79,19 @@ def test_send_mail_from_attachments(db): def test_send_mail_recipient_refused(db, mocker): + smtp_exception = smtplib.SMTPRecipientsRefused({ + 'name@non-existent-domain.abc': ( + 550, + b'1.2.3 : Recipient address rejected: Domain not found' + ) + }) + mocker.patch( 'rdmo.core.mail.EmailMessage.send', - side_effect=smtplib.SMTPRecipientsRefused({ - 'name@non-existent-domain.abc': ( - 550, - b'1.2.3 : Recipient address rejected: Domain not found' - ) - }) + side_effect=smtp_exception ) with pytest.raises(MailSendError) as e: send_mail('Subject', 'Message', to=['name@non-existent-domain.abc']) - assert e.value.reason == ( - '1.2.3 : Recipient address rejected: Domain not found' - ) - assert isinstance(e.value.original_exception, smtplib.SMTPRecipientsRefused) + assert e.value.reason == str(smtp_exception) diff --git a/rdmo/projects/tests/test_view_membership.py b/rdmo/projects/tests/test_view_membership.py index 46d6a40b09..8c0937ce90 100644 --- a/rdmo/projects/tests/test_view_membership.py +++ b/rdmo/projects/tests/test_view_membership.py @@ -169,11 +169,14 @@ def test_membership_create_post_mail_send_error(db, client, settings, mocker): settings.PROJECT_SEND_INVITE = True client.login(username='owner', password='owner') + reason = ( + "{'name@non-existent-domain.abc': (550, " + "b'1.2.3 : Recipient address rejected: " + "Domain not found')}" + ) mocker.patch( 'rdmo.projects.utils.send_mail', - side_effect=MailSendError( - '1.2.3 : Recipient address rejected: Domain not found' - ) + side_effect=MailSendError(reason) ) url = reverse('membership_create', args=[1]) @@ -187,8 +190,7 @@ def test_membership_create_post_mail_send_error(db, client, settings, mocker): 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 : ' - 'Recipient address rejected: Domain not found' + f'Could not send e-mail: {reason}' ) diff --git a/rdmo/projects/tests/test_viewset_project_contact.py b/rdmo/projects/tests/test_viewset_project_contact.py index e82985d8cd..850c0009b9 100644 --- a/rdmo/projects/tests/test_viewset_project_contact.py +++ b/rdmo/projects/tests/test_viewset_project_contact.py @@ -197,11 +197,14 @@ def test_contact_post_error(db, client, username, password, project_id): def test_contact_post_mail_send_error(db, client, mocker): client.login(username='owner', password='owner') + reason = ( + "{'name@non-existent-domain.abc': (550, " + "b'1.2.3 : Recipient address rejected: " + "Domain not found')}" + ) mocker.patch( 'rdmo.projects.utils.send_mail', - side_effect=MailSendError( - '1.2.3 : Recipient address rejected: Domain not found' - ) + side_effect=MailSendError(reason) ) url = reverse(urlnames['contact'], args=[1]) @@ -211,8 +214,5 @@ def test_contact_post_mail_send_error(db, client, mocker): }) assert response.status_code == 400 - assert response.json()['non_field_errors'][0] == ( - 'Could not send e-mail: 1.2.3 : ' - 'Recipient address rejected: Domain not found' - ) + assert response.json()['non_field_errors'][0] == f'Could not send e-mail: {reason}' assert len(mail.outbox) == 0 diff --git a/rdmo/projects/tests/test_viewset_project_invite.py b/rdmo/projects/tests/test_viewset_project_invite.py index 26b3cf020d..ea09b41076 100644 --- a/rdmo/projects/tests/test_viewset_project_invite.py +++ b/rdmo/projects/tests/test_viewset_project_invite.py @@ -128,11 +128,14 @@ def test_create_email_mail_send_error(db, client, settings, mocker): settings.PROJECT_SEND_INVITE = True client.login(username='owner', password='owner') + reason = ( + "{'name@non-existent-domain.abc': (550, " + "b'1.2.3 : Recipient address rejected: " + "Domain not found')}" + ) mocker.patch( 'rdmo.projects.utils.send_mail', - side_effect=MailSendError( - '1.2.3 : Recipient address rejected: Domain not found' - ) + side_effect=MailSendError(reason) ) url = reverse(urlnames['list'], args=[1]) @@ -142,10 +145,7 @@ def test_create_email_mail_send_error(db, client, settings, mocker): }) assert response.status_code == 400 - assert response.json()['non_field_errors'][0] == ( - 'Could not send e-mail: 1.2.3 : ' - 'Recipient address rejected: Domain not found' - ) + assert response.json()['non_field_errors'][0] == f'Could not send e-mail: {reason}' assert not Invite.objects.exclude(id__in=excluded_invite_ids).exists()