Cleanup Djrill exceptions

* Add common base DjrillException * Simplify backend by moving logic to describe errors into base DjrillException * Add NotSerializableForMandrillError for JSON serialization errors
2025-12-20 03:41:05 -05:00 · 2015-12-02 10:52:20 -08:00
parent 221530ae11
commit b8cdc6ce82
7 changed files with 121 additions and 68 deletions
--- a/djrill/init.py
+++ b/djrill/init.py
@@ -1,2 +1,3 @@
 from ._version import __version__, VERSION
-from .exceptions import MandrillAPIError, MandrillRecipientsRefused, NotSupportedByMandrillError
+from .exceptions import (MandrillAPIError, MandrillRecipientsRefused,
+                         NotSerializableForMandrillError, NotSupportedByMandrillError)
--- a/djrill/exceptions.py
+++ b/djrill/exceptions.py
@@ -2,55 +2,86 @@ import json
 from requests import HTTPError


-def format_response(response):
-    """Return a string-formatted version of response
+class DjrillError(Exception):
+    """Base class for exceptions raised by Djrill

-    Format json if available, else just return text.
-    Returns "" if neither json nor text available.
+    Overrides __str__ to provide additional information about
+    Mandrill API call and response.
    """
-    try:
-        json_response = response.json()
-        return "\n" + json.dumps(json_response, indent=2)
-    except (AttributeError, KeyError, ValueError):  # not JSON = ValueError
+
+    def __init__(self, *args, **kwargs):
+        """
+        Optional kwargs:
+          payload: data arg (*not* json-stringified) for the Mandrill send call
+          response: requests.Response from the send call
+        """
+        self.payload = kwargs.pop('payload', None)
+        if isinstance(self, HTTPError):
+            # must leave response in kwargs for HTTPError
+            self.response = kwargs.get('response', None)
+        else:
+            self.response = kwargs.pop('response', None)
+        super(DjrillError, self).__init__(*args, **kwargs)
+
+    def __str__(self):
+        parts = [
+            " ".join([str(arg) for arg in self.args]),
+            self.describe_send(),
+            self.describe_response(),
+        ]
+        return "\n".join(filter(None, parts))
+
+    def describe_send(self):
+        """Return a string describing the Mandrill send in self.payload, or None"""
+        if self.payload is None:
+            return None
+        description = "Sending a message"
        try:
-            return response.text
-        except AttributeError:
+            to_emails = [to['email'] for to in self.payload['message']['to']]
+            description += " to %s" % ','.join(to_emails)
+        except KeyError:
            pass
-    return ""
+        try:
+            description += " from %s" % self.payload['message']['from_email']
+        except KeyError:
+            pass
+        return description
+
+    def describe_response(self):
+        """Return a formatted string of self.response, or None"""
+        if self.response is None:
+            return None
+        description = "Mandrill API response %d:" % self.response.status_code
+        try:
+            json_response = self.response.json()
+            description += "\n" + json.dumps(json_response, indent=2)
+        except (AttributeError, KeyError, ValueError):  # not JSON = ValueError
+            try:
+                description += self.response.text
+            except AttributeError:
+                pass
+        return description


-class MandrillAPIError(HTTPError):
+class MandrillAPIError(DjrillError, HTTPError):
    """Exception for unsuccessful response from Mandrill API."""
-    def __init__(self, status_code, response=None, log_message=None, *args, **kwargs):
+
+    def __init__(self, *args, **kwargs):
        super(MandrillAPIError, self).__init__(*args, **kwargs)
-        self.status_code = status_code
-        self.response = response  # often contains helpful Mandrill info
-        self.log_message = log_message
-
-    def __str__(self):
-        message = "Mandrill API response %d" % self.status_code
-        if self.log_message:
-            message += "\n" + self.log_message
-        # Include the Mandrill response, nicely formatted, if possible
        if self.response is not None:
-            message += "\nMandrill response: " + format_response(self.response)
-        return message
+            self.status_code = self.response.status_code


-class MandrillRecipientsRefused(IOError):
+class MandrillRecipientsRefused(DjrillError):
    """Exception for send where all recipients are invalid or rejected."""
-    def __init__(self, message, response=None, *args, **kwargs):
+
+    def __init__(self, message=None, *args, **kwargs):
+        if message is None:
+            message = "All message recipients were rejected or invalid"
        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


-class NotSupportedByMandrillError(ValueError):
+class NotSupportedByMandrillError(DjrillError, ValueError):
    """Exception for email features that Mandrill doesn't support.

    This is typically raised when attempting to send a Django EmailMessage that
@@ -64,3 +95,21 @@ class NotSupportedByMandrillError(ValueError):
    avoid duplicating Mandrill's validation logic locally.)

    """
+
+
+class NotSerializableForMandrillError(DjrillError, TypeError):
+    """Exception for data that Djrill doesn't know how to convert to JSON.
+
+    This typically results from including something like a date or Decimal
+    in your merge_vars (or other Mandrill-specific EmailMessage option).
+
+    """
+    # inherits from TypeError for backwards compatibility with Djrill 1.x
+
+    def __init__(self, message=None, orig_err=None, *args, **kwargs):
+        if message is None:
+            message = "Don't know how to send this data to Mandrill. " \
+                      "Try converting it to a string or number first."
+        if orig_err is not None:
+            message += "\n%s" % str(orig_err)
+        super(NotSerializableForMandrillError, self).__init__(message, *args, **kwargs)
--- a/djrill/mail/backends/djrill.py
+++ b/djrill/mail/backends/djrill.py
@@ -12,7 +12,8 @@ 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, MandrillRecipientsRefused, NotSupportedByMandrillError
+from ...exceptions import (MandrillAPIError, MandrillRecipientsRefused,
+                           NotSerializableForMandrillError, NotSupportedByMandrillError)


 def encode_date_for_mandrill(dt):
@@ -164,28 +165,13 @@ class DjrillBackend(BaseEmailBackend):
            if self.fail_silently:
                return False
            # Add some context to the "not JSON serializable" message
-            if not err.args:
-                err.args = ('',)
-            err.args = (
-                err.args[0] + " in a Djrill message (perhaps it's a merge var?)."
-                              " Try converting it to a string or number first.",
-            ) + err.args[1:]
-            raise err
+            raise NotSerializableForMandrillError(orig_err=err)

        response = self.session.post(api_url, data=api_data)

        if response.status_code != 200:
            if not self.fail_silently:
-                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=log_message)
+                raise MandrillAPIError(payload=api_params, response=response)
            return False

        # add the response from mandrill to the EmailMessage so callers can inspect it
@@ -194,10 +180,8 @@ class DjrillBackend(BaseEmailBackend):
            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")
+                raise MandrillAPIError("Error parsing Mandrill API response",
+                                       payload=api_params, response=response)
            return False

        # Error if *all* recipients are invalid or refused
@@ -205,10 +189,7 @@ class DjrillBackend(BaseEmailBackend):
        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
-                )
+                raise MandrillRecipientsRefused(payload=api_params, response=response)
            return False

        return True
--- a/djrill/tests/test_mandrill_send.py
+++ b/djrill/tests/test_mandrill_send.py
@@ -4,6 +4,7 @@ from __future__ import unicode_literals

 import json
 import os
+import re
 import six
 import unittest
 from base64 import b64decode
@@ -18,7 +19,8 @@ from django.core.mail import make_msgid
 from django.test import TestCase
 from django.test.utils import override_settings

-from djrill import MandrillAPIError, MandrillRecipientsRefused, NotSupportedByMandrillError
+from djrill import (MandrillAPIError, MandrillRecipientsRefused,
+                    NotSerializableForMandrillError, NotSupportedByMandrillError)

 from .mock_backend import DjrillBackendMockAPITestCase

@@ -339,6 +341,9 @@ class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase):
        self.message = mail.EmailMessage('Subject', 'Text Body',
            'from@example.com', ['to@example.com'])

+    def assertStrContains(self, haystack, needle, msg=None):
+        six.assertRegex(self, haystack, re.escape(needle), msg)
+
    def test_tracking(self):
        # First make sure we're not setting the API param if the track_click
        # attr isn't there. (The Mandrill account option of True for html,
@@ -541,17 +546,17 @@ class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase):
    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')}
-        with self.assertRaisesMessage(
-                TypeError,
-                "Decimal('19.99') is not JSON serializable in a Djrill message (perhaps "
-                "it's a merge var?). Try converting it to a string or number first."
-        ):
+        with self.assertRaises(NotSerializableForMandrillError) as cm:
            self.message.send()
+        err = cm.exception
+        self.assertTrue(isinstance(err, TypeError))  # Djrill 1.x re-raised TypeError from json.dumps
+        self.assertStrContains(str(err), "Don't know how to send this data to Mandrill")  # our added context
+        self.assertStrContains(str(err), "Decimal('19.99') is not JSON serializable")  # original message

    def test_dates_not_serialized(self):
        """Pre-2.0 Djrill accidentally serialized dates to ISO"""
        self.message.global_merge_vars = {'SHIP_DATE': date(2015, 12, 2)}
-        with self.assertRaises(TypeError):
+        with self.assertRaises(NotSerializableForMandrillError):
            self.message.send()


--- a/docs/history.rst
+++ b/docs/history.rst
@@ -88,6 +88,8 @@ Other Djrill 2.0 Changes
  (You can also directly manage your own long-lived Djrill connection across multiple sends,
  by calling open and close on :ref:`Django's email backend <django:topic-email-backends>`.)

+* Add :exc:`djrill.NotSerializableForMandrillError`
+

 Older Releases
 --------------
--- a/docs/usage/sending_mail.rst
+++ b/docs/usage/sending_mail.rst
@@ -392,3 +392,15 @@ Exceptions
    `API error log <https://mandrillapp.com/settings/api>`_ to view the full API
    request and error response.)

+
+.. exception:: djrill.NotSerializableForMandrillError
+
+    The send call will raise a :exc:`~!djrill.NotSerializableForMandrillError` exception
+    if the message has attached data which cannot be serialized to JSON for the Mandrill API.
+
+    See :ref:`formatting-merge-data` for more information.
+
+    .. versionadded:: 2.0
+       Djrill 1.x raised a generic `TypeError` in this case.
+       :exc:`~!djrill.NotSerializableForMandrillError` is a subclass of `TypeError`
+       for compatibility with existing code.
--- a/docs/usage/templates.rst
+++ b/docs/usage/templates.rst
@@ -75,6 +75,9 @@ which means advanced template users can include dicts and lists as merge vars
 (for templates designed to handle objects and arrays).
 See the Python :class:`json.JSONEncoder` docs for a list of allowable types.

+Djrill will raise :exc:`djrill.NotSerializableForMandrillError` if you attempt
+to send a message with non-json-serializable data.
+

 How To Use Default Mandrill Subject and From fields
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~