diff --git a/pytest_httpserver/blocking_httpserver.py b/pytest_httpserver/blocking_httpserver.py index 096e9e4..cf74757 100644 --- a/pytest_httpserver/blocking_httpserver.py +++ b/pytest_httpserver/blocking_httpserver.py @@ -46,6 +46,8 @@ class BlockingHTTPServer(HTTPServerBase): :param timeout: waiting time in seconds for matching and responding to an incoming request. manager + :param startup_timeout: maximum time in seconds to wait for server readiness. + Set to ``None`` to disable readiness waiting. .. py:attribute:: no_handler_status_code @@ -63,8 +65,10 @@ def __init__( port: int = DEFAULT_LISTEN_PORT, ssl_context: SSLContext | None = None, timeout: int = 30, + *, + startup_timeout: float | None = 10.0, ) -> None: - super().__init__(host, port, ssl_context) + super().__init__(host, port, ssl_context, startup_timeout=startup_timeout) self.timeout = timeout self.request_queue: Queue[Request] = Queue() self.request_handlers: dict[Request, Queue[BlockingRequestHandler]] = {} diff --git a/pytest_httpserver/httpserver.py b/pytest_httpserver/httpserver.py index 5e7f09d..b08894e 100644 --- a/pytest_httpserver/httpserver.py +++ b/pytest_httpserver/httpserver.py @@ -8,6 +8,7 @@ import threading import time import urllib.parse +import warnings from collections import defaultdict from collections.abc import Callable from collections.abc import Generator @@ -615,6 +616,8 @@ class HTTPServerBase(abc.ABC): # pylint: disable=too-many-instance-attributes :param port: the TCP port where the server will listen :param ssl_context: the ssl context object to use for https connections :param threaded: whether to handle concurrent requests in separate threads + :param startup_timeout: maximum time in seconds to wait for server readiness. + Set to ``None`` to disable readiness waiting. .. py:attribute:: log @@ -638,6 +641,7 @@ def __init__( ssl_context: SSLContext | None = None, *, threaded: bool = False, + startup_timeout: float | None = 10.0, ) -> None: """ Initializes the instance. @@ -652,7 +656,9 @@ def __init__( self.log: list[tuple[Request, Response]] = [] self.ssl_context = ssl_context self.threaded = threaded + self.startup_timeout = startup_timeout self.no_handler_status_code = 500 + self._server_ready_event: threading.Event = threading.Event() def __repr__(self) -> str: return f"<{self.__class__.__name__} host={self.host} port={self.port}>" @@ -733,8 +739,14 @@ def thread_target(self) -> None: This method serves as a thread target when the server is started. This should not be called directly, but can be overridden to tailor it to your needs. + + If overriding, you should call ``self._server_ready_event.set()`` before starting + to serve requests. If the event is not set within the timeout, :py:meth:`start` + will emit a warning if the thread is still alive; if the thread dies during + startup, :py:meth:`start` raises an error. """ assert self.server is not None + self._server_ready_event.set() self.server.serve_forever() def is_running(self) -> bool: @@ -774,8 +786,33 @@ def start(self) -> None: self.port = self.server.port # Update port (needed if `port` was set to 0) # Explicitly make the new thread daemonic to avoid shutdown issues + # Create a new event for each startup to prevent stale threads from + # signaling readiness for a subsequent start() attempt. + self._server_ready_event = threading.Event() self.server_thread = threading.Thread(target=self.thread_target, daemon=True) self.server_thread.start() + if self.startup_timeout is not None and not self._server_ready_event.wait(timeout=self.startup_timeout): + # Event was not set within timeout. + # Check if thread is still alive (custom thread_target may not set the event) + if self.server_thread.is_alive(): + # Server thread is running, assume it's working (backward compatibility) + warnings.warn( + "Server thread is running but ready event was not set. " + "If you override thread_target(), call self._server_ready_event.set() " + "before serving to ensure reliable startup.", + stacklevel=2, + ) + else: + # Thread died, clean up and raise + self.server.server_close() + self.server_thread.join(timeout=5) + self.server = None + self.server_thread = None + raise HTTPServerError( + "Server thread died during startup. " + "If you override thread_target(), ensure it calls " + "self._server_ready_event.set() before serving." + ) def stop(self) -> None: """ @@ -937,6 +974,8 @@ class HTTPServer(HTTPServerBase): # pylint: disable=too-many-instance-attribute manager :param threaded: whether to handle concurrent requests in separate threads + :param startup_timeout: maximum time in seconds to wait for server readiness. + Set to ``None`` to disable readiness waiting. .. py:attribute:: no_handler_status_code @@ -956,11 +995,18 @@ def __init__( default_waiting_settings: WaitingSettings | None = None, *, threaded: bool = False, + startup_timeout: float | None = 10.0, ) -> None: """ Initializes the instance. """ - super().__init__(host, port, ssl_context, threaded=threaded) + super().__init__( + host, + port, + ssl_context, + threaded=threaded, + startup_timeout=startup_timeout, + ) self.ordered_handlers: list[RequestHandler] = [] self.oneshot_handlers = RequestHandlerList() diff --git a/tests/test_release.py b/tests/test_release.py index 5c3dbfd..caa1ac8 100644 --- a/tests/test_release.py +++ b/tests/test_release.py @@ -232,6 +232,7 @@ def test_sdist_contents(build: Build, version: str): "test_querymatcher.py", "test_querystring.py", "test_release.py", + "test_server_startup.py", "test_ssl.py", "test_thread_type.py", "test_threaded.py", diff --git a/tests/test_server_startup.py b/tests/test_server_startup.py new file mode 100644 index 0000000..adf8647 --- /dev/null +++ b/tests/test_server_startup.py @@ -0,0 +1,188 @@ +from __future__ import annotations + +import contextlib +import socket +import time + +import pytest +import requests +from requests.exceptions import Timeout + +from pytest_httpserver import HTTPServer + + +def test_server_ready_immediately_after_start() -> None: + """Test that the server accepts connections immediately after start() returns.""" + server = HTTPServer(host="localhost", port=0) + server.expect_request("/").respond_with_data("ok") + server.start() + try: + # Attempt to connect immediately - should not fail + sock = socket.create_connection((server.host, server.port), timeout=1) + sock.close() + finally: + server.stop() + + +def test_server_ready_under_load() -> None: + """Test that the server is ready even when started multiple times in succession.""" + for _ in range(10): + server = HTTPServer(host="localhost", port=0) + server.expect_request("/").respond_with_data("ok") + server.start() + try: + sock = socket.create_connection((server.host, server.port), timeout=1) + sock.close() + finally: + server.stop() + + +class SlowStartServer(HTTPServer): + """A server subclass that simulates slow startup.""" + + def thread_target(self) -> None: + time.sleep(0.5) # Simulate slow initialization + self._server_ready_event.set() + assert self.server is not None + self.server.serve_forever() + + +class NoReadyEventServer(HTTPServer): + """A server subclass that never signals readiness.""" + + def thread_target(self) -> None: + assert self.server is not None + self.server.serve_forever() + + +def test_slow_start_server_waits_for_ready() -> None: + """Test that start() waits for slow thread_target implementations.""" + server = SlowStartServer(host="localhost", port=0) + server.expect_request("/").respond_with_data("ok") + + start_time = time.monotonic() + server.start() + elapsed = time.monotonic() - start_time + + try: + # Should have waited at least 0.5 seconds + assert elapsed >= 0.5 + # Server should be ready + sock = socket.create_connection((server.host, server.port), timeout=1) + sock.close() + finally: + server.stop() + + +def test_new_event_created_for_each_start() -> None: + """Test that a new event is created for each start() to isolate retries.""" + server = HTTPServer(host="localhost", port=0) + server.expect_request("/").respond_with_data("ok") + + original_event = server._server_ready_event # noqa: SLF001 + + server.start() + first_start_event = server._server_ready_event # noqa: SLF001 + server.stop() + + server.start() + second_start_event = server._server_ready_event # noqa: SLF001 + server.stop() + + # Each start() should create a new event + assert first_start_event is not original_event + assert second_start_event is not first_start_event + + +def test_warns_when_ready_event_not_set() -> None: + """Test that a warning is emitted when the ready event is never set.""" + server = NoReadyEventServer(host="localhost", port=0, startup_timeout=0.0) + server.expect_request("/").respond_with_data("ok") + + with pytest.warns(UserWarning, match="ready event was not set"): + server.start() + + try: + deadline = time.time() + 1 + while time.time() < deadline: + with contextlib.suppress(OSError): + sock = socket.create_connection((server.host, server.port), timeout=0.1) + sock.close() + break + time.sleep(0.01) + else: + raise AssertionError("Server did not accept connections within 1 second") + finally: + server.stop() + + +class SlowServeServer(HTTPServer): + """A server that delays serve_forever() but does not set ready event early. + + This simulates the scenario where: + - bind() and listen() complete (TCP connections queue in backlog) + - But serve_forever() hasn't started yet (no HTTP responses) + """ + + def thread_target(self) -> None: + assert self.server is not None + # Delay before serve_forever - connections will queue but not be processed + time.sleep(3.0) + self._server_ready_event.set() + self.server.serve_forever() + + +def test_http_request_fails_before_serve_forever_without_wait() -> None: + """ + Demonstrate the race condition: TCP connects but HTTP times out. + + This test shows why waiting for server readiness matters: + - After start(), TCP connections succeed (queued in backlog) + - But HTTP requests timeout because serve_forever() hasn't started + - With short client timeouts (common in production), this causes failures + """ + # Use startup_timeout=0 to NOT wait for ready event (old behavior) + server = SlowServeServer(host="localhost", port=0, startup_timeout=0.0) + server.expect_request("/ping").respond_with_data("pong") + + with pytest.warns(UserWarning, match="ready event was not set"): + server.start() + + try: + # TCP connection succeeds (proves Zsolt's point about backlog) + sock = socket.create_connection((server.host, server.port), timeout=1) + sock.close() + + # But HTTP request with short timeout fails! + # This is the actual problem in containerized environments + with pytest.raises(Timeout): + requests.get(server.url_for("/ping"), timeout=(0.5, 0.5)) + finally: + server.stop() + + +def test_http_request_succeeds_when_waiting_for_ready() -> None: + """ + Demonstrate that waiting for ready event fixes the race condition. + + With startup_timeout enabled (default), start() waits until + serve_forever() begins, so HTTP requests succeed immediately. + """ + # Use default startup_timeout to wait for ready event + server = SlowServeServer(host="localhost", port=0) # default startup_timeout=10.0 + server.expect_request("/ping").respond_with_data("pong") + + start_time = time.monotonic() + server.start() + elapsed = time.monotonic() - start_time + + try: + # Should have waited for the slow startup + assert elapsed >= 3.0, f"Expected to wait >= 3.0s, but only waited {elapsed}s" + + # HTTP request succeeds because serve_forever() has started + response = requests.get(server.url_for("/ping"), timeout=(0.5, 0.5)) + assert response.status_code == 200 + assert response.text == "pong" + finally: + server.stop()