Don't retry exceptions that come from bad email formatting (pypi#13214)

Virtuuueness · Mar 16, 2023 · 951ebf6 · 951ebf6
1 parent 16929bb
commit 951ebf6
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 1 deletion.
diff --git a/tests/unit/email/test_init.py b/tests/unit/email/test_init.py
@@ -16,6 +16,7 @@
 import pretend
 import pytest
 
+from pyramid_mailer.exceptions import BadHeaders, EncodingError, InvalidMessage
 from sqlalchemy.exc import NoResultFound
 
 from warehouse import email
@@ -373,7 +374,7 @@ def record_event(self, user_id, tag, additional):
             }
         ]
 
-    def test_send_email_failure(self, monkeypatch):
+    def test_send_email_failure_retry(self, monkeypatch):
         exc = Exception()
 
         class FakeMailSender:
@@ -415,6 +416,47 @@ def retry(exc):
 
         assert task.retry.calls == [pretend.call(exc=exc)]
 
+    @pytest.mark.parametrize("exc", [InvalidMessage, BadHeaders, EncodingError])
+    def test_send_email_failure_doesnt_retry(self, monkeypatch, exc):
+        class FakeMailSender:
+            def send(self, recipient, msg):
+                raise exc
+
+        class Task:
+            @staticmethod
+            @pretend.call_recorder
+            def retry(exc):
+                raise celery.exceptions.Retry
+
+        sender, task = FakeMailSender(), Task()
+        request = pretend.stub(find_service=lambda *a, **kw: sender)
+        user_id = pretend.stub()
+        msg = EmailMessage(subject="subject", body_text="body")
+
+        with pytest.raises(exc):
+            email.send_email(
+                task,
+                request,
+                "recipient",
+                {
+                    "subject": msg.subject,
+                    "body_text": msg.body_text,
+                    "body_html": msg.body_html,
+                },
+                {
+                    "tag": "account:email:sent",
+                    "user_id": user_id,
+                    "additional": {
+                        "from_": "noreply@example.com",
+                        "to": "recipient",
+                        "subject": msg.subject,
+                        "redact_ip": False,
+                    },
+                },
+            )
+
+        assert task.retry.calls == []
+
 
 class TestSendAdminNewOrganizationRequestedEmail:
     def test_send_admin_new_organization_requested_email(

diff --git a/warehouse/email/__init__.py b/warehouse/email/__init__.py
@@ -19,6 +19,7 @@
 
 from celery.schedules import crontab
 from first import first
+from pyramid_mailer.exceptions import BadHeaders, EncodingError, InvalidMessage
 from sqlalchemy.exc import NoResultFound
 
 from warehouse import tasks
@@ -67,6 +68,8 @@ def send_email(task, request, recipient, msg, success_event):
 
         user_service = request.find_service(IUserService, context=None)
         user_service.record_event(**success_event)
+    except (BadHeaders, EncodingError, InvalidMessage) as exc:
+        raise exc
     except Exception as exc:
         task.retry(exc=exc)