Improve error handling when sending emails (#595)

In one case, we were not catching a family of possible exceptions
(socket.error), and in the two other cases there was no error handling at
all. Sending emails can easily fail if no email server is configured, so
it is really necessary to handle these errors instead of crashing with a
HTTP 500 error.

Refactor email sending code and add proper error handling.

Show alert messages that tell the user if an email was sent or if there
was an error.

When sending a password reminder email or inviting people by email, we
don't proceed to the next step in case of error, because sending emails is
the whole point of these actions.

1 files changed, 113 insertions, 16 deletions

diff --git a/ihatemoney/tests/tests.py b/ihatemoney/tests/tests.py
index 4cdf316..dc6c455 100644
--- a/ihatemoney/tests/tests.py
+++ b/ihatemoney/tests/tests.py
@@ -5,6 +5,8 @@ import io
 import json
 import os
 import re
+import smtplib
+import socket
 from time import sleep
 import unittest
 from unittest.mock import MagicMock, patch
@@ -51,10 +53,10 @@ class BaseTestCase(TestCase):
             follow_redirects=True,
         )
 
-    def post_project(self, name):
+    def post_project(self, name, follow_redirects=True):
         """Create a fake project"""
         # create the project
-        self.client.post(
+        return self.client.post(
             "/create",
             data={
                 "name": name,
@@ -63,6 +65,7 @@ class BaseTestCase(TestCase):
                 "contact_email": f"{name}@notmyidea.org",
                 "default_currency": "USD",
             },
+            follow_redirects=follow_redirects,
         )
 
     def create_project(self, name):
@@ -141,10 +144,15 @@ class BudgetTestCase(IhatemoneyTestCase):
             self.login("raclette")
 
             self.post_project("raclette")
-            self.client.post(
-                "/raclette/invite", data={"emails": "zorglub@notmyidea.org"}
+            resp = self.client.post(
+                "/raclette/invite",
+                data={"emails": "zorglub@notmyidea.org"},
+                follow_redirects=True,
             )
 
+            # success notification
+            self.assertIn("Your invitations have been sent", resp.data.decode("utf-8"))
+
             self.assertEqual(len(outbox), 2)
             self.assertEqual(outbox[0].recipients, ["raclette@notmyidea.org"])
             self.assertEqual(outbox[1].recipients, ["zorglub@notmyidea.org"])
@@ -225,7 +233,15 @@ class BudgetTestCase(IhatemoneyTestCase):
         self.create_project("raclette")
         # Get password resetting link from mail
         with self.app.mail.record_messages() as outbox:
-            self.client.post("/password-reminder", data={"id": "raclette"})
+            resp = self.client.post(
+                "/password-reminder", data={"id": "raclette"}, follow_redirects=True
+            )
+            # Check that we are redirected to the right page
+            self.assertIn(
+                "A link to reset your password has been sent to you",
+                resp.data.decode("utf-8"),
+            )
+            # Check that an email was sent
             self.assertEqual(len(outbox), 1)
             url_start = outbox[0].body.find("You can reset it here: ") + 23
             url_end = outbox[0].body.find(".\n", url_start)
@@ -250,17 +266,26 @@ class BudgetTestCase(IhatemoneyTestCase):
     def test_project_creation(self):
         with self.app.test_client() as c:
 
-            # add a valid project
-            c.post(
-                "/create",
-                data={
-                    "name": "The fabulous raclette party",
-                    "id": "raclette",
-                    "password": "party",
-                    "contact_email": "raclette@notmyidea.org",
-                    "default_currency": "USD",
-                },
-            )
+            with self.app.mail.record_messages() as outbox:
+                # add a valid project
+                resp = c.post(
+                    "/create",
+                    data={
+                        "name": "The fabulous raclette party",
+                        "id": "raclette",
+                        "password": "party",
+                        "contact_email": "raclette@notmyidea.org",
+                        "default_currency": "USD",
+                    },
+                    follow_redirects=True,
+                )
+                # an email is sent to the owner with a reminder of the password
+                self.assertEqual(len(outbox), 1)
+                self.assertEqual(outbox[0].recipients, ["raclette@notmyidea.org"])
+                self.assertIn(
+                    "A reminder email has just been sent to you",
+                    resp.data.decode("utf-8"),
+                )
 
             # session is updated
             self.assertTrue(session["raclette"])
@@ -2265,6 +2290,78 @@ class ModelsTestCase(IhatemoneyTestCase):
                 self.assertEqual(bill.pay_each(), pay_each_expected)
 
 
+class EmailFailureTestCase(IhatemoneyTestCase):
+    def test_creation_email_failure_smtp(self):
+        self.login("raclette")
+        with patch.object(
+            self.app.mail, "send", MagicMock(side_effect=smtplib.SMTPException)
+        ):
+            resp = self.post_project("raclette")
+        # Check that an error message is displayed
+        self.assertIn(
+            "We tried to send you an reminder email, but there was an error",
+            resp.data.decode("utf-8"),
+        )
+        # Check that we were redirected to the home page anyway
+        self.assertIn(
+            'You probably want to <a href="/raclette/members/add"',
+            resp.data.decode("utf-8"),
+        )
+
+    def test_creation_email_failure_socket(self):
+        self.login("raclette")
+        with patch.object(self.app.mail, "send", MagicMock(side_effect=socket.error)):
+            resp = self.post_project("raclette")
+        # Check that an error message is displayed
+        self.assertIn(
+            "We tried to send you an reminder email, but there was an error",
+            resp.data.decode("utf-8"),
+        )
+        # Check that we were redirected to the home page anyway
+        self.assertIn(
+            'You probably want to <a href="/raclette/members/add"',
+            resp.data.decode("utf-8"),
+        )
+
+    def test_password_reset_email_failure(self):
+        self.create_project("raclette")
+        for exception in (smtplib.SMTPException, socket.error):
+            with patch.object(self.app.mail, "send", MagicMock(side_effect=exception)):
+                resp = self.client.post(
+                    "/password-reminder", data={"id": "raclette"}, follow_redirects=True
+                )
+            # Check that an error message is displayed
+            self.assertIn(
+                "there was an error while sending you an email",
+                resp.data.decode("utf-8"),
+            )
+            # Check that we were not redirected to the success page
+            self.assertNotIn(
+                "A link to reset your password has been sent to you",
+                resp.data.decode("utf-8"),
+            )
+
+    def test_invitation_email_failure(self):
+        self.login("raclette")
+        self.post_project("raclette")
+        for exception in (smtplib.SMTPException, socket.error):
+            with patch.object(self.app.mail, "send", MagicMock(side_effect=exception)):
+                resp = self.client.post(
+                    "/raclette/invite",
+                    data={"emails": "toto@notmyidea.org"},
+                    follow_redirects=True,
+                )
+            # Check that an error message is displayed
+            self.assertIn(
+                "there was an error while trying to send the invitation emails",
+                resp.data.decode("utf-8"),
+            )
+            # Check that we are still on the same page (no redirection)
+            self.assertIn(
+                "Invite people to join this project", resp.data.decode("utf-8"),
+            )
+
+
 def em_surround(string, regex_escape=False):
     if regex_escape:
         return r'<em class="font-italic">%s<\/em>' % string