Raise error for invalid/rejected recipients

Raise new MandrillRecipientsRefused exception when Mandrill returns 'reject' or 'invalid' status for *all* recipients of a message. (Similar to Django's SMTP email backend raising SMTPRecipientsRefused.) Add setting MANDRILL_IGNORE_RECIPIENT_STATUS to override the new exception. Trap JSON parsing errors in Mandrill API response, and raise MandrillAPIError for them. (Helps with #93.) Closes #80. Closes #81.
2025-12-20 03:41:05 -05:00 · 2015-12-01 13:26:21 -08:00
parent 8433e6d660
commit d14b87c910
9 changed files with 204 additions and 37 deletions
--- a/djrill/init.py
+++ b/djrill/init.py
@@ -1,3 +1,2 @@
 from ._version import __version__, VERSION
-from .exceptions import MandrillAPIError, NotSupportedByMandrillError
-
+from .exceptions import MandrillAPIError, MandrillRecipientsRefused, NotSupportedByMandrillError
--- a/djrill/exceptions.py
+++ b/djrill/exceptions.py
@@ -2,6 +2,23 @@ import json
 from requests import HTTPError


+def format_response(response):
+    """Return a string-formatted version of response
+
+    Format json if available, else just return text.
+    Returns "" if neither json nor text available.
+    """
+    try:
+        json_response = response.json()
+        return "\n" + json.dumps(json_response, indent=2)
+    except (AttributeError, KeyError, ValueError):  # not JSON = ValueError
+        try:
+            return response.text
+        except AttributeError:
+            pass
+    return ""
+
+
 class MandrillAPIError(HTTPError):
    """Exception for unsuccessful response from Mandrill API."""
    def __init__(self, status_code, response=None, log_message=None, *args, **kwargs):
@@ -15,14 +32,21 @@ class MandrillAPIError(HTTPError):
        if self.log_message:
            message += "\n" + self.log_message
        # Include the Mandrill response, nicely formatted, if possible
-        try:
-            json_response = self.response.json()
-            message += "\nMandrill response:\n" + json.dumps(json_response, indent=2)
-        except (AttributeError, KeyError, ValueError):  # not JSON = ValueError
-            try:
-                message += "\nMandrill response: " + self.response.text
-            except AttributeError:
-                pass
+        if self.response is not None:
+            message += "\nMandrill response: " + format_response(self.response)
+        return message
+
+
+class MandrillRecipientsRefused(IOError):
+    """Exception for send where all recipients are invalid or rejected."""
+    def __init__(self, message, response=None, *args, **kwargs):
+        super(MandrillRecipientsRefused, self).__init__(message, *args, **kwargs)
+        self.response = response
+
+    def __str__(self):
+        message = self.args[0]
+        if self.response is not None:
+            message += "\nMandrill response: " + format_response(self.response)
        return message


--- a/djrill/mail/backends/djrill.py
+++ b/djrill/mail/backends/djrill.py
@@ -12,7 +12,7 @@ from django.core.mail.backends.base import BaseEmailBackend
 from django.core.mail.message import sanitize_address, DEFAULT_ATTACHMENT_MIME_TYPE

 from ..._version import __version__
-from ...exceptions import MandrillAPIError, NotSupportedByMandrillError
+from ...exceptions import MandrillAPIError, MandrillRecipientsRefused, NotSupportedByMandrillError


 def encode_date_for_mandrill(dt):
@@ -56,6 +56,7 @@ class DjrillBackend(BaseEmailBackend):
            self.global_settings[setting_key] = settings.MANDRILL_SETTINGS[setting_key]

        self.subaccount = getattr(settings, "MANDRILL_SUBACCOUNT", None)
+        self.ignore_recipient_status = getattr(settings, "MANDRILL_IGNORE_RECIPIENT_STATUS", False)

        if not self.api_key:
            raise ImproperlyConfigured("You have not set your mandrill api key "
@@ -124,6 +125,8 @@ class DjrillBackend(BaseEmailBackend):
        return num_sent

    def _send(self, message):
+        message.mandrill_response = None  # until we have a response
+
        if not message.recipients():
            return False

@@ -172,10 +175,6 @@ class DjrillBackend(BaseEmailBackend):
        response = self.session.post(api_url, data=api_data)

        if response.status_code != 200:
-
-            # add a mandrill_response for the sake of being explicit
-            message.mandrill_response = None
-
            if not self.fail_silently:
                log_message = "Failed to send a message"
                if 'to' in msg_dict:
@@ -190,7 +189,27 @@ class DjrillBackend(BaseEmailBackend):
            return False

        # add the response from mandrill to the EmailMessage so callers can inspect it
-        message.mandrill_response = response.json()
+        try:
+            message.mandrill_response = response.json()
+            recipient_status = [item["status"] for item in message.mandrill_response]
+        except (ValueError, KeyError):
+            if not self.fail_silently:
+                raise MandrillAPIError(
+                    status_code=response.status_code,
+                    response=response,
+                    log_message="Error parsing Mandrill API response")
+            return False
+
+        # Error if *all* recipients are invalid or refused
+        # (This behavior parallels smtplib.SMTPRecipientsRefused from Django's SMTP EmailBackend)
+        if (not self.ignore_recipient_status and
+                all([status in ('invalid', 'rejected') for status in recipient_status])):
+            if not self.fail_silently:
+                raise MandrillRecipientsRefused(
+                    "All message recipients were rejected or invalid",
+                    response=response
+                )
+            return False

        return True

--- a/djrill/tests/mock_backend.py
+++ b/djrill/tests/mock_backend.py
@@ -7,6 +7,14 @@ from django.test import TestCase
 from django.test.utils import override_settings


+MANDRILL_SUCCESS_RESPONSE = six.b("""[{
+    "email": "to@example.com",
+    "status": "sent",
+    "_id": "abc123",
+    "reject_reason": null
+}]""")
+
+
@override_settings(MANDRILL_API_KEY="FAKE_API_KEY_FOR_TESTING",
                   EMAIL_BACKEND="djrill.mail.backends.djrill.DjrillBackend")
 class DjrillBackendMockAPITestCase(TestCase):
@@ -14,7 +22,7 @@ class DjrillBackendMockAPITestCase(TestCase):

    class MockResponse(requests.Response):
        """requests.post return value mock sufficient for DjrillBackend"""
-        def __init__(self, status_code=200, raw=six.b("{}"), encoding='utf-8'):
+        def __init__(self, status_code=200, raw=six.b(MANDRILL_SUCCESS_RESPONSE), encoding='utf-8'):
            super(DjrillBackendMockAPITestCase.MockResponse, self).__init__()
            self.status_code = status_code
            self.encoding = encoding
--- a/djrill/tests/test_mandrill_integration.py
+++ b/djrill/tests/test_mandrill_integration.py
@@ -7,7 +7,7 @@ from django.core import mail
 from django.test import TestCase
 from django.test.utils import override_settings

-from djrill import MandrillAPIError
+from djrill import MandrillAPIError, MandrillRecipientsRefused


 MANDRILL_TEST_API_KEY = os.getenv('MANDRILL_TEST_API_KEY')
@@ -58,25 +58,41 @@ class DjrillIntegrationTests(TestCase):
    def test_invalid_to(self):
        # Example of detecting when a recipient is not a valid email address
        self.message.to = ['invalid@localhost']
-        sent_count = self.message.send()
-        self.assertEqual(sent_count, 1)  # The send call is "successful"...
-        # noinspection PyUnresolvedReferences
-        response = self.message.mandrill_response
-        if response[0]['status'] == 'queued':
-            self.skipTest("Mandrill queued the send -- can't complete this test")
-        self.assertEqual(response[0]['status'], 'invalid')  # ... but the mail is not delivered
+        try:
+            self.message.send()
+        except MandrillRecipientsRefused:
+            # Mandrill refused to deliver the mail -- message.mandrill_response will tell you why:
+            # noinspection PyUnresolvedReferences
+            response = self.message.mandrill_response
+            self.assertEqual(response[0]['status'], 'invalid')
+        else:
+            # Sometimes Mandrill queues these test sends
+            # noinspection PyUnresolvedReferences
+            response = self.message.mandrill_response
+            if response[0]['status'] == 'queued':
+                self.skipTest("Mandrill queued the send -- can't complete this test")
+            else:
+                self.fail("Djrill did not raise MandrillRecipientsRefused for invalid recipient")

    def test_rejected_to(self):
        # Example of detecting when a recipient is on Mandrill's rejection blacklist
        self.message.to = ['reject@test.mandrillapp.com']
-        sent_count = self.message.send()
-        self.assertEqual(sent_count, 1)  # The send call is "successful"...
-        # noinspection PyUnresolvedReferences
-        response = self.message.mandrill_response
-        if response[0]['status'] == 'queued':
-            self.skipTest("Mandrill queued the send -- can't complete this test")
-        self.assertEqual(response[0]['status'], 'rejected')  # ... but the mail is not delivered
-        self.assertEqual(response[0]['reject_reason'], 'test')  # ... and here's why
+        try:
+            self.message.send()
+        except MandrillRecipientsRefused:
+            # Mandrill refused to deliver the mail -- message.mandrill_response will tell you why:
+            # noinspection PyUnresolvedReferences
+            response = self.message.mandrill_response
+            self.assertEqual(response[0]['status'], 'rejected')
+            self.assertEqual(response[0]['reject_reason'], 'test')
+        else:
+            # Sometimes Mandrill queues these test sends
+            # noinspection PyUnresolvedReferences
+            response = self.message.mandrill_response
+            if response[0]['status'] == 'queued':
+                self.skipTest("Mandrill queued the send -- can't complete this test")
+            else:
+                self.fail("Djrill did not raise MandrillRecipientsRefused for blacklist recipient")

    @override_settings(MANDRILL_API_KEY="Hey, that's not an API key!")
    def test_invalid_api_key(self):
--- a/djrill/tests/test_mandrill_send.py
+++ b/djrill/tests/test_mandrill_send.py
@@ -18,7 +18,7 @@ from django.core.mail import make_msgid
 from django.test import TestCase
 from django.test.utils import override_settings

-from djrill import MandrillAPIError, NotSupportedByMandrillError
+from djrill import MandrillAPIError, MandrillRecipientsRefused, NotSupportedByMandrillError

 from .mock_backend import DjrillBackendMockAPITestCase

@@ -515,7 +515,7 @@ class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase):

    def test_send_attaches_mandrill_response(self):
        """ The mandrill_response should be attached to the message when it is sent """
-        response = [{'mandrill_response': 'would_be_here'}]
+        response = [{'email': 'to1@example.com', 'status': 'sent'}]
        self.mock_post.return_value = self.MockResponse(raw=six.b(json.dumps(response)))
        msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'],)
        sent = msg.send()
@@ -530,6 +530,14 @@ class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase):
        self.assertEqual(sent, 0)
        self.assertIsNone(msg.mandrill_response)

+    def test_send_unparsable_mandrill_response(self):
+        """If the send succeeds, but a non-JSON API response, should raise an API exception"""
+        self.mock_post.return_value = self.MockResponse(status_code=500, raw=b"this isn't json")
+        msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'],)
+        with self.assertRaises(MandrillAPIError):
+            msg.send()
+        self.assertIsNone(msg.mandrill_response)
+
    def test_json_serialization_errors(self):
        """Try to provide more information about non-json-serializable data"""
        self.message.global_merge_vars = {'PRICE': Decimal('19.99')}
@@ -547,6 +555,51 @@ class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase):
            self.message.send()


+class DjrillRecipientsRefusedTests(DjrillBackendMockAPITestCase):
+    """Djrill raises MandrillRecipientsRefused when *all* recipients are rejected or invalid"""
+
+    def test_recipients_refused(self):
+        msg = mail.EmailMessage('Subject', 'Body', 'from@example.com',
+                                ['invalid@localhost', 'reject@test.mandrillapp.com'])
+        self.mock_post.return_value = self.MockResponse(status_code=200, raw=b"""
+            [{ "email": "invalid@localhost", "status": "invalid" },
+             { "email": "reject@test.mandrillapp.com", "status": "rejected" }]""")
+        with self.assertRaises(MandrillRecipientsRefused):
+            msg.send()
+
+    def test_fail_silently(self):
+        self.mock_post.return_value = self.MockResponse(status_code=200, raw=b"""
+            [{ "email": "invalid@localhost", "status": "invalid" },
+             { "email": "reject@test.mandrillapp.com", "status": "rejected" }]""")
+        sent = mail.send_mail('Subject', 'Body', 'from@example.com',
+                              ['invalid@localhost', 'reject@test.mandrillapp.com'],
+                              fail_silently=True)
+        self.assertEqual(sent, 0)
+
+    def test_mixed_response(self):
+        """If *any* recipients are valid or queued, no exception is raised"""
+        msg = mail.EmailMessage('Subject', 'Body', 'from@example.com',
+                                ['invalid@localhost', 'valid@example.com',
+                                 'reject@test.mandrillapp.com', 'also.valid@example.com'])
+        self.mock_post.return_value = self.MockResponse(status_code=200, raw=b"""
+            [{ "email": "invalid@localhost", "status": "invalid" },
+             { "email": "valid@example.com", "status": "sent" },
+             { "email": "reject@test.mandrillapp.com", "status": "rejected" },
+             { "email": "also.valid@example.com", "status": "queued" }]""")
+        sent = msg.send()
+        self.assertEqual(sent, 1)  # one message sent, successfully, to 2 of 4 recipients
+
+    @override_settings(MANDRILL_IGNORE_RECIPIENT_STATUS=True)
+    def test_settings_override(self):
+        """Setting restores Djrill 1.x behavior"""
+        self.mock_post.return_value = self.MockResponse(status_code=200, raw=b"""
+            [{ "email": "invalid@localhost", "status": "invalid" },
+             { "email": "reject@test.mandrillapp.com", "status": "rejected" }]""")
+        sent = mail.send_mail('Subject', 'Body', 'from@example.com',
+                              ['invalid@localhost', 'reject@test.mandrillapp.com'])
+        self.assertEqual(sent, 1)  # refused message is included in sent count
+
+
@override_settings(MANDRILL_SETTINGS={
    'from_name': 'Djrill Test',
    'important': True,
--- a/docs/history.rst
+++ b/docs/history.rst
@@ -48,6 +48,18 @@ Removed DjrillAdminSite
  (Do this only if you changed to SimpleAdminConfig for Djrill, and aren't
  creating custom admin sites for any other Django apps you use.)

+Added exception for invalid or rejected recipients
+  Djrill 2.0 raises a new :exc:`djrill.MandrillRecipientsRefused` exception when
+  all recipients of a message invalid or rejected by Mandrill. This parallels
+  the behavior of Django's default :setting:`SMTP email backend <EMAIL_BACKEND>`,
+  which raises :exc:`SMTPRecipientsRefused <smtplib.SMTPRecipientsRefused>` when
+  all recipients are refused.
+
+  Your email-sending code should handle this exception (along with other
+  exceptions that could occur during a send). However, if you want to retain the
+  Djrill 1.x behavior and treat invalid or rejected recipients as successful sends,
+  you can set :setting:`MANDRILL_IGNORE_RECIPIENT_STATUS` to ``True`` in your settings.py.
+
 Removed unintended date-to-string conversion
  If your code was relying on Djrill to automatically convert date or datetime
  values to strings in :attr:`merge_vars`, :attr:`metadata`, or other Mandrill
--- a/docs/installation.rst
+++ b/docs/installation.rst
@@ -47,11 +47,25 @@ Djrill includes optional support for Mandrill webhooks, including inbound email.
 See the Djrill :ref:`webhooks <webhooks>` section for configuration details.


-Mandrill Subaccounts (Optional)
-------------------------------
+Other Optional Settings
+-----------------------
+
+.. setting:: MANDRILL_IGNORE_RECIPIENT_STATUS
+
+MANDRILL_IGNORE_RECIPIENT_STATUS
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Set to ``True`` to disable :exc:`djrill.MandrillRecipientsRefused` exceptions
+on invalid or rejected recipients. (Default ``False``.)
+
+.. versionadded:: 2.0
+

 .. setting:: MANDRILL_SUBACCOUNT

+MANDRILL_SUBACCOUNT
+~~~~~~~~~~~~~~~~~~~
+
 If you are using Mandrill's `subaccounts`_ feature, you can globally set the
 subaccount for all messages sent through Djrill::

--- a/docs/usage/sending_mail.rst
+++ b/docs/usage/sending_mail.rst
@@ -362,6 +362,27 @@ Exceptions
    of :exc:`ValueError`).


+.. exception:: djrill.MandrillRecipientsRefused
+
+    If *all* recipients (to, cc, bcc) of a message are invalid or rejected by Mandrill
+    (e.g., because they are your Mandrill blacklist), the send call will raise a
+    :exc:`~!djrill.MandrillRecipientsRefused` exception.
+    You can examine the message's :ref:`mandrill_response property <mandrill-response>`
+    to determine the cause of the error.
+
+    If a single message is sent to multiple recipients, and *any* recipient is valid
+    (or the message is queued by Mandrill because of rate limiting or :attr:`send_at`), then
+    this exception will not be raised. You can still examine the mandrill_response
+    property after the send to determine the status of each recipient.
+
+    You can disable this exception by setting :setting:`MANDRILL_IGNORE_RECIPIENT_STATUS`
+    to True in your settings.py, which will cause Djrill to treat any non-API-error response
+    from Mandrill as a successful send.
+
+    .. versionadded:: 2.0
+       Djrill 1.x behaved as if ``MANDRILL_IGNORE_RECIPIENT_STATUS = True``.
+
+
 .. exception:: djrill.MandrillAPIError

    If the Mandrill API fails or returns an error response, the send call will
@@ -370,3 +391,4 @@ Exceptions
    help explain what went wrong. (Tip: you can also check Mandrill's
    `API error log <https://mandrillapp.com/settings/api>`_ to view the full API
    request and error response.)
+