Skip to content

Commit 25e1ea2

Browse files
g97iulio1609Copilot
andcommitted
fix: handle ClosedResourceError when logging errors to disconnected clients
When a client disconnects mid-request, _handle_message catches the resulting Exception and tries to send_log_message() back to the client. Since the session write stream is already closed, this raises ClosedResourceError (or BrokenResourceError), which is unhandled and crashes the stateless session with an ExceptionGroup. This is a different code path from PR #1384, which fixed the message router loop. This bug is in the error recovery path: catch exception → try to log it to client → write stream closed → crash. Fix: catch ClosedResourceError and BrokenResourceError around the send_log_message call in _handle_message, since failing to notify a disconnected client is expected and harmless. Fixes #2064 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 62575ed commit 25e1ea2

2 files changed

Lines changed: 56 additions & 5 deletions

File tree

src/mcp/server/lowlevel/server.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -417,11 +417,16 @@ async def _handle_message(
417417
)
418418
case Exception():
419419
logger.error(f"Received exception from stream: {message}")
420-
await session.send_log_message(
421-
level="error",
422-
data="Internal Server Error",
423-
logger="mcp.server.exception_handler",
424-
)
420+
try:
421+
await session.send_log_message(
422+
level="error",
423+
data="Internal Server Error",
424+
logger="mcp.server.exception_handler",
425+
)
426+
except (anyio.ClosedResourceError, anyio.BrokenResourceError):
427+
logger.warning(
428+
"Could not send error log to client: write stream already closed"
429+
)
425430
if raise_exceptions:
426431
raise message
427432
case _:

tests/server/test_lowlevel_exception_handling.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,49 @@ async def test_normal_message_handling_not_affected():
7272

7373
# Verify _handle_request was called
7474
server._handle_request.assert_called_once()
75+
76+
77+
@pytest.mark.anyio
78+
async def test_exception_handling_tolerates_closed_write_stream():
79+
"""send_log_message should not crash when the client has already disconnected."""
80+
import anyio
81+
82+
server = Server("test-server")
83+
session = Mock(spec=ServerSession)
84+
session.send_log_message = AsyncMock(side_effect=anyio.ClosedResourceError)
85+
86+
test_exception = RuntimeError("client went away")
87+
88+
# Must not raise — the ClosedResourceError should be caught internally
89+
await server._handle_message(test_exception, session, {}, raise_exceptions=False)
90+
91+
92+
@pytest.mark.anyio
93+
async def test_exception_handling_tolerates_broken_write_stream():
94+
"""send_log_message should not crash when the write stream is broken."""
95+
import anyio
96+
97+
server = Server("test-server")
98+
session = Mock(spec=ServerSession)
99+
session.send_log_message = AsyncMock(side_effect=anyio.BrokenResourceError)
100+
101+
test_exception = RuntimeError("client went away")
102+
103+
# Must not raise — the BrokenResourceError should be caught internally
104+
await server._handle_message(test_exception, session, {}, raise_exceptions=False)
105+
106+
107+
@pytest.mark.anyio
108+
async def test_exception_handling_closed_stream_with_raise_exceptions():
109+
"""Even with raise_exceptions=True, ClosedResourceError from log should be tolerated."""
110+
import anyio
111+
112+
server = Server("test-server")
113+
session = Mock(spec=ServerSession)
114+
session.send_log_message = AsyncMock(side_effect=anyio.ClosedResourceError)
115+
116+
test_exception = RuntimeError("client went away")
117+
118+
# raise_exceptions re-raises the *original* exception, not the ClosedResourceError
119+
with pytest.raises(RuntimeError, match="client went away"):
120+
await server._handle_message(test_exception, session, {}, raise_exceptions=True)

0 commit comments

Comments
 (0)