Repository: awdeorio/mailmerge
Pull request #73 (2019-12-19 18:33:28)

Unified exceptions
Single, top-level exception for entire program.


Patch:
diff --git a/mailmerge/__init__.py b/mailmerge/__init__.py
index 062b59e..dc48419 100644
--- a/mailmerge/__init__.py
+++ b/mailmerge/__init__.py
@@ -3,3 +3,8 @@ Mail merge module importable API.
 
 Andrew DeOrio <awdeorio@umich.edu>
 """
+
+
+from .sendmail_client import SendmailClient
+from .template_message import TemplateMessage
+from .exceptions import MailmergeError
diff --git a/mailmerge/__main__.py b/mailmerge/__main__.py
index b88bae2..c17bcbe 100644
--- a/mailmerge/__main__.py
+++ b/mailmerge/__main__.py
@@ -5,15 +5,11 @@ Andrew DeOrio <awdeorio@umich.edu>
 """
 from __future__ import print_function
 import sys
-import socket
-import configparser
-import smtplib
 import textwrap
-import jinja2
 import click
 from .template_message import TemplateMessage
 from .sendmail_client import SendmailClient
-from .utils import MailmergeError
+from .exceptions import MailmergeError
 
 # Python 2 pathlib support requires backport
 try:
@@ -101,21 +97,6 @@ def main(sample, dry_run, limit, no_limit,
             for filename in get_attachment_filenames(message):
                 print(">>> attached {}".format(filename))
             print(">>> sent message {}".format(i + 1))
-    except jinja2.exceptions.TemplateError as err:
-        sys.exit(">>> Error in Jinja2 template: {}".format(err))
-    except csv.Error as err:
-        sys.exit(">>> Error reading CSV file: {}".format(err))
-    except smtplib.SMTPAuthenticationError as err:
-        sys.exit(">>> Authentication error: {}".format(err))
-    except configparser.Error as err:
-        sys.exit(
-            ">>> Error reading config file {filename}: {message}"
-            .format(filename=config_path, message=err)
-        )
-    except smtplib.SMTPException as err:
-        sys.exit(">>> Error sending message", err, sep=' ', file=sys.stderr)
-    except socket.error:
-        sys.exit(">>> Error connecting to server")
     except MailmergeError as err:
         sys.exit(">>> {}".format(err))
 
@@ -249,8 +230,13 @@ def read_csv_database(database_path):
 
     with database_path.open("r") as database_file:
         reader = csv.DictReader(database_file, dialect=StrictExcel)
-        for row in reader:
-            yield row
+        try:
+            for row in reader:
+                yield row
+        except csv.Error as err:
+            raise MailmergeError(
+                "{}:{}: {}".format(database_path, reader.line_num, err)
+            )
 
 
 def enumerate_limit(iterable, limit):
diff --git a/mailmerge/exceptions.py b/mailmerge/exceptions.py
new file mode 100644
index 0000000..7ba05b8
--- /dev/null
+++ b/mailmerge/exceptions.py
@@ -0,0 +1,5 @@
+"""Errors raised by mailmerge."""
+
+
+class MailmergeError(Exception):
+    """Top level exception raised by mailmerge functions."""
diff --git a/mailmerge/sendmail_client.py b/mailmerge/sendmail_client.py
index 31dedeb..e91385d 100644
--- a/mailmerge/sendmail_client.py
+++ b/mailmerge/sendmail_client.py
@@ -3,9 +3,11 @@ SMTP client reads configuration and sends message.
 
 Andrew DeOrio <awdeorio@umich.edu>
 """
+import socket
 import smtplib
 import configparser
 import getpass
+from .exceptions import MailmergeError
 
 
 class SendmailClient(object):
@@ -18,28 +20,33 @@ class SendmailClient(object):
     # https://python-future.org/compatible_idioms.html#custom-class-behaviour
     # pylint: disable=bad-option-value,useless-object-inheritance
 
-    def __init__(self, config_filename, dry_run=False):
-        """Read configuration from config_filename."""
-        config = configparser.RawConfigParser()
-        config.read(str(config_filename))  # str need for older Python versions
+    def __init__(self, config_path, dry_run=False):
+        """Read configuration from server configuration file."""
+        self.config_path = config_path
         self.dry_run = dry_run
-        self.host = config.get("smtp_server", "host")
-        self.port = config.getint("smtp_server", "port")
-        self.security = config.get("smtp_server", "security", fallback=None)
+        self.username = None
         self.password = None
-
-        # Coerce legacy option "security = Never"
-        if self.security == "Never":
-            self.security = None
-
-        # Read username only if needed
-        if self.security is not None:
-            self.username = config.get("smtp_server", "username")
+        try:
+            config = configparser.RawConfigParser()
+            config.read(str(config_path))
+            self.host = config.get("smtp_server", "host")
+            self.port = config.getint("smtp_server", "port")
+            self.security = config.get("smtp_server", "security",
+                                       fallback=None)
+            if self.security == "Never":
+                # Coerce legacy option "security = Never"
+                self.security = None
+            if self.security is not None:
+                # Read username only if needed
+                self.username = config.get("smtp_server", "username")
+        except configparser.Error as err:
+            raise MailmergeError("{}: {}".format(self.config_path, err))
 
         # Verify security type
         if self.security not in [None, "SSL/TLS", "STARTTLS"]:
-            raise configparser.Error(
-                "Unrecognized security type: {}".format(self.security)
+            raise MailmergeError(
+                "{}: unrecognized security type: '{}'"
+                .format(self.config_path, self.security)
             )
 
     def sendmail(self, sender, recipients, message):
@@ -59,17 +66,33 @@ class SendmailClient(object):
             self.password = getpass.getpass(prompt)
 
         # Send
-        if self.security == "SSL/TLS":
-            with smtplib.SMTP_SSL(self.host, self.port) as smtp:
-                smtp.login(self.username, self.password)
-                smtp.sendmail(sender, recipients, message.as_string())
-        elif self.security == "STARTTLS":
-            with smtplib.SMTP(self.host, self.port) as smtp:
-                smtp.ehlo()
-                smtp.starttls()
-                smtp.ehlo()
-                smtp.login(self.username, self.password)
-                smtp.sendmail(sender, recipients, message.as_string())
-        elif self.security is None:
-            with smtplib.SMTP(self.host, self.port) as smtp:
-                smtp.sendmail(sender, recipients, message.as_string())
+        try:
+            if self.security == "SSL/TLS":
+                with smtplib.SMTP_SSL(self.host, self.port) as smtp:
+                    smtp.login(self.username, self.password)
+                    smtp.sendmail(sender, recipients, message.as_string())
+            elif self.security == "STARTTLS":
+                with smtplib.SMTP(self.host, self.port) as smtp:
+                    smtp.ehlo()
+                    smtp.starttls()
+                    smtp.ehlo()
+                    smtp.login(self.username, self.password)
+                    smtp.sendmail(sender, recipients, message.as_string())
+            elif self.security is None:
+                with smtplib.SMTP(self.host, self.port) as smtp:
+                    smtp.sendmail(sender, recipients, message.as_string())
+        except smtplib.SMTPAuthenticationError as err:
+            raise MailmergeError(
+                "{}:{} failed to authenticate user '{}': {}"
+                .format(self.host, self.port, self.username, err)
+            )
+        except smtplib.SMTPException as err:
+            raise MailmergeError(
+                "{}:{} failed to send message: {}"
+                .format(self.host, self.port, err)
+            )
+        except socket.error as err:
+            raise MailmergeError(
+                "{}:{} failed to connect to server: {}"
+                .format(self.host, self.port, err)
+            )
diff --git a/mailmerge/template_message.py b/mailmerge/template_message.py
index edee1c5..58500f2 100644
--- a/mailmerge/template_message.py
+++ b/mailmerge/template_message.py
@@ -14,7 +14,7 @@ import future.backports.email.utils
 import future.backports.email.generator
 import markdown
 import jinja2
-from . import utils
+from .exceptions import MailmergeError
 
 # Python 2 pathlib support requires backport
 try:
@@ -59,7 +59,12 @@ class TemplateMessage(object):
 
     def render(self, context):
         """Return rendered message object."""
-        raw_message = self.template.render(context)
+        try:
+            raw_message = self.template.render(context)
+        except jinja2.exceptions.TemplateError as err:
+            raise MailmergeError(
+                "{}: {}".format(self.template_path, err)
+            )
         self._message = email.message_from_string(raw_message)
         self._transform_encoding(raw_message)
         self._transform_recipients()
@@ -186,7 +191,7 @@ class TemplateMessage(object):
         """Find attachment file or raise MailmergeError."""
         # Error on empty path
         if not path.strip():
-            raise utils.MailmergeError("Empty attachment header.")
+            raise MailmergeError("Empty attachment header.")
 
         # Create a Path object and handle home directory (tilde ~) notation
         path = Path(path.strip())
@@ -201,7 +206,7 @@ class TemplateMessage(object):
 
         # Check that the attachment exists
         if not path.exists():
-            raise utils.MailmergeError("Attachment not found: {}".format(path))
+            raise MailmergeError("Attachment not found: {}".format(path))
 
         return path
 
diff --git a/mailmerge/utils.py b/mailmerge/utils.py
deleted file mode 100644
index 5a16fbe..0000000
--- a/mailmerge/utils.py
+++ /dev/null
@@ -1,9 +0,0 @@
-"""
-Utilities used in multiple mailmerge modules.
-
-Andrew DeOrio <awdeorio@umich.edu>
-"""
-
-
-class MailmergeError(Exception):
-    """Top-level exception raised by mailmerge functions."""


Test patch:
diff --git a/tests/test_helpers.py b/tests/test_helpers.py
index 18fa79c..21e6e7b 100644
--- a/tests/test_helpers.py
+++ b/tests/test_helpers.py
@@ -8,6 +8,7 @@ Andrew DeOrio <awdeorio@umich.edu>
 import textwrap
 import pytest
 import mailmerge.__main__
+from mailmerge import MailmergeError
 
 # Python 2 pathlib support requires backport
 try:
@@ -15,12 +16,6 @@ try:
 except ImportError:
     from pathlib import Path
 
-# Python 2 UTF8 support requires csv backport
-try:
-    from backports import csv
-except ImportError:
-    import csv
-
 
 def test_enumerate_limit_no_limit():
     """Verify limit=-1 results in no early termination."""
@@ -54,16 +49,13 @@ def test_enumerate_limit_zero():
 
 
 def test_csv_bad(tmpdir):
-    """Bad CSV includes includes filename and line number."""
-    # CSV with unmatched quote
+    """CSV with unmatched quote."""
     database_path = Path(tmpdir/"database.csv")
     database_path.write_text(textwrap.dedent(u"""\
         a,b
         1,"2
     """))
-
-    # The first line of data triggers an error
-    with pytest.raises(csv.Error):
+    with pytest.raises(MailmergeError):
         next(mailmerge.__main__.read_csv_database(database_path))
 
 
diff --git a/tests/test_main.py b/tests/test_main.py
index 2af83a8..8451d82 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -396,8 +396,7 @@ def test_bad_template(tmpdir):
     stdout = output.stdout.decode("utf-8")
     stderr = output.stderr.decode("utf-8")
     assert stdout == ""
-    assert "Error in Jinja2 template" in stderr
-    assert "error_not_in_database" in stderr
+    assert "template.txt: 'error_not_in_database' is undefined" in stderr
 
 
 def test_bad_database(tmpdir):
@@ -438,7 +437,7 @@ def test_bad_database(tmpdir):
     stdout = output.stdout.decode("utf-8")
     stderr = output.stderr.decode("utf-8")
     assert stdout == ""
-    assert "Error reading CSV file: unexpected end of data" in stderr
+    assert "database.csv:1: unexpected end of data" in stderr
 
 
 def test_bad_config(tmpdir):
@@ -446,18 +445,15 @@ def test_bad_config(tmpdir):
     # Normal template
     template_path = Path(tmpdir/"template.txt")
     template_path.write_text(textwrap.dedent(u"""\
-        TO: {{email}}
-        SUBJECT: Testing mailmerge
+        TO: to@test.com
         FROM: from@test.com
-
-        Hi {{name}},
     """))
 
     # Normal database
     database_path = Path(tmpdir/"database.csv")
     database_path.write_text(textwrap.dedent(u"""\
-        email,name
-        to@test.com,Bob
+        dummy
+        asdf
     """))
 
     # Server config is missing host
@@ -479,7 +475,7 @@ def test_bad_config(tmpdir):
     stdout = output.stdout.decode("utf-8")
     stderr = output.stderr.decode("utf-8")
     assert stdout == ""
-    assert "Error reading config file" in stderr
+    assert "server.conf: No option 'host' in section: 'smtp_server'" in stderr
 
 
 def test_attachment(tmpdir):
diff --git a/tests/test_sendmail_client.py b/tests/test_sendmail_client.py
index 059a30b..3495970 100644
--- a/tests/test_sendmail_client.py
+++ b/tests/test_sendmail_client.py
@@ -4,12 +4,12 @@ Tests for SendmailClient.
 Andrew DeOrio <awdeorio@umich.edu>
 """
 import textwrap
-import configparser
+import socket
 import smtplib
 import pytest
 import future.backports.email as email
 import future.backports.email.parser  # pylint: disable=unused-import
-from mailmerge.sendmail_client import SendmailClient
+from mailmerge import SendmailClient, MailmergeError
 
 try:
     from unittest import mock  # Python 3
@@ -132,7 +132,7 @@ def test_bad_config_key(tmp_path):
         [smtp_server]
         badkey = open-smtp.example.com
     """))
-    with pytest.raises(configparser.Error):
+    with pytest.raises(MailmergeError):
         SendmailClient(config_path, dry_run=True)
 
 
@@ -146,7 +146,7 @@ def test_security_error(tmp_path):
         security = bad_value
         username = YOUR_USERNAME_HERE
     """))
-    with pytest.raises(configparser.Error):
+    with pytest.raises(MailmergeError):
         SendmailClient(config_path, dry_run=False)
 
 
@@ -292,7 +292,7 @@ def test_security_ssl(mock_getpass, mock_SMTP_SSL, mock_SMTP, tmp_path):
 
 
 def test_missing_username(tmp_path):
-    """Verify open (Never) security configuration."""
+    """Verify exception on missing username."""
     config_path = tmp_path/"server.conf"
     config_path.write_text(textwrap.dedent(u"""\
         [smtp_server]
@@ -300,22 +300,22 @@ def test_missing_username(tmp_path):
         port = 465
         security = SSL/TLS
     """))
-    with pytest.raises(configparser.Error):
+    with pytest.raises(MailmergeError):
         SendmailClient(config_path, dry_run=False)
 
 
 @mock.patch('smtplib.SMTP_SSL')
 @mock.patch('getpass.getpass')
-def test_smtp_error(mock_getpass, mock_SMTP_SSL, tmp_path):
-    """Connection is closed on error."""
+def test_smtp_login_error(mock_getpass, mock_SMTP_SSL, tmp_path):
+    """Login failure."""
     # Config for SSL SMTP server
     config_path = tmp_path/"server.conf"
     config_path.write_text(textwrap.dedent(u"""\
         [smtp_server]
-        host = smtp.mail.umich.edu
+        host = smtp.gmail.com
         port = 465
         security = SSL/TLS
-        username = YOUR_USERNAME_HERE
+        username = awdeorio
     """))
 
     # Simple template
@@ -325,18 +325,108 @@ def test_smtp_error(mock_getpass, mock_SMTP_SSL, tmp_path):
     # Mock the password entry
     mock_getpass.return_value = "password"
 
-    # Configure SMTP_SSL to raise an exception
+    # Configure SMTP login() to raise an exception
     mock_SMTP_SSL.return_value.__enter__.return_value.login = mock.Mock(
         side_effect=smtplib.SMTPAuthenticationError(
-            code=534,
-            msg="Error from mock",
+            code=535,
+            msg=(
+                "5.7.8 Username and Password not accepted. Learn more at "
+                "5.7.8  https://support.google.com/mail/?p=BadCredentials "
+                "xyzxyz.32 - gsmtp"
+            )
+        )
+    )
+
+    # Send a message
+    with pytest.raises(MailmergeError) as err:
+        sendmail_client.sendmail(
+            sender="test@test.com",
+            recipients=["test@test.com"],
+            message=message,
+        )
+
+    # Verify exception string
+    assert "smtp.gmail.com:465 failed to authenticate user 'awdeorio'" in\
+        str(err.value)
+    assert "535" in str(err.value)
+    assert (
+        "5.7.8 Username and Password not accepted. Learn more at "
+        "5.7.8  https://support.google.com/mail/?p=BadCredentials "
+        "xyzxyz.32 - gsmtp"
+    ) in str(err.value)
+
+
+@mock.patch('smtplib.SMTP_SSL')
+@mock.patch('getpass.getpass')
+def test_smtp_sendmail_error(mock_getpass, mock_SMTP_SSL, tmp_path):
+    """Failure during SMTP protocol."""
+    # Config for SSL SMTP server
+    config_path = tmp_path/"server.conf"
+    config_path.write_text(textwrap.dedent(u"""\
+        [smtp_server]
+        host = smtp.gmail.com
+        port = 465
+        security = SSL/TLS
+        username = awdeorio
+    """))
+
+    # Simple template
+    sendmail_client = SendmailClient(config_path, dry_run=False)
+    message = email.message_from_string(u"Hello world")
+
+    # Mock the password entry
+    mock_getpass.return_value = "password"
+
+    # Configure SMTP sendmail() to raise an exception
+    mock_SMTP_SSL.return_value.__enter__.return_value.sendmail = mock.Mock(
+        side_effect=smtplib.SMTPException("Dummy error message")
+    )
+
+    # Send a message
+    with pytest.raises(MailmergeError) as err:
+        sendmail_client.sendmail(
+            sender="test@test.com",
+            recipients=["test@test.com"],
+            message=message,
         )
+
+    # Verify exception string
+    assert "Dummy error message" in str(err.value)
+
+
+@mock.patch('smtplib.SMTP_SSL')
+@mock.patch('getpass.getpass')
+def test_socket_error(mock_getpass, mock_SMTP_SSL, tmp_path):
+    """Failed socket connection."""
+    # Config for SSL SMTP server
+    config_path = tmp_path/"server.conf"
+    config_path.write_text(textwrap.dedent(u"""\
+        [smtp_server]
+        host = smtp.gmail.com
+        port = 465
+        security = SSL/TLS
+        username = awdeorio
+    """))
+
+    # Simple template
+    sendmail_client = SendmailClient(config_path, dry_run=False)
+    message = email.message_from_string(u"Hello world")
+
+    # Mock the password entry
+    mock_getpass.return_value = "password"
+
+    # Configure SMTP_SSL constructor to raise an exception
+    mock_SMTP_SSL.return_value.__enter__ = mock.Mock(
+        side_effect=socket.error("Dummy error message")
     )
 
     # Send a message
-    with pytest.raises(smtplib.SMTPAuthenticationError):
+    with pytest.raises(MailmergeError) as err:
         sendmail_client.sendmail(
             sender="test@test.com",
             recipients=["test@test.com"],
             message=message,
         )
+
+    # Verify exception string
+    assert "Dummy error message" in str(err.value)
diff --git a/tests/test_template_message.py b/tests/test_template_message.py
index bd696fe..9a70f51 100644
--- a/tests/test_template_message.py
+++ b/tests/test_template_message.py
@@ -10,11 +10,9 @@ import re
 import shutil
 import textwrap
 import collections
-import jinja2
 import pytest
 import markdown
-from mailmerge.template_message import TemplateMessage
-from mailmerge.utils import MailmergeError
+from mailmerge import TemplateMessage, MailmergeError
 from . import utils
 
 # Python 2 pathlib support requires backport
@@ -91,7 +89,7 @@ def test_bad_jinja(tmp_path):
     template_path = tmp_path / "template.txt"
     template_path.write_text(u"TO: {{error_not_in_database}}")
     template_message = TemplateMessage(template_path)
-    with pytest.raises(jinja2.exceptions.UndefinedError):
+    with pytest.raises(MailmergeError):
         template_message.render({"name": "Bob", "number": 17})