From d14b87c910d34ed703288d9e947b407be821b38c Mon Sep 17 00:00:00 2001 From: medmunds Date: Tue, 1 Dec 2015 13:26:21 -0800 Subject: [PATCH] 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. --- djrill/__init__.py | 3 +- djrill/exceptions.py | 40 ++++++++++++---- djrill/mail/backends/djrill.py | 31 +++++++++--- djrill/tests/mock_backend.py | 10 +++- djrill/tests/test_mandrill_integration.py | 48 ++++++++++++------- djrill/tests/test_mandrill_send.py | 57 ++++++++++++++++++++++- docs/history.rst | 12 +++++ docs/installation.rst | 18 ++++++- docs/usage/sending_mail.rst | 22 +++++++++ 9 files changed, 204 insertions(+), 37 deletions(-) diff --git a/djrill/__init__.py b/djrill/__init__.py index 571dc0b..621fdf1 100644 --- 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 diff --git a/djrill/exceptions.py b/djrill/exceptions.py index 147d914..a8a83d6 100644 --- 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 diff --git a/djrill/mail/backends/djrill.py b/djrill/mail/backends/djrill.py index a2c5106..432056e 100644 --- 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 diff --git a/djrill/tests/mock_backend.py b/djrill/tests/mock_backend.py index 9679932..47e0dfa 100644 --- 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 diff --git a/djrill/tests/test_mandrill_integration.py b/djrill/tests/test_mandrill_integration.py index bc8d7fa..fbdfefd 100644 --- 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): diff --git a/djrill/tests/test_mandrill_send.py b/djrill/tests/test_mandrill_send.py index 62fe8c5..ec910a0 100644 --- 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, diff --git a/docs/history.rst b/docs/history.rst index 0567b56..656a6de 100644 --- 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 `, + which raises :exc:`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 diff --git a/docs/installation.rst b/docs/installation.rst index 743f53b..5aaf15b 100644 --- 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 ` 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:: diff --git a/docs/usage/sending_mail.rst b/docs/usage/sending_mail.rst index deca0dd..2853284 100644 --- 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 ` + 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 `_ to view the full API request and error response.) +