Repository: aio-libs/aiosmtpd (python)

Issue:
Controller.factory() exceptions not caught
(Got bitten by this when mangling things up during unittest-to-pytest conversion)

As currently implemented, `Controller._run()` catches exceptions raised by `loop.create_server()`. The problem is that `create_server` is "lazy": It invokes the `factory()` method only when (1) a connection is made, _and_ (2) some data has been exchanged. Due to this "lazy"-ness, `create_server` only generates an Exception if it cannot bind to the `host:port` as specified.

As a result, if the call to `factory()` somehow caused an Exception, that Exception will go under the radar. And since `SMTP` hasn't been instantiated, `handle_exception` in the handler object won't ever be called. From client side, one ends up only with `smtplib.SMTPServerDisconnected: Connection unexpectedly closed` error, with no hints as to the root cause, potentially misleading users.

(I personally spent quite some time puzzling out why SMTPClient failed to connect, before finding out that I made a mistake in the instantiation of SMTP.)

Since users of this library has likely followed the example of subclassing `Controller` and overriding `factory()` with a simple `SMTP()` instantiation, I think the only non-disruptive backward-compatible way to implement a mechanism to catch `factory()` errors is to wrap calls to `factory()` using another (private) method whose sole purpose is to watch for success/failure of `factory()` and act accordingly.

I'm preparing a PR that will implement this mechanism.

**Edit:** The code is currently un-PR-ed in [pepoluan/aiosmtpd:catch_factory](https://github.com/pepoluan/aiosmtpd/tree/catch_factory). Am in process of makening ensuration* it passes all tests on the 7 environments I have.

\* Totally madened up words  (😁)

Pull request:
Catch exceptions raised by Controller.factory()
## What do these changes do?

Implement mechanism to catch Exceptions raised by `factory()`.

## Are there changes in behavior for the user?

User must be aware that it is now possible for `Controller.start()` to raise Exception caused by _other_ than host:port conflict

## Related issue number

Closes #212 

## Checklist

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] tox testenvs have been executed in the following environments:
  - [x] Windows 10 (via PyCharm): `(ALL)`
  - [x] Windows 10 (via PSCore 7.0.3): `(ALL)`
  - [x] WIndows 10 (via Cygwin): `qa,py36-{nocov,cov}`
  - [x] FreeBSD 12.1 on VBox: `(ALL)` + `pypy3-nocov`
  - [x] Ubuntu 18.04 on VBox: `(ALL)` + `pypy3-{nocov,cov,diffcov}`
  - [x] Ubuntu 18.04 on WSL 1.0: `(ALL)` + `pypy3-{nocov,cov,diffcov}`
  - [x] Ubuntu 20.04 on VBox: `(ALL)` + `pypy3-{nocov,cov,diffcov}`
- [x] Documentation reflects the changes
- [x] Add a news fragment into the `NEWS.rst` file


Patch:
diff --git a/.coveragerc b/.coveragerc
index 0ed6f61..1c2cd01 100644
--- a/.coveragerc
+++ b/.coveragerc
@@ -19,3 +19,4 @@ exclude_lines =
     pragma: nocover
     pragma: no${PLATFORM}
     pragma: skipif_${PY_39}_py39
+    pragma: skipif_${PY_36}_py36
diff --git a/aiosmtpd/controller.py b/aiosmtpd/controller.py
index 90c080f..9fa2cf5 100644
--- a/aiosmtpd/controller.py
+++ b/aiosmtpd/controller.py
@@ -1,48 +1,109 @@
 import os
+import ssl
 import asyncio
 import threading
 
 from aiosmtpd.smtp import SMTP
+from contextlib import ExitStack
 from public import public
-from typing import Any, Dict
+from socket import create_connection
+from typing import Any, Coroutine, Dict, Optional
+
+
+AsyncServer = asyncio.base_events.Server
+
+
+class _FakeServer(asyncio.StreamReaderProtocol):
+    """
+    Returned by _factory_invoker() in lieu of an SMTP instance in case
+    factory() failed to instantiate an SMTP instance.
+    """
+
+    def __init__(self, loop):
+        # Imitate what SMTP does
+        super().__init__(
+            asyncio.StreamReader(loop=loop),
+            client_connected_cb=self._client_connected_cb,
+            loop=loop,
+        )
+
+    def _client_connected_cb(self, reader, writer):
+        pass
 
 
 @public
 class Controller:
-    def __init__(self, handler, loop=None, hostname=None, port=8025, *,
-                 ready_timeout=1.0, enable_SMTPUTF8=True, ssl_context=None,
-                 server_kwargs: Dict[str, Any] = None):
+    server: Optional[AsyncServer] = None
+    server_coro: Coroutine = None
+    smtpd = None
+    _thread: Optional[threading.Thread] = None
+    _thread_exception: Optional[Exception] = None
+
+    def __init__(
+        self,
+        handler,
+        loop=None,
+        hostname=None,
+        port=8025,
+        *,
+        ready_timeout=1.0,
+        enable_SMTPUTF8=True,
+        ssl_context: ssl.SSLContext = None,
+        server_kwargs: Dict[str, Any] = None
+    ):
         """
         `Documentation can be found here
         <http://aiosmtpd.readthedocs.io/en/latest/aiosmtpd\
 /docs/controller.html#controller-api>`_.
         """
         self.handler = handler
-        self.hostname = '::1' if hostname is None else hostname
+        self.hostname = "::1" if hostname is None else hostname
         self.port = port
         self.enable_SMTPUTF8 = enable_SMTPUTF8
         self.ssl_context = ssl_context
         self.loop = asyncio.new_event_loop() if loop is None else loop
-        self.server = None
-        self._thread = None
-        self._thread_exception = None
-        self.ready_timeout = os.getenv(
-            'AIOSMTPD_CONTROLLER_TIMEOUT', ready_timeout)
+        self.ready_timeout = os.getenv("AIOSMTPD_CONTROLLER_TIMEOUT", ready_timeout)
         self.server_kwargs: Dict[str, Any] = server_kwargs or {}
 
     def factory(self):
         """Allow subclasses to customize the handler/server creation."""
-        return SMTP(self.handler, enable_SMTPUTF8=self.enable_SMTPUTF8,
-                    **self.server_kwargs)
+        return SMTP(
+            self.handler, enable_SMTPUTF8=self.enable_SMTPUTF8, **self.server_kwargs
+        )
+
+    def _factory_invoker(self):
+        """Wraps factory() to catch exceptions during instantiation"""
+        try:
+            self.smtpd = self.factory()
+            if self.smtpd is None:
+                raise RuntimeError("factory() returned None")
+            return self.smtpd
+        except Exception as err:
+            self._thread_exception = err
+            return _FakeServer(self.loop)
 
     def _run(self, ready_event):
         asyncio.set_event_loop(self.loop)
         try:
-            self.server = self.loop.run_until_complete(
-                self.loop.create_server(
-                    self.factory, host=self.hostname, port=self.port,
-                    ssl=self.ssl_context))
+            # Need to do two-step assignments here to ensure IDEs can properly
+            # detect the types of the vars. Cannot use `assert isinstance`, because
+            # Python 3.6 in asyncio debug mode has a bug wherein CoroWrapper is not
+            # an instance of Coroutine
+            srv_coro: Coroutine = self.loop.create_server(
+                self._factory_invoker,
+                host=self.hostname,
+                port=self.port,
+                ssl=self.ssl_context,
+            )
+            self.server_coro = srv_coro
+            srv: AsyncServer = self.loop.run_until_complete(
+                srv_coro
+            )
+            self.server = srv
         except Exception as error:  # pragma: nowsl
+            # Usually will enter this part only if create_server() cannot bind to the
+            # specified host:port.
+            #
             # Somehow WSL1.0 (Windows Subsystem for Linux) allows multiple
             # listeners on one port?!
             # That is why we add "pragma: nowsl" there, so when testing on
@@ -56,8 +117,20 @@ class Controller:
         self.loop.close()
         self.server = None
 
+    def _testconn(self):
+        """
+        Opens a socket connection to the newly launched server, wrapping in an SSL
+        Context if necessary, and read some data from it to ensure that factory()
+        gets invoked.
+        """
+        with ExitStack() as stk:
+            s = stk.enter_context(create_connection((self.hostname, self.port), 1.0))
+            if self.ssl_context:
+                s = stk.enter_context(self.ssl_context.wrap_socket(s))
+            _ = s.recv(1024)
+
     def start(self):
-        assert self._thread is None, 'SMTP daemon already running'
+        assert self._thread is None, "SMTP daemon already running"
         ready_event = threading.Event()
         self._thread = threading.Thread(target=self._run, args=(ready_event,))
         self._thread.daemon = True
@@ -67,18 +140,33 @@ class Controller:
         if self._thread_exception is not None:  # pragma: nowsl
             # See comment about WSL1.0 in the _run() method
             raise self._thread_exception
+        # Apparently create_server invokes factory() "lazily", so exceptions in
+        # factory() go undetected. To trigger factory() invocation we need to open
+        # a connection to the server and 'exchange' some traffic.
+        try:
+            self._testconn()
+        except Exception:
+            # We totally don't care of exceptions experienced by _testconn,
+            # which _will_ happen if factory() experienced problems.
+            pass
+        if self._thread_exception is not None:
+            raise self._thread_exception
+        # Defensive
+        if self.smtpd is None:
+            raise RuntimeError("Unknown Error, failed to init SMTP server")
 
     def _stop(self):
         self.loop.stop()
         try:
-            _all_tasks = asyncio.Task.all_tasks
-        except AttributeError:   # pragma: skipif_lt_py39
             _all_tasks = asyncio.all_tasks
+        except AttributeError:  # pragma: skipif_gt_py36
+            _all_tasks = asyncio.Task.all_tasks
         for task in _all_tasks(self.loop):
             task.cancel()
 
     def stop(self):
-        assert self._thread is not None, 'SMTP daemon not running'
+        assert self._thread is not None, "SMTP daemon not running"
         self.loop.call_soon_threadsafe(self._stop)
         self._thread.join()
         self._thread = None
+        self._thread_exception = None
diff --git a/aiosmtpd/docs/NEWS.rst b/aiosmtpd/docs/NEWS.rst
index 2acd503..181277e 100644
--- a/aiosmtpd/docs/NEWS.rst
+++ b/aiosmtpd/docs/NEWS.rst
@@ -10,6 +10,7 @@ Fixed/Improved
 * ``handle_exception()`` no longer gets called when the client disconnected (Closes #127, #162)
 * Implement & enforce line-length-limit, thus becoming Compliant with RFC 5321 § 4.5.3.1.6
 * Delay all SMTP Status Code replies during ``DATA`` phase until the phase termination (Closes #9)
+* Now catches ``Controller.factory()`` failure during ``Controller.start()`` (Closes #212)
 
 
 1.2.2 (2020-11-08)
diff --git a/aiosmtpd/docs/controller.rst b/aiosmtpd/docs/controller.rst
index 351ac79..74c2852 100644
--- a/aiosmtpd/docs/controller.rst
+++ b/aiosmtpd/docs/controller.rst
@@ -161,7 +161,9 @@ The EHLO response does not include the ``SMTPUTF8`` ESMTP option.
 Controller API
 ==============
 
-.. class:: Controller(handler, loop=None, hostname=None, port=8025, *, ready_timeout=1.0, enable_SMTPUTF8=True, ssl_context=None, server_kwargs=None)
+.. class:: Controller(handler, loop=None, hostname=None, port=8025, *, \
+   ready_timeout=1.0, enable_SMTPUTF8=True, ssl_context=None, \
+   server_kwargs=None)
 
    *handler* is an instance of a :ref:`handler <handlers>` class.
 
@@ -230,7 +232,24 @@ Controller API
       Start the server in the subthread.  The subthread is always a daemon
       thread (i.e. we always set ``thread.daemon=True``.  Exceptions can be
       raised if the server does not start within the *ready_timeout*, or if
-      any other exception occurs in while creating the server.
+      any other exception occurs in :meth:`factory` while creating the server.
+
+      .. important::
+
+         If :meth:`start` raises an Exception,
+         :class:`Controller` does not automatically perform cleanup,
+         to support deep inspection post-exception (if you wish to do so.)
+         Cleanup must still be performed manually by calling :meth:`stop`
+
+         For example::
+
+             controller = Controller(handler)
+             try:
+                 controller.start()
+             except ...:
+                 ... exception handling and/or inspection ...
+             finally:
+                 controller.stop()
 
    .. method:: stop()
 
@@ -242,8 +261,7 @@ Controller API
       class being controlled.  By default, this creates an ``SMTP`` instance,
       passing in your handler and setting the ``enable_SMTPUTF8`` flag.
       Examples of why you would want to override this method include creating
-      an ``LMTP`` server instance instead, or passing in a different set of
-      arguments to the ``SMTP`` constructor.
+      an ``LMTP`` server instance instead, or using your own ``SMTP`` class.
 
 
 .. _`asyncio event loop`: https://docs.python.org/3/library/asyncio-eventloop.html
diff --git a/aiosmtpd/docs/handlers.rst b/aiosmtpd/docs/handlers.rst
index 4a99255..b9776aa 100644
--- a/aiosmtpd/docs/handlers.rst
+++ b/aiosmtpd/docs/handlers.rst
@@ -120,7 +120,7 @@ synchronously (i.e. they are **not** coroutines).
     exception).  The exception object is passed in.  This method *must* return
     a status string, such as ``'542 Internal server error'``.  If the method
     returns ``None`` or raises an exception, an exception will be logged, and a
-    ``500`` code will be returned to the client.
+    ``451`` code will be returned to the client.
     **Note:** If client connection losted function will not be called.
 
 
diff --git a/aiosmtpd/smtp.py b/aiosmtpd/smtp.py
index 54c8606..38c20a9 100644
--- a/aiosmtpd/smtp.py
+++ b/aiosmtpd/smtp.py
@@ -51,7 +51,7 @@ _TriStateType = Union[None, _Missing, bytes]
 
 # region #### Constant & Constant-likes ###############################################
 
-__version__ = '1.2.3a2'
+__version__ = '1.2.3a3'
 __ident__ = 'Python SMTP {}'.format(__version__)
 log = logging.getLogger('mail.log')
 
@@ -315,7 +315,7 @@ class SMTP(asyncio.StreamReaderProtocol):
             return status
         else:
             log.exception('SMTP session exception')
-            status = '500 Error: ({}) {}'.format(
+            status = '451 Error: ({}) {}'.format(
                 error.__class__.__name__, str(error))
             return status
 
@@ -425,10 +425,10 @@ class SMTP(asyncio.StreamReaderProtocol):
                 except Exception as error:
                     try:
                         log.exception('Exception in handle_exception()')
-                        status = '500 Error: ({}) {}'.format(
+                        status = '451 Error: ({}) {}'.format(
                             error.__class__.__name__, str(error))
                     except Exception:
-                        status = '500 Error: Cannot describe error'
+                        status = '451 Error: Cannot describe error'
                     await self.push(status)
 
     async def check_helo_needed(self, helo: str = "HELO") -> bool:


Test patch:
diff --git a/aiosmtpd/tests/test_handlers.py b/aiosmtpd/tests/test_handlers.py
index 49b69f4..5ff869b 100644
--- a/aiosmtpd/tests/test_handlers.py
+++ b/aiosmtpd/tests/test_handlers.py
@@ -17,6 +17,19 @@ from unittest.mock import call, patch
 CRLF = '\r\n'
 
 
+ModuleResources = ExitStack()
+
+
+def setUpModule():
+    # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
+    # and oftentimes (not always, though) leads to Error
+    ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
+
+
+def tearDownModule():
+    ModuleResources.close()
+
+
 class DecodingController(Controller):
     def factory(self):
         return Server(self.handler, decode_data=True)
diff --git a/aiosmtpd/tests/test_lmtp.py b/aiosmtpd/tests/test_lmtp.py
index a3855e4..66fe8f3 100644
--- a/aiosmtpd/tests/test_lmtp.py
+++ b/aiosmtpd/tests/test_lmtp.py
@@ -6,7 +6,22 @@ import unittest
 from aiosmtpd.controller import Controller
 from aiosmtpd.handlers import Sink
 from aiosmtpd.lmtp import LMTP
+from contextlib import ExitStack
 from smtplib import SMTP
+from unittest.mock import patch
+
+
+ModuleResources = ExitStack()
+
+
+def setUpModule():
+    # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
+    # and oftentimes (not always, though) leads to Error
+    ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
+
+
+def tearDownModule():
+    ModuleResources.close()
 
 
 class LMTPController(Controller):
diff --git a/aiosmtpd/tests/test_main.py b/aiosmtpd/tests/test_main.py
index ee544ed..cd244c3 100644
--- a/aiosmtpd/tests/test_main.py
+++ b/aiosmtpd/tests/test_main.py
@@ -18,6 +18,19 @@ has_setuid = hasattr(os, 'setuid')
 log = logging.getLogger('mail.log')
 
 
+ModuleResources = ExitStack()
+
+
+def setUpModule():
+    # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
+    # and oftentimes (not always, though) leads to Error
+    ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
+
+
+def tearDownModule():
+    ModuleResources.close()
+
+
 class TestHandler1:
     def __init__(self, called):
         self.called = called
diff --git a/aiosmtpd/tests/test_server.py b/aiosmtpd/tests/test_server.py
index 545d9c5..c53920e 100644
--- a/aiosmtpd/tests/test_server.py
+++ b/aiosmtpd/tests/test_server.py
@@ -1,13 +1,30 @@
 """Test other aspects of the server implementation."""
 
+import gc
 import os
 import socket
 import unittest
 
-from aiosmtpd.controller import Controller
+from aiosmtpd.controller import asyncio, Controller, _FakeServer
 from aiosmtpd.handlers import Sink
 from aiosmtpd.smtp import SMTP as Server
+from contextlib import ExitStack
+from functools import wraps
 from smtplib import SMTP
+from unittest.mock import patch
+
+
+ModuleResources = ExitStack()
+
+
+def setUpModule():
+    # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
+    # and oftentimes (not always, though) leads to Error
+    ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
+
+
+def tearDownModule():
+    ModuleResources.close()
 
 
 def in_wsl():
@@ -23,9 +40,9 @@ class TestServer(unittest.TestCase):
         controller.start()
         self.addCleanup(controller.stop)
         with SMTP(controller.hostname, controller.port) as client:
-            code, response = client.ehlo('example.com')
+            code, response = client.ehlo("example.com")
         self.assertEqual(code, 250)
-        self.assertIn(b'SMTPUTF8', response.splitlines())
+        self.assertIn(b"SMTPUTF8", response.splitlines())
 
     def test_default_max_command_size_limit(self):
         server = Server(Sink())
@@ -33,7 +50,7 @@ class TestServer(unittest.TestCase):
 
     def test_special_max_command_size_limit(self):
         server = Server(Sink())
-        server.command_size_limits['DATA'] = 1024
+        server.command_size_limits["DATA"] = 1024
         self.assertEqual(server.max_command_size_limit, 1024)
 
     @unittest.skipIf(in_wsl(), "WSL prevents socket collisions")
@@ -56,3 +73,143 @@ class TestServer(unittest.TestCase):
         finally:
             controller.stop()
             self.assertIsNone(controller.server)
+
+
+# Silence the "Exception ignored ... RuntimeError: Event loop is closed" message.
+# This goes hand-in-hand with TestFactory.setUpClass() below.
+# Source: https://github.com/aio-libs/aiohttp/issues/4324#issuecomment-733884349
+def silencer(func):
+    @wraps(func)
+    def wrapper(self, *args, **kwargs):
+        try:
+            return func(self, *args, **kwargs)
+        except RuntimeError as e:
+            if str(e) != "Event loop is closed":
+                raise
+        except AttributeError as e:
+            # Added this to suppress the perplexing ignored exception below.
+            # Perplexing because I really don't know where that comes from. SMTP object
+            # could NOT have existed during that particular test case!
+            if str(e) != "'SMTP' object has no attribute '_closed'":
+                raise
+
+    return wrapper
+
+
+class TestFactory(unittest.TestCase):
+    Proactor = None
+    old_pa_del = None
+    SRProto = None
+    old_srp_del = None
+
+    @classmethod
+    def setUpClass(cls) -> None:
+        # See silencer() above
+        try:
+            # noinspection PyUnresolvedReferences
+            cls.Proactor = asyncio.proactor_events._ProactorBasePipeTransport
+            cls.old_pa_del = cls.Proactor.__del__
+            cls.Proactor.__del__ = silencer(cls.old_pa_del)
+        except AttributeError:
+            # proactor_events only available on Windows. So we'll just skip if it's
+            # not found (indicating non-Windows platform)
+            pass
+        try:
+            cls.SRProto = asyncio.streams.StreamReaderProtocol
+            # noinspection PyUnresolvedReferences
+            cls.old_srp_del = cls.SRProto.__del__
+            cls.SRProto.__del__ = silencer(cls.old_srp_del)
+        except AttributeError:
+            # Sometimes the __del__ method is ... missing?? What???
+            pass
+
+    @classmethod
+    def tearDownClass(cls) -> None:
+        # gc.collect() hinted in https://stackoverflow.com/a/25067818/149900
+        # Probably to remove leftover "Exception ignored"?
+        gc.collect()
+        if cls.old_pa_del is not None:
+            cls.Proactor.__del__ = cls.old_pa_del
+        if cls.old_srp_del is not None:
+            cls.SRProto.__del__ = cls.old_srp_del
+
+    def test_normal_situation(self):
+        cont = Controller(Sink())
+        try:
+            cont.start()
+            self.assertIsNotNone(cont.smtpd)
+            self.assertIsNone(cont._thread_exception)
+        finally:
+            cont.stop()
+
+    def test_unknown_args(self):
+        unknown = "this_is_an_unknown_kwarg"
+        cont = Controller(Sink(), server_kwargs={unknown: True})
+        try:
+            with self.assertRaises(TypeError) as cm:
+                cont.start()
+            self.assertIsNone(cont.smtpd)
+            excm = str(cm.exception)
+            self.assertIn("unexpected keyword", excm)
+            self.assertIn(unknown, excm)
+        finally:
+            cont.stop()
+
+    def test_conflict_smtputf8(self):
+        kwargs = dict(enable_SMTPUTF8=True)
+        cont = Controller(Sink(), server_kwargs=kwargs)
+        try:
+            with self.assertRaises(TypeError) as cm:
+                cont.start()
+            self.assertIsNone(cont.smtpd)
+            excm = str(cm.exception)
+            self.assertIn("multiple values", excm)
+            self.assertIn("enable_SMTPUTF8", excm)
+        finally:
+            cont.stop()
+
+    def test_factory_none(self):
+        # Hypothetical situation where factory() did not raise an Exception
+        # but returned None instead
+        with ExitStack() as stk:
+            cont = Controller(Sink())
+            stk.callback(cont.stop)
+
+            stk.enter_context(patch("aiosmtpd.controller.SMTP", return_value=None))
+
+            with self.assertRaises(RuntimeError) as cm:
+                cont.start()
+            self.assertIsNone(cont.smtpd)
+            excm = str(cm.exception)
+            self.assertEqual("factory() returned None", excm)
+
+    def test_noexc_smtpd_missing(self):
+        # Hypothetical situation where factory() failed but no
+        # Exception was generated.
+        with ExitStack() as stk:
+            cont = Controller(Sink())
+            stk.callback(cont.stop)
+
+            def hijacker(*args, **kwargs):
+                cont._thread_exception = None
+                # Must still return an (unmocked) _FakeServer to prevent a whole bunch
+                # of messy exceptions, although they doesn't affect the test at all.
+                return _FakeServer(cont.loop)
+
+            stk.enter_context(
+                patch("aiosmtpd.controller._FakeServer", side_effect=hijacker)
+            )
+
+            stk.enter_context(
+                patch(
+                    "aiosmtpd.controller.SMTP",
+                    side_effect=RuntimeError("Simulated Failure"),
+                )
+            )
+
+            with self.assertRaises(RuntimeError) as cm:
+                cont.start()
+            self.assertIsNone(cont.smtpd)
+            self.assertIsNone(cont._thread_exception)
+            excm = str(cm.exception)
+            self.assertEqual("Unknown Error, failed to init SMTP server", excm)
diff --git a/aiosmtpd/tests/test_smtp.py b/aiosmtpd/tests/test_smtp.py
index e29da7c..cb530c2 100644
--- a/aiosmtpd/tests/test_smtp.py
+++ b/aiosmtpd/tests/test_smtp.py
@@ -30,6 +30,19 @@ CRLF = '\r\n'
 BCRLF = b'\r\n'
 
 
+ModuleResources = ExitStack()
+
+
+def setUpModule():
+    # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
+    # and oftentimes (not always, though) leads to Error
+    ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
+
+
+def tearDownModule():
+    ModuleResources.close()
+
+
 def authenticator(mechanism, login, password):
     if login and login.decode() == 'goodlogin':
         return True
@@ -167,7 +180,7 @@ class ErroringHandlerCustomResponse:
 
     async def handle_exception(self, error):
         self.error = error
-        return '451 Temporary error: ({}) {}'.format(
+        return '554 Persistent error: ({}) {}'.format(
             error.__class__.__name__, str(error))
 
 
@@ -1516,7 +1529,7 @@ Testing
         self.addCleanup(controller.stop)
         with SMTP(controller.hostname, controller.port) as client:
             code, mesg = client.helo('example.com')
-        self.assertEqual(code, 500)
+        self.assertEqual(code, 451)
         self.assertEqual(mesg, b'Error: (ValueError) test')
         # handler.error did not change because the handler does not have a
         # handle_exception() method.
@@ -1532,8 +1545,8 @@ Testing
         self.addCleanup(controller.stop)
         with SMTP(controller.hostname, controller.port) as client:
             code, mesg = client.helo('example.com')
-        self.assertEqual(code, 451)
-        self.assertEqual(mesg, b'Temporary error: (ValueError) test')
+        self.assertEqual(code, 554)
+        self.assertEqual(mesg, b'Persistent error: (ValueError) test')
         self.assertIsInstance(handler.error, ValueError)
 
     # Suppress logging to the console during the tests.  Depending on
@@ -1546,7 +1559,7 @@ Testing
         self.addCleanup(controller.stop)
         with SMTP(controller.hostname, controller.port) as client:
             code, mesg = client.helo('example.com')
-        self.assertEqual(code, 500)
+        self.assertEqual(code, 451)
         self.assertEqual(mesg, b'Error: (ValueError) ErroringErrorHandler test')
         self.assertIsInstance(handler.error, ValueError)
 
@@ -1587,7 +1600,7 @@ Testing
         self.addCleanup(controller.stop)
         with SMTP(controller.hostname, controller.port) as client:
             code, mesg = client.helo('example.com')
-        self.assertEqual(code, 500)
+        self.assertEqual(code, 451)
         self.assertEqual(mesg, b'Error: Cannot describe error')
         self.assertIsInstance(handler.error, ValueError)
 
diff --git a/aiosmtpd/tests/test_smtps.py b/aiosmtpd/tests/test_smtps.py
index 1e999fe..e69b9cf 100644
--- a/aiosmtpd/tests/test_smtps.py
+++ b/aiosmtpd/tests/test_smtps.py
@@ -7,8 +7,23 @@ import pkg_resources
 
 from aiosmtpd.controller import Controller as BaseController
 from aiosmtpd.smtp import SMTP as SMTPProtocol
+from contextlib import ExitStack
 from email.mime.text import MIMEText
 from smtplib import SMTP_SSL
+from unittest.mock import patch
+
+
+ModuleResources = ExitStack()
+
+
+def setUpModule():
+    # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
+    # and oftentimes (not always, though) leads to Error
+    ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
+
+
+def tearDownModule():
+    ModuleResources.close()
 
 
 class Controller(BaseController):
diff --git a/aiosmtpd/tests/test_starttls.py b/aiosmtpd/tests/test_starttls.py
index 88fa5de..b9fc261 100644
--- a/aiosmtpd/tests/test_starttls.py
+++ b/aiosmtpd/tests/test_starttls.py
@@ -9,8 +9,23 @@ from aiosmtpd.testing.helpers import (
     SUPPORTED_COMMANDS_TLS,
     assert_auth_invalid,
 )
+from contextlib import ExitStack
 from email.mime.text import MIMEText
 from smtplib import SMTP
+from unittest.mock import patch
+
+
+ModuleResources = ExitStack()
+
+
+def setUpModule():
+    # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
+    # and oftentimes (not always, though) leads to Error
+    ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
+
+
+def tearDownModule():
+    ModuleResources.close()
 
 
 class Controller(BaseController):