Improve error when reply_to isn't list.

Issue a better error message if message.reply_to
is set to a single string.

(Would also like to do this for to, cc, and bcc,
but Django core EmailMessage.recipients is called
and stumbles over thoses cases before Anymail's
backend gets involved.)

Fixes #57
This commit is contained in:
medmunds
2017-04-04 11:57:11 -07:00
parent 3dd05ae882
commit 09bec9e463
2 changed files with 47 additions and 1 deletions

View File

@@ -1,5 +1,6 @@
from datetime import date, datetime from datetime import date, datetime
import six
from django.conf import settings from django.conf import settings
from django.core.mail.backends.base import BaseEmailBackend from django.core.mail.backends.base import BaseEmailBackend
from django.utils.timezone import is_naive, get_current_timezone, make_aware, utc from django.utils.timezone import is_naive, get_current_timezone, make_aware, utc
@@ -8,7 +9,7 @@ from ..exceptions import AnymailCancelSend, AnymailError, AnymailUnsupportedFeat
from ..message import AnymailStatus from ..message import AnymailStatus
from ..signals import pre_send, post_send from ..signals import pre_send, post_send
from ..utils import (Attachment, ParsedEmail, UNSET, combine, last, get_anymail_setting, from ..utils import (Attachment, ParsedEmail, UNSET, combine, last, get_anymail_setting,
force_non_lazy, force_non_lazy_list, force_non_lazy_dict) force_non_lazy, force_non_lazy_list, force_non_lazy_dict, is_lazy)
class AnymailBaseBackend(BaseEmailBackend): class AnymailBaseBackend(BaseEmailBackend):
@@ -252,6 +253,8 @@ class BasePayload(object):
message_attrs = self.base_message_attrs + self.anymail_message_attrs + self.esp_message_attrs message_attrs = self.base_message_attrs + self.anymail_message_attrs + self.esp_message_attrs
for attr, combiner, converter in message_attrs: for attr, combiner, converter in message_attrs:
value = getattr(message, attr, UNSET) value = getattr(message, attr, UNSET)
if attr in ('to', 'cc', 'bcc', 'reply_to') and value is not UNSET:
self.validate_not_bare_string(attr, value)
if combiner is not None: if combiner is not None:
default_value = self.defaults.get(attr, UNSET) default_value = self.defaults.get(attr, UNSET)
value = combiner(default_value, value) value = combiner(default_value, value)
@@ -273,6 +276,26 @@ class BasePayload(object):
raise AnymailUnsupportedFeature("%s does not support %s" % (self.esp_name, feature), raise AnymailUnsupportedFeature("%s does not support %s" % (self.esp_name, feature),
email_message=self.message, payload=self, backend=self.backend) email_message=self.message, payload=self, backend=self.backend)
#
# Attribute validators
#
def validate_not_bare_string(self, attr, value):
"""EmailMessage to, cc, bcc, and reply_to are specced to be lists of strings.
This catches the common error where a single string is used instead.
(See also checks in EmailMessage.__init__.)
"""
# Note: this actually only runs for reply_to. If to, cc, or bcc are
# set to single strings, you'll end up with an earlier cryptic TypeError
# from EmailMesssage.recipients (called from EmailMessage.send) before
# the Anymail backend even gets involved:
# TypeError: must be str, not list
# TypeError: can only concatenate list (not "str") to list
# TypeError: Can't convert 'list' object to str implicitly
if isinstance(value, six.string_types) or is_lazy(value):
raise TypeError('"{attr}" attribute must be a list or other iterable'.format(attr=attr))
# #
# Attribute converters # Attribute converters
# #

View File

@@ -293,3 +293,26 @@ class LazyStringsTest(TestBackendTestCase):
params = self.get_send_params() params = self.get_send_params()
self.assertNotLazy(params['merge_data']['to@example.com']['duration']) self.assertNotLazy(params['merge_data']['to@example.com']['duration'])
self.assertNotLazy(params['merge_global_data']['order_type']) self.assertNotLazy(params['merge_global_data']['order_type'])
class CatchCommonErrorsTests(TestBackendTestCase):
"""Anymail should catch and provide useful errors for common mistakes"""
def test_explains_reply_to_must_be_list(self):
"""reply_to must be a list (or other iterable), not a single string"""
# Django's EmailMessage.__init__ catches this and warns, but isn't
# involved if you assign attributes later. Anymail should catch that case.
# (This also applies to to, cc, and bcc, but Django stumbles over those cases
# in EmailMessage.recipients (called from EmailMessage.send) before
# Anymail gets a chance to complain.)
self.message.reply_to = "single-reply-to@example.com"
with self.assertRaisesMessage(TypeError, '"reply_to" attribute must be a list or other iterable'):
self.message.send()
def test_explains_reply_to_must_be_list_lazy(self):
"""Same as previous tests, with lazy strings"""
# Lazy strings can fool string/iterable detection
self.message.reply_to = ugettext_lazy("single-reply-to@example.com")
with self.assertRaisesMessage(TypeError, '"reply_to" attribute must be a list or other iterable'):
self.message.send()