Merge pull request #2004 from mvalentsev/fix/2003-http-client-disconnect
fix(mcp): don't log a traceback when an HTTP client disconnects mid-response (#2003)
This commit is contained in:
commit
32e545eaaa
|
|
@ -5205,6 +5205,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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue