Enabled logging of failed port bind (v2)

CHANGES.txt

@@ -19,6 +19,11 @@ Bugfix
   https://github.com/Pylons/waitress/pull/475 and
   https://github.com/Pylons/waitress/issues/464
 
+- Enabled logging of failed port bind in `waysyncore.py:dispatcher.bind` to aid
+  in debugging application bind failures.  See
+  https://github.com/Pylons/waitress/issues/471
+
+
 3.0.2 (2024-11-16)
 ------------------
 

src/waitress/wasyncore.py

@@ -371,7 +371,11 @@ def listen(self, num):
 
     def bind(self, addr):
         self.addr = addr
-        return self.socket.bind(addr)
+        try:
+            return self.socket.bind(addr)
+        except Exception as exc:
+            self.logger.critical("Failed bind to: `%s` : `%s`", str(addr), exc)
+            raise
 
     def accept(self):
         # XXX can return either an address pair or None

tests/test_server.py

@@ -1,11 +1,26 @@
 import errno
+import os
 import socket
+import sys
+from typing import TYPE_CHECKING, List, Union
 import unittest
 
+if TYPE_CHECKING:
+    from waitress.server import BaseWSGIServer, MultiSocketServer, UnixWSGIServer
+
 dummy_app = object()
 
 
 class TestWSGIServer(unittest.TestCase):
+
+    # maintain a list of instantiated servers (for cleanup)
+    insts: List[Union["BaseWSGIServer", "MultiSocketServer"]]
+
+    def __init__(self, *args, **kwargs):
+        # override base `__init__` to track `self.insts` for `self.tearDown`
+        super(TestWSGIServer, self).__init__(*args, **kwargs)
+        self.insts = []
+
     def _makeOne(
         self,
         application=dummy_app,
@@ -20,7 +35,7 @@ def _makeOne(
     ):
         from waitress.server import create_server
 
-        self.inst = create_server(
+        inst = create_server(
             application,
             host=host,
             port=port,
@@ -29,7 +44,8 @@ def _makeOne(
             _start=_start,
             _sock=_sock,
         )
-        return self.inst
+        self.insts.append(inst)
+        return inst
 
     def _makeOneWithMap(
         self, adj=None, _start=True, host="127.0.0.1", port=0, app=dummy_app
@@ -55,15 +71,16 @@ def _makeOneWithMulti(
         map = {}
         from waitress.server import create_server
 
-        self.inst = create_server(
+        inst = create_server(
             app,
             listen=listen,
             map=map,
             _dispatcher=task_dispatcher,
             _start=_start,
             _sock=sock,
         )
-        return self.inst
+        self.insts.append(inst)
+        return inst
 
     def _makeWithSockets(
         self,
@@ -80,22 +97,23 @@ def _makeWithSockets(
         _sockets = []
         if sockets is not None:
             _sockets = sockets
-        self.inst = create_server(
+        inst = create_server(
             application,
             map=map,
             _dispatcher=_dispatcher,
             _start=_start,
             _sock=_sock,
             sockets=_sockets,
         )
-        return self.inst
+        self.insts.append(inst)
+        return inst
 
     def tearDown(self):
-        if self.inst is not None:
-            self.inst.close()
+        # iterate the list of instantiated servers and `close()` them
+        for inst in self.insts:
+            inst.close()
 
     def test_ctor_app_is_None(self):
-        self.inst = None
         self.assertRaises(ValueError, self._makeOneWithMap, app=None)
 
     def test_ctor_start_true(self):
@@ -263,10 +281,13 @@ def test_backward_compatibility(self):
         from waitress.server import TcpWSGIServer, WSGIServer
 
         self.assertIs(WSGIServer, TcpWSGIServer)
-        self.inst = WSGIServer(None, _start=False, port=1234)
+        inst = WSGIServer(None, _start=False, port=1234)
         # Ensure the adjustment was actually applied.
-        self.assertNotEqual(Adjustments.port, 1234)
-        self.assertEqual(self.inst.adj.port, 1234)
+        try:
+            self.assertNotEqual(Adjustments.port, 1234)
+            self.assertEqual(inst.adj.port, 1234)
+        finally:
+            inst.close()
 
     def test_create_with_one_tcp_socket(self):
         from waitress.server import TcpWSGIServer
@@ -311,12 +332,49 @@ def test_create_with_one_socket_handle_accept_noerror(self):
         self.assertListEqual(innersock.opts, [("level", "optname", "value")])
         self.assertListEqual(L, [(inst, innersock, None, inst.adj)])
 
+    @unittest.skipIf(
+        sys.platform.startswith("win"),
+        "Windows doesn't raise an exception when reusing a port",
+    )
+    def test_port_bind_failure_logging(self):
+        """
+        Ensure the address is logged on a failed port bind.
+
+        This test was developed for #471 - logging a failed bind.
+
+        This test will leave a socket open until #480 is fixed.
+        """
+
+        # create a first app correctly
+        inst_a = self._makeOne(port=8080)
+
+        # Ensure a second app correctly binds to a different host+port
+        inst_b = self._makeOne(port=8081)
+
+        # a third app should fail the bind to the fist app's host+port
+        with self.assertLogs("waitress", level="ERROR") as cm_log:
+            with self.assertRaises(OSError) as cm:
+                # this line will trigger #480 and leave a socket open
+                inst_c = self._makeOne(port=8080)
+            self.assertEqual(cm.exception.errno, errno.EADDRINUSE)
+
+        # check log for inst_c failure
+        self.assertIn(
+            "CRITICAL:waitress:"
+            "Failed bind to: `('127.0.0.1', 8080)` : "
+            f"`[Errno {errno.EADDRINUSE}] {os.strerror(errno.EADDRINUSE)}`",
+            cm_log.output,
+        )
+
 
 if hasattr(socket, "AF_UNIX"):
 
     class TestUnixWSGIServer(unittest.TestCase):
         unix_socket = "/tmp/waitress.test.sock"
 
+        # maintain a list of instantiated servers (for cleanup)
+        inst: List[Union["BaseWSGIServer", "MultiSocketServer", "UnixWSGIServer"]]
+
         def _makeOne(self, _start=True, _sock=None):
             from waitress.server import create_server