Skip to content

Commit

Permalink
Anymail: retry sending failed emails
Browse files Browse the repository at this point in the history
Before this commit, emails that couldn’t be sent were simply discarded,
because the code didn’t check the status from anymail.

Anymail exposes a status for each recipient of an email, raising many
questions on the expected behavior should one fail to be delivered out
of multiple recipients.

As a first step, simply trust Mailjet API Status field.

https://anymail.dev/en/stable/sending/django_email/#refused-recipients
https://anymail.dev/en/stable/sending/anymail_additions/#anymail.message.AnymailStatus
  • Loading branch information
francoisfreitag committed May 22, 2024
1 parent 0610f47 commit eee51cd
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 3 deletions.
15 changes: 13 additions & 2 deletions itou/utils/tasks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
from functools import partial

import sentry_sdk
from django.conf import settings
from django.core.mail import get_connection
from django.core.mail.backends.base import BaseEmailBackend
Expand Down Expand Up @@ -111,8 +112,8 @@ def _async_send_messages(serializable_email_messages):
return len(messages)


@task(retries=_NB_RETRIES, retry_delay=settings.SEND_EMAIL_DELAY_BETWEEN_RETRIES_IN_SECONDS)
def _async_send_message(serialized_email):
@task(retries=_NB_RETRIES, retry_delay=settings.SEND_EMAIL_DELAY_BETWEEN_RETRIES_IN_SECONDS, context=True)
def _async_send_message(serialized_email, task=None):
"""
An `EmailMessage` instance holds references to some non-serializable
ressources, such as a connection to the email backend (if not `None`).
Expand All @@ -123,6 +124,16 @@ def _async_send_message(serialized_email):
with get_connection(backend=settings.ASYNC_EMAIL_BACKEND) as connection:
email = _deserializeEmailMessage(serialized_email)
connection.send_messages([email])
# Mailjet has a global flag for an email to indicate success.
# Anymail iterates over recipients and assign each a status, following its
# documented API. Mailjet indicating success is good enough.
# https://dev.mailjet.com/email/guides/send-api-v31/
esp_response = email.anymail_status.esp_response.json()
[result] = esp_response["Messages"]
if result["Status"] != "success":
if task and task.retries:
raise Exception("Huey, please retry this task.")
sentry_sdk.capture_message(f"Could not send email: {result}", "error")
return 1


Expand Down
5 changes: 5 additions & 0 deletions requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,11 @@ requests==2.32.0 \
# via
# -r requirements/test.txt
# django-anymail
# requests-mock
requests-mock==1.12.1 \
--hash=sha256:b1e37054004cdd5e56c84454cc7df12b25f90f382159087f4b6915aaeef39563 \
--hash=sha256:e9e12e333b525156e82a3c852f22016b9158220d2f47454de9cae8a77d371401
# via -r requirements/test.txt
respx==0.21.1 \
--hash=sha256:05f45de23f0c785862a2c92a3e173916e8ca88e4caad715dd5f68584d6053c20 \
--hash=sha256:0bd7fe21bfaa52106caa1223ce61224cf30786985f17c63c5d71eff0307ee8af
Expand Down
2 changes: 2 additions & 0 deletions requirements/test.in
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,7 @@ pytest-randomly # https://github.com/pytest-dev/pytest-randomly
pytest-subtests # https://github.com/pytest-dev/pytest-subtests
pytest-xdist # https://pypi.org/project/pytest-xdist/
unittest-parametrize # https://github.com/adamchainz/unittest-parametrize
# Mock Anymail requests.
requests-mock # https://github.com/jamielennox/requests-mock
respx # https://lundberg.github.io/respx/
syrupy # https://github.com/tophat/syrupy
5 changes: 5 additions & 0 deletions requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,11 @@ requests==2.32.0 \
# via
# -r requirements/base.txt
# django-anymail
# requests-mock
requests-mock==1.12.1 \
--hash=sha256:b1e37054004cdd5e56c84454cc7df12b25f90f382159087f4b6915aaeef39563 \
--hash=sha256:e9e12e333b525156e82a3c852f22016b9158220d2f47454de9cae8a77d371401
# via -r requirements/test.in
respx==0.21.1 \
--hash=sha256:05f45de23f0c785862a2c92a3e173916e8ca88e4caad715dd5f68584d6053c20 \
--hash=sha256:0bd7fe21bfaa52106caa1223ce61224cf30786985f17c63c5d71eff0307ee8af
Expand Down
91 changes: 90 additions & 1 deletion tests/utils/test_emails.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import pytest
from django.core.mail.message import EmailMessage
from factory import Faker

from itou.utils.tasks import AsyncEmailBackend
from itou.utils.tasks import AsyncEmailBackend, _async_send_message


class TestAsyncEmailBackend:
Expand All @@ -28,3 +29,91 @@ def test_send_messages_splits_recipients(self, django_capture_on_commit_callback
assert email.from_email == "[email protected]"
assert email.subject == "subject"
assert email.body == "body"


@pytest.fixture
def anymail_mailjet_settings(settings):
settings.ASYNC_EMAIL_BACKEND = "anymail.backends.mailjet.EmailBackend"
settings.ANYMAIL = {
"MAILJET_API_URL": settings.ANYMAIL["MAILJET_API_URL"],
"MAILJET_API_KEY": "MAILJET_API_SECRET",
"MAILJET_SECRET_KEY": "MAILJET_SECRET_KEY",
}
return settings


class TestAsyncSendMessage:
EXC_TEXT = "Exception: Huey, please retry this task."

def test_send_ok(self, anymail_mailjet_settings, caplog, requests_mock):
requests_mock.post(
f"{anymail_mailjet_settings.ANYMAIL['MAILJET_API_URL']}send",
# https://dev.mailjet.com/email/guides/send-api-v31/#send-in-bulk
json={
"Messages": [
{
"Status": "success",
"To": [
{
"Email": "[email protected]",
"MessageUUID": "124",
"MessageID": 20547681647433001,
"MessageHref": "https://api.mailjet.com/v3/message/20547681647433001",
},
],
},
],
},
)
_async_send_message({"to": ["[email protected]"], "cc": [], "bcc": [], "subject": "Hi", "body": "Hello"})
assert self.EXC_TEXT not in caplog.text

def test_raises_on_error(self, anymail_mailjet_settings, caplog, requests_mock):
"""An exception is raised, to make Huey retry the task."""
requests_mock.post(
f"{anymail_mailjet_settings.ANYMAIL['MAILJET_API_URL']}send",
# https://dev.mailjet.com/email/guides/send-api-v31/#send-in-bulk
json={
"Messages": [
{
"Errors": [
{
"ErrorIdentifier": "88b5ca9f-5f1f-42e7-a45e-9ecbad0c285e",
"ErrorCode": "send-0003",
"StatusCode": 400,
"ErrorMessage": 'At least "HTMLPart", "TextPart" or "TemplateID" must be provided.',
"ErrorRelatedTo": ["HTMLPart", "TextPart"],
},
],
"Status": "error",
},
],
},
)
_async_send_message({"to": ["[email protected]"], "cc": [], "bcc": [], "subject": "Hi", "body": "Hello"})
assert self.EXC_TEXT in caplog.text

def test_logs_to_sentry_after_using_all_retries(self, anymail_mailjet_settings, caplog, requests_mock, mocker):
error_payload = {
"Errors": [
{
"ErrorIdentifier": "88b5ca9f-5f1f-42e7-a45e-9ecbad0c285e",
"ErrorCode": "send-0003",
"StatusCode": 400,
"ErrorMessage": 'At least "HTMLPart", "TextPart" or "TemplateID" must be provided.',
"ErrorRelatedTo": ["HTMLPart", "TextPart"],
},
],
"Status": "error",
}
requests_mock.post(
f"{anymail_mailjet_settings.ANYMAIL['MAILJET_API_URL']}send",
# https://dev.mailjet.com/email/guides/send-api-v31/#send-in-bulk
json={"Messages": [error_payload]},
)
sentry_mock = mocker.patch("itou.utils.tasks.sentry_sdk.capture_message")
_async_send_message(
{"to": ["[email protected]"], "cc": [], "bcc": [], "subject": "Hi", "body": "Hello"}, retries=0
)
sentry_mock.assert_called_once_with(f"Could not send email: {error_payload}", "error")
assert self.EXC_TEXT not in caplog.text

0 comments on commit eee51cd

Please sign in to comment.