#294 - not failing service startup if smtp fails to start

9 files changed, 75 insertions, 29 deletions

diff --git a/service/pixelated/adapter/services/mail_sender.py b/service/pixelated/adapter/services/mail_sender.py
index 24ae839d..9f42fbbc 100644
--- a/service/pixelated/adapter/services/mail_sender.py
+++ b/service/pixelated/adapter/services/mail_sender.py
@@ -16,14 +16,20 @@
 from StringIO import StringIO
 import re
 
-from twisted.internet.defer import Deferred
+from twisted.internet.defer import Deferred, fail
 from twisted.mail.smtp import SMTPSenderFactory
 from twisted.internet import reactor
 from pixelated.support.functional import flatten
 
 
+class SMTPDownException(Exception):
+    def __init__(self):
+        Exception.__init__(self, "Couldn't send mail now, try again later.")
+
+
 class MailSender(object):
-    def __init__(self, account_email_address, smtp_client=None):
+    def __init__(self, account_email_address, ensure_smtp_is_running_cb):
+        self.ensure_smtp_is_running_cb = ensure_smtp_is_running_cb
         self.account_email_address = account_email_address
 
     def recepients_normalizer(self, mail_list):
@@ -40,15 +46,17 @@ class MailSender(object):
         return self.recepients_normalizer(clean_mail_list)
 
     def sendmail(self, mail):
-        recipients = flatten([mail.to, mail.cc, mail.bcc])
-        normalized_recipients = self.get_email_addresses(recipients)
-        resultDeferred = Deferred()
-        senderFactory = SMTPSenderFactory(
-            fromEmail=self.account_email_address,
-            toEmail=normalized_recipients,
-            file=StringIO(mail.to_smtp_format()),
-            deferred=resultDeferred)
-
-        reactor.connectTCP('localhost', 4650, senderFactory)
-
-        return resultDeferred
+        if self.ensure_smtp_is_running_cb():
+            recipients = flatten([mail.to, mail.cc, mail.bcc])
+            normalized_recipients = self.get_email_addresses(recipients)
+            resultDeferred = Deferred()
+            senderFactory = SMTPSenderFactory(
+                fromEmail=self.account_email_address,
+                toEmail=normalized_recipients,
+                file=StringIO(mail.to_smtp_format()),
+                deferred=resultDeferred)
+
+            reactor.connectTCP('localhost', 4650, senderFactory)
+
+            return resultDeferred
+        return fail(SMTPDownException())
diff --git a/service/pixelated/bitmask_libraries/session.py b/service/pixelated/bitmask_libraries/session.py
index 158d6605..9f21fbe6 100644
--- a/service/pixelated/bitmask_libraries/session.py
+++ b/service/pixelated/bitmask_libraries/session.py
@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 import errno
+import logging
 import traceback
 import sys
 
@@ -65,7 +66,7 @@ class LeapSession(object):
     - ``incoming_mail_fetcher`` Background job for fetching incoming mails from LEAP server (LeapIncomingMail)
     """
 
-    def __init__(self, provider, srp_session, soledad_session, nicknym, soledad_account, incoming_mail_fetcher):
+    def __init__(self, provider, srp_session, soledad_session, nicknym, soledad_account, incoming_mail_fetcher, smtp):
         """
         Constructor.
 
@@ -73,6 +74,7 @@ class LeapSession(object):
         :type leap_config: LeapConfig
 
         """
+        self.smtp = smtp
         self.config = provider.config
         self.provider = provider
         self.srp_session = srp_session
@@ -133,9 +135,10 @@ class LeapSessionFactory(object):
                                                                    account, auth)
 
         smtp = LeapSmtp(self._provider, nicknym.keymanager, auth)
-        smtp.start()
 
-        return LeapSession(self._provider, auth, soledad, nicknym, account, incoming_mail_fetcher)
+        smtp.ensure_running()
+
+        return LeapSession(self._provider, auth, soledad, nicknym, account, incoming_mail_fetcher, smtp)
 
     def _lookup_session(self, key):
         global SESSIONS
diff --git a/service/pixelated/bitmask_libraries/smtp.py b/service/pixelated/bitmask_libraries/smtp.py
index 6bbe0051..d5236e8e 100644
--- a/service/pixelated/bitmask_libraries/smtp.py
+++ b/service/pixelated/bitmask_libraries/smtp.py
@@ -13,12 +13,16 @@
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
+import logging
 import os
 import requests
 from .certs import which_bundle
 from leap.mail.smtp import setup_smtp_gateway
 
 
+logger = logging.getLogger(__name__)
+
+
 class LeapSmtp(object):
 
     TWISTED_PORT = 4650
@@ -86,6 +90,15 @@ class LeapSmtp(object):
             encrypted_only=False
         )
 
+    def ensure_running(self):
+        if not self._smtp_service:
+            try:
+                self.start()
+            except Exception as e:
+                logger.warning("Couldn't start the SMTP server now, will try again when the user tries to use it")
+                return False
+        return True
+
     def stop(self):
         if self._smtp_service is not None:
             self._smtp_port.stopListening()
diff --git a/service/pixelated/config/app_factory.py b/service/pixelated/config/app_factory.py
index 8671eaa6..f63b49ed 100644
--- a/service/pixelated/config/app_factory.py
+++ b/service/pixelated/config/app_factory.py
@@ -102,7 +102,8 @@ def init_app(app, leap_home, leap_session):
 
     tag_service = TagService()
     search_engine = SearchEngine(soledad_querier, agent_home=leap_home)
-    pixelated_mail_sender = MailSender(leap_session.account_email())
+    pixelated_mail_sender = MailSender(leap_session.account_email(),
+                                       lambda: leap_session.smtp.ensure_running())
 
     pixelated_mailboxes = Mailboxes(leap_session.account, soledad_querier, search_engine)
     draft_service = DraftService(pixelated_mailboxes)
diff --git a/service/pixelated/resources/__init__.py b/service/pixelated/resources/__init__.py
index a2e4c9d4..b244900a 100644
--- a/service/pixelated/resources/__init__.py
+++ b/service/pixelated/resources/__init__.py
@@ -14,6 +14,8 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
+import json
+
 
 def respond_json(entity, request, status_code=200):
     json_response = json.dumps(entity)
@@ -28,6 +30,3 @@ def respond_json_deferred(entity, request, status_code=200):
     request.code = status_code
     request.write(json_response)
     request.finish()
-
-
-import json
diff --git a/service/pixelated/resources/mails_resource.py b/service/pixelated/resources/mails_resource.py
index f387076b..c057031a 100644
--- a/service/pixelated/resources/mails_resource.py
+++ b/service/pixelated/resources/mails_resource.py
@@ -1,4 +1,5 @@
 import json
+from pixelated.adapter.services.mail_sender import SMTPDownException
 from pixelated.adapter.model.mail import InputMail
 from pixelated.resources import respond_json, respond_json_deferred
 from twisted.web.resource import Resource
@@ -95,7 +96,10 @@ class MailsResource(Resource):
             respond_json_deferred(data, request)
 
         def onError(error):
-            respond_json_deferred({'message': str(error)}, request, status_code=422)
+            if isinstance(error.value, SMTPDownException):
+                respond_json_deferred({'message': str(error.value)}, request, status_code=503)
+            else:
+                respond_json_deferred({'message': str(error)}, request, status_code=422)
 
         deferred.addCallback(onSuccess)
         deferred.addErrback(onError)
diff --git a/service/test/unit/adapter/services/test_mail_sender.py b/service/test/unit/adapter/services/test_mail_sender.py
index 8536e5e3..0e7ec015 100644
--- a/service/test/unit/adapter/services/test_mail_sender.py
+++ b/service/test/unit/adapter/services/test_mail_sender.py
@@ -16,7 +16,7 @@
 from twisted.trial import unittest
 
 from mockito import mock, when, verify, any, unstub
-from pixelated.adapter.services.mail_sender import MailSender
+from pixelated.adapter.services.mail_sender import MailSender, SMTPDownException
 from pixelated.adapter.model.mail import PixelatedMail, InputMail
 from test.support.test_helper import mail_dict
 from twisted.internet import reactor
@@ -24,10 +24,14 @@ from twisted.internet.defer import Deferred
 
 
 class MailSenderTest(unittest.TestCase):
+    def setUp(self):
+        self.ensure_smtp_is_running_cb = lambda: True
+        self.ensure_smtp_is_not_running_cb = lambda: False
+
     def test_sendmail(self):
         when(reactor).connectTCP('localhost', 4650, any()).thenReturn(None)
         input_mail = InputMail.from_dict(mail_dict())
-        mail_sender = MailSender('someone@somedomain.tld')
+        mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_running_cb)
 
         return self._succeed(mail_sender.sendmail(input_mail))
 
@@ -38,7 +42,8 @@ class MailSenderTest(unittest.TestCase):
         when(reactor).connectTCP('localhost', 4650, any()).thenReturn(None)
 
         input_mail = InputMail.from_dict(mail_dict())
-        mail_sender = MailSender('someone@somedomain.tld')
+
+        mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_running_cb)
 
         sent_deferred = mail_sender.sendmail(input_mail)
 
@@ -49,7 +54,7 @@ class MailSenderTest(unittest.TestCase):
     def test_senmail_returns_deffered(self):
         when(reactor).connectTCP('localhost', 4650, any()).thenReturn(None)
         input_mail = InputMail.from_dict(mail_dict())
-        mail_sender = MailSender('someone@somedomain.tld')
+        mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_running_cb)
 
         deferred = mail_sender.sendmail(input_mail)
 
@@ -58,6 +63,18 @@ class MailSenderTest(unittest.TestCase):
 
         return self._succeed(deferred)
 
+    def test_doesnt_send_mail_if_smtp_is_not_running(self):
+        mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_not_running_cb)
+
+        deferred = mail_sender.sendmail({})
+
+        def _assert(_):
+            self.assertTrue(isinstance(deferred.result.value, SMTPDownException))
+
+        deferred.addErrback(_assert)
+
+        return deferred
+
     def _succeed(self, deferred):
         deferred.callback(None)
         return deferred
diff --git a/service/test/unit/bitmask_libraries/test_session.py b/service/test/unit/bitmask_libraries/test_session.py
index 67722557..0bfd59f2 100644
--- a/service/test/unit/bitmask_libraries/test_session.py
+++ b/service/test/unit/bitmask_libraries/test_session.py
@@ -24,6 +24,7 @@ class SessionTest(AbstractLeapTest):
 
     def setUp(self):
         self.mail_fetcher_mock = MagicMock()
+        self.smtp_mock = MagicMock()
 
     def tearDown(self):
         self.mail_fetcher_mock = MagicMock()
@@ -65,7 +66,7 @@ class SessionTest(AbstractLeapTest):
 
     def _create_session(self):
         return LeapSession(self.provider, self.srp_session, self.soledad_session, self.nicknym, self.soledad_account,
-                           self.mail_fetcher_mock)
+                           self.mail_fetcher_mock, self.smtp_mock)
 
 
 def _execute_func(func):
diff --git a/service/test/unit/bitmask_libraries/test_smtp.py b/service/test/unit/bitmask_libraries/test_smtp.py
index 4087cbf5..c0e30573 100644
--- a/service/test/unit/bitmask_libraries/test_smtp.py
+++ b/service/test/unit/bitmask_libraries/test_smtp.py
@@ -71,7 +71,7 @@ class LeapSmtpTest(AbstractLeapTest):
         smtp.TWISTED_PORT = port
         gateway_mock.return_value = (None, None)
         with HTTMock(ca_cert_mock, not_found_mock):
-            smtp.start()
+            smtp.ensure_running()
 
         cert_path = self._client_cert_path()
         gateway_mock.assert_called_with(keymanager=self.keymanager, smtp_cert=cert_path, smtp_key=cert_path, userid='test_user@some-server.test', smtp_port='1234', encrypted_only=False, smtp_host='smtp.some-sever.test', port=port)
@@ -91,7 +91,7 @@ class LeapSmtpTest(AbstractLeapTest):
         gateway_mock.return_value = (smtp_service, smtp_port)
 
         with HTTMock(ca_cert_mock, not_found_mock):
-            smtp.start()
+            smtp.ensure_running()
             smtp.stop()
 
         smtp_port.stopListening.assert_called_with()