Report a refused message instead of swallowing it

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`

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:

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"

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()