From a9a52b60b5c8b607d8107b435bffb4396a51bc18 Mon Sep 17 00:00:00 2001 From: Will James Date: Tue, 5 May 2026 14:34:10 -0500 Subject: [PATCH 1/2] fix(lowlevel): log Disconnect/Cancel exceptions at DEBUG, not ERROR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The low-level server's generic case-Exception handler logs any exception from the session stream at ERROR level. When the streamable HTTP transport sends ClientDisconnect through the writer (to unblock the inner session task), it was caught here and logged as ERROR — producing the same noise we're trying to eliminate. Add a guard clause matching Disconnect/Cancel exception types (by name, to keep the low-level server transport-agnostic) and log at DEBUG instead. Also skip sending 'Internal Server Error' to the client (there is none). --- src/mcp/server/lowlevel/server.py | 3 +++ src/mcp/server/streamable_http.py | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index 2dd1a8277..0beb58de2 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -703,6 +703,9 @@ async def _handle_message( await self._handle_request(message, req, session, lifespan_context, raise_exceptions) case types.ClientNotification(root=notify): await self._handle_notification(notify) + case Exception() if "Disconnect" in type(message).__name__ or "Cancel" in type(message).__name__: # pragma: no cover + # Client went away (Disconnect) or task was cancelled — not a server error. + logger.debug(f"Received {type(message).__name__} from stream") case Exception(): # pragma: no cover logger.error(f"Received exception from stream: {message}") await session.send_log_message( diff --git a/src/mcp/server/streamable_http.py b/src/mcp/server/streamable_http.py index 72f88af44..366add99d 100644 --- a/src/mcp/server/streamable_http.py +++ b/src/mcp/server/streamable_http.py @@ -654,8 +654,9 @@ async def sse_writer(): if writer is not None: with suppress(Exception): await writer.send(ClientDisconnect()) - # 499 = Client Closed Request (nginx convention, not in stdlib HTTPStatus) - response = self._create_json_response(None, 499) # type: ignore[arg-type] + # 499 = Client Closed Request (nginx convention, not in stdlib HTTPStatus). + # Build Response directly to avoid _create_json_response's HTTPStatus type hint. + response = Response(content=b"", status_code=499, media_type=CONTENT_TYPE_JSON) with suppress(Exception): await response(scope, receive, send) return From 38d1b5b8638493d0fe89ee7b96633b21cab31638 Mon Sep 17 00:00:00 2001 From: Will James Date: Tue, 5 May 2026 14:49:33 -0500 Subject: [PATCH 2/2] refactor(streamable_http): remove writer.send(ClientDisconnect) on disconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The connect() context manager closing streams is sufficient to unblock the session task — no need to send ClientDisconnect through the writer. This avoids the low-level server's case-Exception handler catching it and producing ERROR-level noise (or requiring a separate DEBUG-level handler). Session task exits cleanly when streams are closed on context manager exit. --- src/mcp/server/lowlevel/server.py | 3 - src/mcp/server/streamable_http.py | 7 +- .../test_client_disconnect_post.py | 79 +------------------ 3 files changed, 2 insertions(+), 87 deletions(-) diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index 0beb58de2..2dd1a8277 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -703,9 +703,6 @@ async def _handle_message( await self._handle_request(message, req, session, lifespan_context, raise_exceptions) case types.ClientNotification(root=notify): await self._handle_notification(notify) - case Exception() if "Disconnect" in type(message).__name__ or "Cancel" in type(message).__name__: # pragma: no cover - # Client went away (Disconnect) or task was cancelled — not a server error. - logger.debug(f"Received {type(message).__name__} from stream") case Exception(): # pragma: no cover logger.error(f"Received exception from stream: {message}") await session.send_log_message( diff --git a/src/mcp/server/streamable_http.py b/src/mcp/server/streamable_http.py index 366add99d..46a010e41 100644 --- a/src/mcp/server/streamable_http.py +++ b/src/mcp/server/streamable_http.py @@ -649,13 +649,8 @@ async def sse_writer(): # server error — log at WARNING and send a response so middleware chains # (e.g. Starlette BaseHTTPMiddleware) don't raise "No response returned". # The ASGI server will drop the response if the socket is already closed. - # Notify the writer so the inner session task can unblock cleanly. + # The connect() context manager will close streams, unblocking the session task. logger.warning("Client disconnected during POST request") - if writer is not None: - with suppress(Exception): - await writer.send(ClientDisconnect()) - # 499 = Client Closed Request (nginx convention, not in stdlib HTTPStatus). - # Build Response directly to avoid _create_json_response's HTTPStatus type hint. response = Response(content=b"", status_code=499, media_type=CONTENT_TYPE_JSON) with suppress(Exception): await response(scope, receive, send) diff --git a/tests/server/streamable_http/test_client_disconnect_post.py b/tests/server/streamable_http/test_client_disconnect_post.py index cc5d5062c..a20d97542 100644 --- a/tests/server/streamable_http/test_client_disconnect_post.py +++ b/tests/server/streamable_http/test_client_disconnect_post.py @@ -12,7 +12,7 @@ import logging from typing import Any -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock import pytest from starlette.requests import ClientDisconnect, Request @@ -125,81 +125,4 @@ async def dummy_send(message): assert send_calls[0]["type"] == "http.response.start" assert send_calls[0]["status"] == 499 - @pytest.mark.anyio - async def test_client_disconnect_notifies_writer(self): - """Writer should receive ClientDisconnect so the inner session task can unblock.""" - transport = StreamableHTTPServerTransport(mcp_session_id=None) - scope = self._make_scope() - - # Capture what the writer receives - writer_messages: list[Any] = [] - - async def capture_writer(msg): - writer_messages.append(msg) - - # Patch the internal writer - with patch.object(transport, "_read_stream_writer", MagicMock(send=capture_writer)): - mock_request = MagicMock(spec=Request) - mock_request.body = AsyncMock(side_effect=ClientDisconnect()) - mock_request.headers = { - "content-type": "application/json", - "accept": "application/json, text/event-stream", - } - mock_request.scope = scope - - send_calls: list[Any] = [] - - async def dummy_receive(): - return {"type": "http.request", "body": b""} - - async def dummy_send(message): - send_calls.append(message) - - await transport._handle_post_request( - scope, mock_request, dummy_receive, dummy_send - ) - - # Writer should have been notified with ClientDisconnect - assert len(writer_messages) == 1, ( - f"Expected writer to receive 1 message, got {len(writer_messages)}" - ) - assert isinstance(writer_messages[0], ClientDisconnect), ( - f"Expected ClientDisconnect sent to writer, got {type(writer_messages[0])}" - ) - @pytest.mark.anyio - async def test_client_disconnect_writer_suppresses_errors(self): - """If the writer itself is broken, we should not crash (suppress(Exception)).""" - transport = StreamableHTTPServerTransport(mcp_session_id=None) - scope = self._make_scope() - - broken_send = AsyncMock(side_effect=RuntimeError("writer is broken")) - - with patch.object(transport, "_read_stream_writer", MagicMock(send=broken_send)): - mock_request = MagicMock(spec=Request) - mock_request.body = AsyncMock(side_effect=ClientDisconnect()) - mock_request.headers = { - "content-type": "application/json", - "accept": "application/json, text/event-stream", - } - mock_request.scope = scope - - async def dummy_receive(): - return {"type": "http.request", "body": b""} - - send_calls: list[Any] = [] - - async def dummy_send(message): - send_calls.append(message) - - # Should not raise even though writer.send() fails - await transport._handle_post_request( - scope, mock_request, dummy_receive, dummy_send - ) - - # The broken writer.send was called once (suppressed) - broken_send.assert_called_once() - # Response is still sent even though writer was broken - assert len(send_calls) >= 1 - assert send_calls[0]["type"] == "http.response.start" - assert send_calls[0]["status"] == 499