From 09bec9e4638857242133a3567662ff8372cd8918 Mon Sep 17 00:00:00 2001
From: medmunds <medmunds@gmail.com>
Date: Tue, 4 Apr 2017 11:57:11 -0700
Subject: [PATCH] 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
---
 anymail/backends/base.py      | 25 ++++++++++++++++++++++++-
 tests/test_general_backend.py | 23 +++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/anymail/backends/base.py b/anymail/backends/base.py
index e05bb0c..f6c1015 100644
--- a/anymail/backends/base.py
+++ b/anymail/backends/base.py
@@ -1,5 +1,6 @@
 from datetime import date, datetime
 
+import six
 from django.conf import settings
 from django.core.mail.backends.base import BaseEmailBackend
 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 ..signals import pre_send, post_send
 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):
@@ -252,6 +253,8 @@ class BasePayload(object):
         message_attrs = self.base_message_attrs + self.anymail_message_attrs + self.esp_message_attrs
         for attr, combiner, converter in message_attrs:
             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:
                 default_value = self.defaults.get(attr, UNSET)
                 value = combiner(default_value, value)
@@ -273,6 +276,26 @@ class BasePayload(object):
             raise AnymailUnsupportedFeature("%s does not support %s" % (self.esp_name, feature),
                                             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
     #
diff --git a/tests/test_general_backend.py b/tests/test_general_backend.py
index 5515891..5901392 100644
--- a/tests/test_general_backend.py
+++ b/tests/test_general_backend.py
@@ -293,3 +293,26 @@ class LazyStringsTest(TestBackendTestCase):
         params = self.get_send_params()
         self.assertNotLazy(params['merge_data']['to@example.com']['duration'])
         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()
+