From 00ddd2f4f6ba8d970a5e8ceb1de97115179cd560 Mon Sep 17 00:00:00 2001 From: medmunds Date: Wed, 13 May 2015 15:43:54 -0700 Subject: [PATCH] Improve MandrillAPIError.__str__ * Include formatted response from Mandrill API (if any) * Clean up recipient address(es) --- djrill/exceptions.py | 12 ++++++++++-- djrill/mail/backends/djrill.py | 13 +++++++------ djrill/tests/test_mandrill_send.py | 25 +++++++++++++++++++++++++ docs/history.rst | 2 ++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/djrill/exceptions.py b/djrill/exceptions.py index 6672739..c7913b8 100644 --- a/djrill/exceptions.py +++ b/djrill/exceptions.py @@ -1,3 +1,4 @@ +import json from requests import HTTPError import warnings @@ -14,8 +15,15 @@ class MandrillAPIError(HTTPError): message = "Mandrill API response %d" % self.status_code if self.log_message: message += "\n" + self.log_message - if self.response: - message += "\nResponse: " + getattr(self.response, 'content', "") + # 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 return message diff --git a/djrill/mail/backends/djrill.py b/djrill/mail/backends/djrill.py index f403039..bb9d459 100644 --- a/djrill/mail/backends/djrill.py +++ b/djrill/mail/backends/djrill.py @@ -137,15 +137,16 @@ class DjrillBackend(BaseEmailBackend): message.mandrill_response = None if not self.fail_silently: - from_email = msg_dict.get('from_email', None) - from_message = "" - if from_email: - from_message = ", from %s" % from_email + log_message = "Failed to send a message" + if 'to' in msg_dict: + log_message += " to " + ','.join( + to['email'] for to in msg_dict.get('to', []) if 'email' in to) + if 'from_email' in msg_dict: + log_message += " from %s" % msg_dict['from_email'] raise MandrillAPIError( status_code=response.status_code, response=response, - log_message="Failed to send a message to %s%s" % - (msg_dict['to'], from_message)) + log_message=log_message) return False # add the response from mandrill to the EmailMessage so callers can inspect it diff --git a/djrill/tests/test_mandrill_send.py b/djrill/tests/test_mandrill_send.py index d84c5ce..8bbbc9a 100644 --- a/djrill/tests/test_mandrill_send.py +++ b/djrill/tests/test_mandrill_send.py @@ -308,6 +308,31 @@ class DjrillBackendTests(DjrillBackendMockAPITestCase): ['to@example.com'], fail_silently=True) self.assertEqual(sent, 0) + def test_api_error_includes_details(self): + """MandrillAPIError should include Mandrill's error message""" + msg = mail.EmailMessage('Subject', 'Body', 'from@example.com', ['to@example.com']) + + # JSON error response: + error_response = b"""{ + "status": "error", + "code": 12, + "name": "Error_Name", + "message": "Helpful explanation from Mandrill" + }""" + self.mock_post.return_value = self.MockResponse(status_code=400, raw=error_response) + with self.assertRaisesMessage(MandrillAPIError, "Helpful explanation from Mandrill"): + msg.send() + + # Non-JSON error response: + self.mock_post.return_value = self.MockResponse(status_code=500, raw=b"Invalid API key") + with self.assertRaisesMessage(MandrillAPIError, "Invalid API key"): + msg.send() + + # No content in the error response: + self.mock_post.return_value = self.MockResponse(status_code=502, raw=None) + with self.assertRaises(MandrillAPIError): + msg.send() + class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase): """Test Djrill backend support for Mandrill-specific features""" diff --git a/docs/history.rst b/docs/history.rst index 8ad4d26..a96ce0f 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -76,6 +76,8 @@ Version 1.4 (development): (Specifying a :ref:`Reply-To header ` still works, with any version of Django, and will override the reply_to param if you use both.) +* Include Mandrill error response in str(MandrillAPIError), + to make errors easier to understand. * More-helpful exception when using a non-JSON-serializable type in merge_vars and other Djrill message attributes * Deprecation warnings for upcoming 2.0 changes (see above)