diff --git a/CHANGES.txt b/CHANGES.txt index c7f32ea4..3163b252 100644 --- a/CHANGES.txt +++ b/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) ------------------ diff --git a/src/waitress/wasyncore.py b/src/waitress/wasyncore.py index 79a593bf..3aaa53f7 100644 --- a/src/waitress/wasyncore.py +++ b/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 diff --git a/tests/test_server.py b/tests/test_server.py index cede49a7..d0a49e00 100644 --- a/tests/test_server.py +++ b/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,7 +71,7 @@ def _makeOneWithMulti( map = {} from waitress.server import create_server - self.inst = create_server( + inst = create_server( app, listen=listen, map=map, @@ -63,7 +79,8 @@ def _makeOneWithMulti( _start=_start, _sock=sock, ) - return self.inst + self.insts.append(inst) + return inst def _makeWithSockets( self, @@ -80,7 +97,7 @@ def _makeWithSockets( _sockets = [] if sockets is not None: _sockets = sockets - self.inst = create_server( + inst = create_server( application, map=map, _dispatcher=_dispatcher, @@ -88,14 +105,15 @@ def _makeWithSockets( _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