Backends: identify source of problem in AnymailInvalidAddress message

Include the name of the field with the the unparsable email address
in AnymailInvalidAddress error messages.

Should help tracking down problems like in #98.

Author: medmunds

anymail/backends/base.py

@@ -268,7 +268,11 @@ def __init__(self, message, defaults, backend):
                 if converter is not None:
                     if not callable(converter):
                         converter = getattr(self, converter)
-                    value = converter(value)
+                    if converter in (parse_address_list, parse_single_address):
+                        # hack to include field name in error message
+                        value = converter(value, field=attr)
+                    else:
+                        value = converter(value)
             if value is not UNSET:
                 if attr == 'body':
                     setter = self.set_html_body if message.content_subtype == 'html' else self.set_text_body
@@ -296,16 +300,16 @@ def process_extra_headers(self, headers):
             # message.extra_headers['Reply-To'] will override message.reply_to
             # (because the extra_headers attr is processed after reply_to).
             # This matches the behavior of Django's EmailMessage.message().
-            self.set_reply_to(parse_address_list([reply_to]))
+            self.set_reply_to(parse_address_list([reply_to], field="extra_headers['Reply-To']"))
 
         if 'From' in headers:
             # If message.extra_headers['From'] is supplied, it should override message.from_email,
             # but message.from_email should be used as the envelope_sender. See:
             #   - https://code.djangoproject.com/ticket/9214
             #   - https://github.com/django/django/blob/1.8/django/core/mail/message.py#L269
             #   - https://github.com/django/django/blob/1.8/django/core/mail/backends/smtp.py#L118
-            header_from = parse_address_list(headers.pop('From'))
-            envelope_sender = parse_single_address(self.message.from_email)  # must be single address
+            header_from = parse_address_list(headers.pop('From'), field="extra_headers['From']")
+            envelope_sender = parse_single_address(self.message.from_email, field="from_email")  # must be single
             self.set_from_email_list(header_from)
             self.set_envelope_sender(envelope_sender)
 

anymail/utils.py

@@ -116,7 +116,7 @@ def update_deep(dct, other):
     # (like dict.update(), no return value)
 
 
-def parse_address_list(address_list):
+def parse_address_list(address_list, field=None):
     """Returns a list of EmailAddress objects from strings in address_list.
 
     Essentially wraps :func:`email.utils.getaddresses` with better error
@@ -127,6 +127,8 @@ def parse_address_list(address_list):
 
     :param list[str]|str|None|list[None] address_list:
         the address or addresses to parse
+    :param str|None field:
+        optional description of the source of these addresses, for error message
     :return list[:class:`EmailAddress`]:
     :raises :exc:`AnymailInvalidAddress`:
     """
@@ -151,26 +153,32 @@ def parse_address_list(address_list):
     for address in parsed:
         if address.username == '' or address.domain == '':
             # Django SMTP allows username-only emails, but they're not meaningful with an ESP
-            errmsg = "Invalid email address '%s' parsed from '%s'." % (
-                address.addr_spec, ", ".join(address_list_strings))
+            errmsg = "Invalid email address '{problem}' parsed from '{source}'{where}.".format(
+                problem=address.addr_spec,
+                source=", ".join(address_list_strings),
+                where=" in `%s`" % field if field else "",
+            )
             if len(parsed) > len(address_list):
                 errmsg += " (Maybe missing quotes around a display-name?)"
             raise AnymailInvalidAddress(errmsg)
 
     return parsed
 
 
-def parse_single_address(address):
+def parse_single_address(address, field=None):
     """Parses a single EmailAddress from str address, or raises AnymailInvalidAddress
 
     :param str address: the fully-formatted email str to parse
+    :param str|None field: optional description of the source of this address, for error message
     :return :class:`EmailAddress`: if address contains a single email
     :raises :exc:`AnymailInvalidAddress`: if address contains no or multiple emails
     """
-    parsed = parse_address_list([address])
+    parsed = parse_address_list([address], field=field)
     count = len(parsed)
     if count > 1:
-        raise AnymailInvalidAddress("Only one email address is allowed; found %d in %r" % (count, address))
+        raise AnymailInvalidAddress(
+            "Only one email address is allowed; found {count} in '{address}'{where}.".format(
+                count=count, address=address, where=" in `%s`" % field if field else ""))
     else:
         return parsed[0]
 

tests/test_general_backend.py

@@ -11,7 +11,7 @@
 from django.utils.timezone import utc
 from django.utils.translation import ugettext_lazy
 
-from anymail.exceptions import AnymailConfigurationError, AnymailUnsupportedFeature
+from anymail.exceptions import AnymailConfigurationError, AnymailInvalidAddress, AnymailUnsupportedFeature
 from anymail.message import AnymailMessage
 
 from .utils import AnymailTestMixin
@@ -319,6 +319,38 @@ def test_explains_reply_to_must_be_list_lazy(self):
         with self.assertRaisesMessage(TypeError, '"reply_to" attribute must be a list or other iterable'):
             self.message.send()
 
+    def test_identifies_source_of_parsing_errors(self):
+        """Errors parsing email addresses should say which field had the problem"""
+        # Note: General email address parsing tests are in test_utils.ParseAddressListTests.
+        # This just checks the error includes the field name when parsing for sending a message.
+        self.message.from_email = ''
+        with self.assertRaisesMessage(AnymailInvalidAddress,
+                                      "Invalid email address '' parsed from '' in `from_email`."):
+            self.message.send()
+        self.message.from_email = '[email protected]'
+
+        # parse_address_list
+        self.message.to = ['[email protected]', 'oops']
+        with self.assertRaisesMessage(AnymailInvalidAddress,
+                                      "Invalid email address 'oops' parsed from '[email protected], oops' in `to`."):
+            self.message.send()
+        self.message.to = ['[email protected]']
+
+        # parse_single_address
+        self.message.envelope_sender = '[email protected], [email protected]'
+        with self.assertRaisesMessage(AnymailInvalidAddress,
+                                      "Only one email address is allowed; found 2"
+                                      " in '[email protected], [email protected]' in `envelope_sender`."):
+            self.message.send()
+        delattr(self.message, 'envelope_sender')
+
+        # process_extra_headers
+        self.message.extra_headers['From'] = 'Mail, Inc. <[email protected]>'
+        with self.assertRaisesMessage(AnymailInvalidAddress,
+                                      "Invalid email address 'Mail' parsed from 'Mail, Inc. <[email protected]>'"
+                                      " in `extra_headers['From']`. (Maybe missing quotes around a display-name?)"):
+            self.message.send()
+
 
 def flatten_emails(emails):
     return [str(email) for email in emails]