Fix AttributeError in the unserviceable-connection error handler

cheroot/server.py

@@ -1904,7 +1904,7 @@ def _serve_unservicable(self):
                 # We can't just raise an exception because that will kill this
                 # thread, and prevent 503 errors from being sent to future
                 # connections.
-                self.server.error_log(
+                self.error_log(
                     repr(ex),
                     level=logging.ERROR,
                     traceback=True,

cheroot/test/test_server.py

@@ -9,6 +9,7 @@
 import tempfile
 import threading
 import types
+import typing as _t
 import urllib.parse  # noqa: WPS301
 import uuid
 from http import HTTPStatus
@@ -20,7 +21,7 @@
 from pypytools.gc.custom import DefaultGc
 
 from .._compat import IS_LINUX, IS_MACOS, IS_WINDOWS, SYS_PLATFORM, bton, ntob
-from ..server import IS_UID_GID_RESOLVABLE, Gateway, HTTPServer
+from ..server import IS_UID_GID_RESOLVABLE, Gateway, HTTPRequest, HTTPServer
 from ..testing import (
     ANY_INTERFACE_IPV4,
     ANY_INTERFACE_IPV6,
@@ -583,11 +584,11 @@ def test_threadpool_multistart_validation(monkeypatch):
         tp.start()
 
 
-def test_overload_results_in_suitable_http_error(request):
-    """A server that can't keep up with requests returns a 503 HTTP error."""
-    localhost = '127.0.0.1'
+@pytest.fixture
+def overloaded_http_server() -> _t.Iterator[HTTPServer]:
+    """Return a running server that answers every request with a 503."""
     httpserver = HTTPServer(
-        bind_addr=(localhost, EPHEMERAL_PORT),
+        bind_addr=('127.0.0.1', EPHEMERAL_PORT),
         gateway=Gateway,
     )
     # Can only handle on request in parallel:
@@ -602,19 +603,76 @@ def test_overload_results_in_suitable_http_error(request):
     httpserver.prepare()
     serve_thread = threading.Thread(target=httpserver.serve)
     serve_thread.start()
-    request.addfinalizer(httpserver.stop)
-    # Stop the thread pool to ensure the queue fills up:
-    httpserver.requests.stop()
+    try:
+        # Stop the thread pool to ensure the queue fills up:
+        httpserver.requests.stop()
+
+        # Use up the very limited thread pool queue we've set up, so future
+        # requests fail:
+        httpserver.requests._queue.put(None)
+
+        yield httpserver
+    finally:
+        httpserver.stop()
+
+
+def test_overload_results_in_suitable_http_error(
+    overloaded_http_server: HTTPServer,
+) -> None:
+    """A server that can't keep up with requests returns a 503 HTTP error."""
+    host, port = overloaded_http_server.bind_addr
+
+    response = requests.get(f'http://{host}:{port}', timeout=20)
+    assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE
+
+
+def test_overload_survives_failure_to_send_http_error(
+    overloaded_http_server: HTTPServer,
+    monkeypatch: pytest.MonkeyPatch,
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    """An unexpected error while sending a 503 is logged, not fatal.
+
+    The overload handling must keep on answering later connections.
+
+    This is a test for issue #797.
+    """
+    host, port = overloaded_http_server.bind_addr
+    real_simple_response = HTTPRequest.simple_response
+    attempts = []
+
+    def failing_once_simple_response(
+        request: HTTPRequest,
+        status: str,
+        msg: str = '',
+    ) -> None:
+        attempts.append(status)
+        if len(attempts) == 1:
+            raise RuntimeError('unexpected failure sending the 503')
+        real_simple_response(request, status, msg)
 
-    _host, port = httpserver.bind_addr
+    monkeypatch.setattr(
+        HTTPRequest,
+        'simple_response',
+        failing_once_simple_response,
+    )
 
-    # Use up the very limited thread pool queue we've set up, so future
-    # requests fail:
-    httpserver.requests._queue.put(None)
+    # The server fails to send the 503 to this connection and leaves it
+    # lingering, so the client won't ever get a response over it:
+    with socket.create_connection((host, port), timeout=20):
+        # The overload thread handles connections one by one, so once this
+        # one is answered, the failed attempt above has been dealt with:
+        response = requests.get(f'http://{host}:{port}', timeout=20)
 
-    response = requests.get(f'http://{localhost}:{port}', timeout=20)
     assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE
 
+    # The original failure is reported instead of a bogus `AttributeError`:
+    captured_stderr = capsys.readouterr().err
+    assert (
+        'RuntimeError: unexpected failure sending the 503' in captured_stderr
+    )
+    assert 'AttributeError' not in captured_stderr
+
 
 def test_overload_thread_does_not_leak():
     """On shutdown the overload thread exits.

docs/changelog-fragments.d/797.bugfix.rst

@@ -0,0 +1,7 @@
+Fixed the server's ability to send ``503 Service Unavailable`` whenever the
+server becomes too busy. Failures to send caused the handler for this message
+to crash, leaving new connections during busy times unanswered. Any failure in
+sending this message is now logged, and subsequent connections continue to be
+answered.
+
+-- by :user:`Tatamis`, :user:`avinashkamat48` and :user:`andrewkernel`

docs/conf.py

@@ -150,6 +150,10 @@
     ('py:class', '_pyio.BufferedWriter'),
     ('py:class', '_pyio.BufferedReader'),
     ('py:class', 'unittest.case.TestCase'),
+    # Pytest exposes these under the public `pytest.` names but the
+    # documented objects live in its private `_pytest` package:
+    ('py:class', '_pytest.capture.CaptureFixture'),
+    ('py:class', '_pytest.monkeypatch.MonkeyPatch'),
     ('py:meth', 'cheroot.connections.ConnectionManager.get_conn'),
     # Ref: https://github.com/pyca/pyopenssl/issues/1012
     ('py:class', 'pyopenssl:OpenSSL.SSL.Context'),