From 3c0ecb5b3e804b6d07937e4dcb1a242ab3969e44 Mon Sep 17 00:00:00 2001 From: mvalentsev Date: Tue, 14 Jul 2026 13:59:49 +0500 Subject: [PATCH] fix(mcp): don't log a traceback when an HTTP client disconnects mid-response On the HTTP transport, a client that hangs up mid-response makes the send path raise an unhandled BrokenPipeError / ConnectionResetError, so the default socketserver handle_error logs a full traceback for a routine disconnect. Override handle_error on the HTTP server to log the disconnect (ConnectionError, plus ssl.SSLEOFError over TLS) at DEBUG and drop it, while every other exception still reaches the default handler with its traceback. The wider ssl.SSLError tree is left uncaught, so genuine TLS handshake/cert failures still surface. Addresses the disconnect half of #2003. --- mempalace/mcp_server.py | 22 +++++++++++ tests/test_mcp_http_transport.py | 64 ++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py index 73654a8..e2de9b9 100644 --- a/mempalace/mcp_server.py +++ b/mempalace/mcp_server.py @@ -5188,6 +5188,28 @@ def _build_http_server(host: str, port: int): daemon_threads = True allow_reuse_address = True + def handle_error(self, request, client_address): + # A client hanging up mid-response makes the send path raise + # ConnectionError (BrokenPipeError / ConnectionResetError), or + # ssl.SSLEOFError over TLS. That is a routine disconnect, not a + # server fault, so log it at DEBUG rather than let the default + # handler dump a per-request traceback. Real errors (including + # genuine TLS handshake/cert failures) still reach that handler. + exc = sys.exc_info()[1] + is_disconnect = isinstance(exc, ConnectionError) + if not is_disconnect: + import ssl + + # Only the abrupt-EOF SSLError; genuine TLS errors must surface. + is_disconnect = isinstance(exc, ssl.SSLEOFError) + if is_disconnect: + logger.debug( + "HTTP client %s disconnected before the response completed", + client_address, + ) + return + super().handle_error(request, client_address) + class _Handler(BaseHTTPRequestHandler): protocol_version = "HTTP/1.1" timeout = 10 diff --git a/tests/test_mcp_http_transport.py b/tests/test_mcp_http_transport.py index 80b0915..8c14f2f 100644 --- a/tests/test_mcp_http_transport.py +++ b/tests/test_mcp_http_transport.py @@ -19,6 +19,9 @@ Design constraints import http.client import json +import logging +import socketserver +import ssl import threading import pytest @@ -232,6 +235,67 @@ def test_read_only_off_exposes_mutating_tools(http_server): assert "mempalace_add_drawer" in names +@pytest.mark.parametrize( + "disconnect_exc", + [ + ConnectionResetError(104, "connection reset by peer"), + BrokenPipeError(32, "broken pipe"), + ssl.SSLEOFError("unexpected eof while reading"), + ], + ids=["connreset", "brokenpipe", "ssleof"], +) +def test_handle_error_quiets_client_disconnect(caplog, monkeypatch, disconnect_exc): + """Regression for #2003: a client that hangs up mid-response makes the send + path raise ConnectionError (BrokenPipeError / ConnectionResetError), or + ssl.SSLEOFError on the TLS transport. The server must log that quietly at + DEBUG instead of routing it to the default handler's per-request traceback. + """ + httpd = mcp._build_http_server("127.0.0.1", 0) + try: + delegated = [] + monkeypatch.setattr( + socketserver.BaseServer, + "handle_error", + lambda self, request, addr: delegated.append(addr), + ) + addr = ("127.0.0.1", 51234) + + with caplog.at_level(logging.DEBUG, logger="mempalace_mcp"): + try: + raise disconnect_exc + except type(disconnect_exc): + httpd.handle_error(None, addr) + + assert delegated == [] # noisy default handler NOT invoked + rec = next(r for r in caplog.records if "disconnect" in r.getMessage().lower()) + assert rec.levelno == logging.DEBUG + assert rec.name == "mempalace_mcp" + finally: + httpd.server_close() + + +def test_handle_error_delegates_real_errors(monkeypatch): + """A genuine error is NOT misclassified as a disconnect: it reaches the + default handler, so its traceback is still surfaced. + """ + httpd = mcp._build_http_server("127.0.0.1", 0) + try: + delegated = [] + monkeypatch.setattr( + socketserver.BaseServer, + "handle_error", + lambda self, request, addr: delegated.append(addr), + ) + addr = ("127.0.0.1", 51234) + try: + raise ValueError("boom") + except ValueError: + httpd.handle_error(None, addr) + assert delegated == [addr] + finally: + httpd.server_close() + + def _make_self_signed_cert(tmp_path): """Write a throwaway self-signed cert/key via openssl; skip if unavailable.""" import shutil