From ba52a6d357b48b4b776446bb9fda3c538919fd14 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Mon, 14 Sep 2026 08:44:33 +0000 Subject: [PATCH] Report a refused message instead of swallowing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backend accepted `fail_silently` from `get_connection` and dropped it, so a message the API rejected came back only as a send count one short: callers had no way to learn why, and the reason — GetResponse's own error document, which says whether asking again could work — reached the log and nowhere else. Honouring the flag is what a Django backend owes its callers, and it makes the two halves alternatives rather than a log that hopes someone is reading it. A 2xx that is not a 201 was the worse case: it creates no message and was reported nowhere at all, not even to the log. The refusal is an `OSError`, so generic handling keeps working across backends: `smtplib.SMTPException`, which Django's own SMTP backend raises, and the `requests.RequestException` wrapped here are both subclasses of it already. The refusal the log still carries names the reason through an argument rather than inside the message, so a monitor grouping by message sees one recurring event rather than a new one per call. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VFDRdudEpf5nC61rJZZ8dz --- README.md | 2 + getresponse/mail.py | 61 +++++++++++++++++++----- pyproject.toml | 2 +- tests/test_faults.py | 110 +++++++++++++++++++++++++++++++++++++++---- 4 files changed, 151 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 8de12f2..1cefd80 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,8 @@ The `.from_email` attribute must be present in `GETRESPONSE_ADDRESSES` as key, w Result returned from sending mail is an int with extra attribute `getresponse_ids`. +A message the API does not accept raises `GetResponseSendError`, carrying GetResponse's own error document where the API answered with one. It is an `OSError`, like the `smtplib` exceptions Django's SMTP backend raises, so code that handles either does not need to know which backend it got. Opening the connection with `fail_silently=True` logs the same reason at `ERROR` instead and leaves it out of the send count, as Django's own backends do. + ## Settings * `GETRESPONSE_API_TOKEN` diff --git a/getresponse/mail.py b/getresponse/mail.py index c7db83c..4c5a3cf 100644 --- a/getresponse/mail.py +++ b/getresponse/mail.py @@ -14,6 +14,38 @@ logger = logging.getLogger(__name__) +# One wording for a refusal however it is reported, and a `%s` so that the reason can be +# logged as an argument rather than formatted into the message it is reported under. +_REFUSAL = 'GetResponse refused a message: %s' + + +class GetResponseSendError(OSError): + """GetResponse did not accept a message for delivery. + + An `OSError` because that is what a caller handling mail generically already + catches: `smtplib.SMTPException`, which Django's own SMTP backend raises, and + `requests.RequestException`, which this wraps, are both subclasses of it. + """ + + def __init__(self, reason): + super().__init__(_REFUSAL % reason) + self.reason = reason + + +def _failure_reason(exc): + """What GetResponse said, or what stopped it being asked. + + A rejected request answers with the API's own error document, which names the + problem where the status line only numbers it; a transport failure has no response. + """ + if exc.response is None: + return str(exc) + try: + return pformat(exc.response.json()) + except json.decoder.JSONDecodeError: + return exc.response.content + + class GetResponseSendResult(int): def __new__(cls, value, getresponse_ids): return super().__new__(cls, value) @@ -23,7 +55,8 @@ def __init__(self, value, getresponse_ids): class GetResponseBackend(BaseEmailBackend): - def __init__(self, **kwargs): + def __init__(self, fail_silently=False, **kwargs): + super().__init__(fail_silently=fail_silently, **kwargs) self._session = None self._endpoint = getattr(settings, 'GETRESPONSE_ENDPOINT', 'https://api.getresponse.com/v3/') self._lock = threading.RLock() @@ -34,7 +67,14 @@ def send_messages(self, msgs): with self._lock, self: # self is used to obtain connection for msg in msgs: - transactional_email_id = self._send_message(msg) + try: + transactional_email_id = self._send_message(msg) + except GetResponseSendError as e: + if not self.fail_silently: + raise + # Silenced for the caller, so the log is where the reason still goes. + logger.exception(_REFUSAL, e.reason) + transactional_email_id = None if transactional_email_id: count += 1 @@ -48,19 +88,14 @@ def _send_message(self, msg): timeout = getattr(settings, 'GETRESPONSE_TIMEOUT', 10) try: response = self._session.post(url, json=payload, timeout=timeout) - except requests.RequestException as e: - logger.exception(f"GetResponse API call failed:\n{e}") - return None - try: response.raise_for_status() except requests.RequestException as e: - try: - reason = pformat(e.response.json()) - except json.decoder.JSONDecodeError: - reason = e.response.content - logger.exception(f"GetResponse API call failed:\n{reason}") - return None - return response.json()["transactionalEmailId"] if response.status_code == 201 else None + raise GetResponseSendError(_failure_reason(e)) from e + if response.status_code != 201: + # The id lives in the body of a 201; any other success code means the API + # accepted the request without creating a message. + raise GetResponseSendError(f'the API answered {response.status_code} instead of creating the message') + return response.json()["transactionalEmailId"] def message_to_payload(self, msg): if len(msg.to) != 1: diff --git a/pyproject.toml b/pyproject.toml index b1f347c..09d7ae6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "django-getresponse" -version = "0.2.1" +version = "0.3.0" description = "A Django email backend for GetResponse" authors = [] readme = "README.md" diff --git a/tests/test_faults.py b/tests/test_faults.py index 9225a73..76ea217 100644 --- a/tests/test_faults.py +++ b/tests/test_faults.py @@ -1,20 +1,43 @@ +import logging + import pytest import requests import responses from django.core.mail import EmailMessage -from getresponse.mail import GetResponseBackend +from getresponse.mail import GetResponseBackend, GetResponseSendError + +ENDPOINT = 'https://api.getresponse.com/v3/transactional-emails' + +# What GetResponse answers a request it will not act on: an error document naming +# the problem, where the status code alone only says which kind of problem it is. +ERROR_DOCUMENT = { + 'code': 1019, + 'codeDescription': 'The server is currently unable to handle the request due to a maintenance', + 'httpStatus': 503, + 'message': 'Transactional email service is experiencing temporary issues', +} @pytest.fixture -def backend(settings): +def settings_with_sender(settings): settings.GETRESPONSE_API_TOKEN = 'test-token' settings.GETRESPONSE_ADDRESSES = { 'webmaster@localhost': 'gr-id-1', } + return settings + + +@pytest.fixture +def backend(settings_with_sender): return GetResponseBackend() +@pytest.fixture +def silent_backend(settings_with_sender): + return GetResponseBackend(fail_silently=True) + + @pytest.fixture def email_message(): return EmailMessage( @@ -25,11 +48,78 @@ def email_message(): @responses.activate -def test_read_timeout(backend, email_message): - responses.add( - responses.POST, - 'https://api.getresponse.com/v3/transactional-emails', - body=requests.exceptions.ReadTimeout(), - ) - n = backend.send_messages([email_message]) - assert n == 0 +def test_a_delivered_message_reports_the_id_it_was_given(backend, email_message): + responses.add(responses.POST, ENDPOINT, json={'transactionalEmailId': 'gr-msg-1'}, status=201) + + result = backend.send_messages([email_message]) + + assert result == 1 + assert result.getresponse_ids == ['gr-msg-1'] + + +@responses.activate +def test_a_transport_failure_reaches_the_caller(backend, email_message): + responses.add(responses.POST, ENDPOINT, body=requests.exceptions.ReadTimeout('no answer')) + + with pytest.raises(GetResponseSendError) as refusal: + backend.send_messages([email_message]) + + # No response came back, so the exception raised on the way out is the whole + # of what there is to say about why. + assert 'no answer' in str(refusal.value) + assert isinstance(refusal.value.__cause__, requests.exceptions.ReadTimeout) + + +@responses.activate +def test_a_rejected_request_reaches_the_caller_with_what_the_api_said(backend, email_message): + responses.add(responses.POST, ENDPOINT, json=ERROR_DOCUMENT, status=503) + + with pytest.raises(GetResponseSendError) as refusal: + backend.send_messages([email_message]) + + # The error document rather than the status line: "1019, under maintenance" is + # what tells a caller whether asking again could work. + assert '1019' in str(refusal.value) + assert isinstance(refusal.value.__cause__, requests.HTTPError) + # A caller handling mail generically catches `OSError`, which is what both + # `smtplib.SMTPException` and the wrapped `requests` failure already are. + assert isinstance(refusal.value, OSError) + + +@responses.activate +def test_a_success_that_creates_nothing_reaches_the_caller(backend, email_message): + """A 2xx that is not a 201 carries no id, so nothing was queued for delivery. + + The send count cannot say so on its own — it comes back one short either way — + which is why this path reports a reason like any other refusal. + """ + responses.add(responses.POST, ENDPOINT, json={}, status=202) + + with pytest.raises(GetResponseSendError, match='202'): + backend.send_messages([email_message]) + + +@responses.activate +def test_a_silenced_refusal_is_logged_rather_than_lost(silent_backend, email_message, caplog): + responses.add(responses.POST, ENDPOINT, json=ERROR_DOCUMENT, status=503) + + with caplog.at_level(logging.ERROR): + result = silent_backend.send_messages([email_message]) + + assert result == 0 + record, = caplog.records + assert '1019' in record.getMessage() + # What varies is an argument, so the message itself is the same for every refusal. + assert '1019' not in record.msg + assert record.exc_info is not None + + +@responses.activate +def test_a_silenced_transport_failure_is_logged_rather_than_lost(silent_backend, email_message, caplog): + responses.add(responses.POST, ENDPOINT, body=requests.exceptions.ReadTimeout('no answer')) + + with caplog.at_level(logging.ERROR): + result = silent_backend.send_messages([email_message]) + + assert result == 0 + assert 'no answer' in caplog.records[0].getMessage()