diff --git a/CHANGELOG.md b/CHANGELOG.md index 96ddc6e..dc12401 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed - `Manager.monitor_connection` no longer crashes its monitoring thread when the connection drops. Pinging a downed connection raised `ManagerSocketError` (broken socket) or `ManagerError` (the liveness check inside `send_action` failing when the connection dropped just after the loop's own check) inside the thread, which dumped a traceback to stderr and killed the monitor. The monitor now catches both and stops cleanly, logging the reason at debug level when a logger is set. The method also returns the monitoring thread so callers can join it (#3). - `Manager.send_action` no longer raises a raw `AttributeError` when a concurrent `disconnect()` clears the connection between the liveness check and the send. It now drops the just-registered request and raises `ManagerSocketError` instead (#3). +- The FastAGI server no longer prints an unhandled traceback to stderr when a client disconnects during the AGI environment handshake. A caller hanging up, Asterisk aborting the leg, or a bare TCP probe raised `AGISIGPIPEHangup` from the handler and printed a full traceback for a routine event. The handler now ends the request quietly. Errors raised by the script handler itself still propagate (#49). ## [1.3.0] - 2026-06-24 diff --git a/pystrix/agi/fastagi.py b/pystrix/agi/fastagi.py index 43bc693..8758c99 100644 --- a/pystrix/agi/fastagi.py +++ b/pystrix/agi/fastagi.py @@ -96,7 +96,17 @@ def handle(self): Creates an instance of an AGI-interface object and passes it to a pre-specified callable, selected by matching the request parameters against a series of regular expressions. """ - agi_instance = FastAGI(self.rfile, self.wfile, debug=self.server.debug) + try: + agi_instance = FastAGI(self.rfile, self.wfile, debug=self.server.debug) + except AGISIGPIPEHangup: + # The client's pipe closed before the full AGI environment arrived: + # the caller hung up as the call reached the AGI step, Asterisk + # aborted the leg, or a bare TCP probe connected and closed. No call + # is in flight, so end the request quietly instead of letting the + # hangup propagate into a socketserver stderr traceback. This catches + # only the handshake-disconnect signal; errors from the handler + # itself are raised below, outside this guard, so they still surface. + return (path, kwargs) = self._extract_query_elements(agi_instance) args = self._extract_positional_args(agi_instance) diff --git a/tests/test_fastagi_handler.py b/tests/test_fastagi_handler.py new file mode 100644 index 0000000..939d45e --- /dev/null +++ b/tests/test_fastagi_handler.py @@ -0,0 +1,87 @@ +"""Tests for the FastAGI client handler (`_AGIClientHandler`).""" + +import io +import types + +import pytest + +from pystrix.agi.fastagi import _AGIClientHandler + + +class _ClosedReader: + """The read end of a socket whose client has already disconnected.""" + + def readline(self): + return b"" # EOF: nothing was sent before the connection closed + + +class _EnvReader: + """Yields a minimal AGI environment block, then EOF.""" + + def __init__(self, *lines): + self._lines = list(lines) + self._index = 0 + + def readline(self): + if self._index >= len(self._lines): + return b"" + line = self._lines[self._index] + self._index += 1 + return line + + +def _handler(rfile, handler_callable=None): + # Bypass socketserver's __init__ (which would call handle() itself) and wire + # up only what handle() touches: the read/write files and the server. + instance = _AGIClientHandler.__new__(_AGIClientHandler) + instance.rfile = rfile + instance.wfile = io.BytesIO() + instance.client_address = ("198.51.100.7", 51234) + instance.server = types.SimpleNamespace( + debug=False, + get_script_handler=lambda path: (handler_callable, None), + ) + return instance + + +def test_handle_returns_quietly_when_client_disconnects_during_handshake(): + # Regression for #49: a client that closes before sending the AGI + # environment makes the handshake raise AGISIGPIPEHangup. handle() must end + # the request quietly instead of letting that propagate into a stderr + # traceback from socketserver. + invoked = [] + handler = _handler( + _ClosedReader(), handler_callable=lambda *args: invoked.append(args) + ) + + handler.handle() # must not raise + + assert invoked == [] # no script handler runs when the client is already gone + assert handler.wfile.getvalue() == b"" # nothing written back on the quiet path + + +def test_handle_returns_quietly_when_client_disconnects_mid_handshake(): + # A client that sends part of the environment then drops hits EOF inside the + # parse loop, not on the first read. Still a handshake hangup that must end + # the request quietly. + invoked = [] + reader = _EnvReader(b"agi_network_script: demo\n") # no blank terminator, then EOF + handler = _handler(reader, handler_callable=lambda *args: invoked.append(args)) + + handler.handle() # must not raise + + assert invoked == [] + + +def test_handle_propagates_handler_errors(): + # The handshake-hangup suppression is scoped to the environment read only. + # A genuine error raised by the script handler must still propagate. + def boom(agi, args, kwargs, match, path): + raise RuntimeError("handler blew up") + + handler = _handler( + _EnvReader(b"agi_network_script: demo\n", b"\n"), + handler_callable=boom, + ) + with pytest.raises(RuntimeError): + handler.handle()