Quietly end FastAGI request when a client disconnects during the handshake#64
Merged
Merged
Conversation
When a FastAGI client closes before sending the full AGI environment, the handshake read raises AGISIGPIPEHangup from inside _AGIClientHandler.handle(). It propagated uncaught, so socketserver printed a full traceback to stderr for a routine event (caller hung up, Asterisk aborted the leg, or a bare TCP probe). Wrap only the FastAGI(...) handshake construction in except AGIHangup and return early. The suppression is scoped to the environment read, so genuine errors from the script handler, which runs after this guard, still propagate. Add regression tests: a client that closes during the handshake makes handle() return without raising, and a handler that raises still propagates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…iew) Review panel convergence: - Narrow the handler guard from the AGIHangup base to AGISIGPIPEHangup, the only hangup reachable during the environment read (no command is sent, so AGIResultHangup cannot arise). This documents intent precisely, makes the code match the comment/CHANGELOG/test which already name AGISIGPIPEHangup, and avoids silently swallowing a future non-disconnect hangup introduced into construction. - Strengthen the disconnect test: assert the script handler is never invoked and nothing is written back on the quiet path. - Add a mid-handshake disconnect test (partial environment then EOF) covering the parse-loop EOF path, distinct from the first-read EOF case. - CHANGELOG: say the traceback is printed to stderr rather than "logged", since socketserver writes directly to stderr. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #49.
Problem
When a FastAGI client connection closes before the AGI environment handshake completes,
_AGIClientHandler.handle()constructsFastAGI(...)as its first action, which reads the environment block. If the connection is already gone,_read_lineraisesAGISIGPIPEHangup.handle()did not catch it, so the defaultsocketservererror handling printed a full traceback to stderr for a routine, expected condition.Common triggers, all benign:
AGI()step.No in-flight call is affected. The only problem is the alarming traceback noise.
Fix
Wrap only the
FastAGI(...)handshake construction inexcept AGIHangupand return early (option A from the issue). The suppression is scoped to the environment read, so it cannot hide handler-time failures: errors raised by the script handler, which runs after this guard, still propagate.AGIHangupis the base of the handshake hangup (AGISIGPIPEHangup), so the guard covers the disconnect category without swallowing protocol or handler errors.I left the optional debug log line out. The module has no logger and the issue lists it as optional, so the surgical change is to simply end the request quietly. Easy to add later if a disconnect log is wanted.
Tests
New
tests/test_fastagi_handler.py:handle()return without raising (no traceback). Fails against the old code, passes against the fix.Full suite: 52 passing, ruff clean.
Out of scope
Disconnects after the handshake (mid-call, inside a handler) are a separate path, typically a SIGPIPE on the next channel write. Not covered here, per the issue.