Skip to content

Commit 9f63603

Browse files
committed
fix: InMemoryTransport EOFs the server instead of cancelling; coverage cleanup
src/mcp/client/_memory.py: replace tg.cancel_scope.cancel() with stream aclose(). Cancelling here throws CancelledError into the host (test's) task; on CPython 3.11 (gh-106749) coro.throw() drops 'call' trace events for the outer await chain, underflowing coverage's CTracer past the test frame. Post-swap the dispatcher's empty-at-EOF inner task group takes a one-tick fast path, so the join no longer suspends a second time to heal via .send(). EOF teardown is equivalent (the dispatcher's run() cancels its own in-flight handlers on read-stream EOF). Also bundled (coverage cleanup): - shared/session.py: delete server-only BaseSession code now unreachable after the swap (RequestResponder.cancel/_cancel_scope/_on_complete, CancelledNotification handling, _in_flight dict, deferred-respond arm). ClientSession is the only remaining subclass. - tests/client/test_session.py: add tests for the client-reachable BaseSession paths (malformed inbound request -> INVALID_PARAMS, sampling callback raises -> INVALID_PARAMS, progress callback exception swallowed, malformed notification dropped, transport exception forwarded to message_handler). - tests/shared/test_session.py: drop test_in_flight_requests_cleared (asserts the deleted _in_flight dict). - tests/shared/test_dispatcher.py: add contract test for ValidationError -> INVALID_PARAMS (covers DirectDispatcher's arm). - tests/server/test_validation.py: cover the previous-message-has-no- tool-use branch. - examples/everything-server: _add_request_handler (deleted) -> add_request_handler with explicit params type.
1 parent 535d621 commit 9f63603

7 files changed

Lines changed: 192 additions & 95 deletions

File tree

‎examples/servers/everything-server/mcp_everything_server/server.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,9 +417,15 @@ async def handle_unsubscribe(ctx: ServerRequestContext, params: UnsubscribeReque
417417
return EmptyResult()
418418

419419

420-
mcp._lowlevel_server._add_request_handler("logging/setLevel", handle_set_logging_level) # pyright: ignore[reportPrivateUsage]
421-
mcp._lowlevel_server._add_request_handler("resources/subscribe", handle_subscribe) # pyright: ignore[reportPrivateUsage]
422-
mcp._lowlevel_server._add_request_handler("resources/unsubscribe", handle_unsubscribe) # pyright: ignore[reportPrivateUsage]
420+
mcp._lowlevel_server.add_request_handler( # pyright: ignore[reportPrivateUsage]
421+
"logging/setLevel", SetLevelRequestParams, handle_set_logging_level
422+
)
423+
mcp._lowlevel_server.add_request_handler( # pyright: ignore[reportPrivateUsage]
424+
"resources/subscribe", SubscribeRequestParams, handle_subscribe
425+
)
426+
mcp._lowlevel_server.add_request_handler( # pyright: ignore[reportPrivateUsage]
427+
"resources/unsubscribe", UnsubscribeRequestParams, handle_unsubscribe
428+
)
423429

424430

425431
@mcp.completion()

‎src/mcp/client/_memory.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,15 @@ async def _connect(self) -> AsyncIterator[TransportStreams]:
6262
try:
6363
yield client_read, client_write
6464
finally:
65-
tg.cancel_scope.cancel()
65+
# EOF the server (and our own read side) instead of
66+
# cancelling. The dispatcher's run() cancels its own
67+
# in-flight handlers on read-stream EOF, so cleanup is
68+
# equivalent. Cancelling here would `coro.throw()` into the
69+
# host task, which on CPython 3.11 (gh-106749) drops
70+
# `'call'` trace events for the outer await chain and
71+
# desyncs coverage's CTracer past the test frame.
72+
await client_write.aclose()
73+
await server_write.aclose()
6674

6775
async def __aenter__(self) -> TransportStreams:
6876
"""Connect to the server and return streams for communication."""

‎src/mcp/shared/session.py‎

Lines changed: 23 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import contextvars
44
import logging
5-
from collections.abc import Callable
65
from contextlib import AsyncExitStack
76
from types import TracebackType
87
from typing import Any, Generic, Protocol, TypeVar
@@ -21,7 +20,6 @@
2120
CONNECTION_CLOSED,
2221
INVALID_PARAMS,
2322
REQUEST_TIMEOUT,
24-
CancelledNotification,
2523
ClientNotification,
2624
ClientRequest,
2725
ClientResult,
@@ -80,7 +78,6 @@ def __init__(
8078
request_meta: RequestParamsMeta | None,
8179
request: ReceiveRequestT,
8280
session: BaseSession[SendRequestT, SendNotificationT, SendResultT, ReceiveRequestT, ReceiveNotificationT],
83-
on_complete: Callable[[RequestResponder[ReceiveRequestT, SendResultT]], Any],
8481
message_metadata: MessageMetadata = None,
8582
context: contextvars.Context | None = None,
8683
) -> None:
@@ -91,15 +88,10 @@ def __init__(
9188
self.context = context
9289
self._session = session
9390
self._completed = False
94-
self._cancel_scope = anyio.CancelScope()
95-
self._on_complete = on_complete
9691
self._entered = False # Track if we're in a context manager
9792

9893
def __enter__(self) -> RequestResponder[ReceiveRequestT, SendResultT]:
99-
"""Enter the context manager, enabling request cancellation tracking."""
10094
self._entered = True
101-
self._cancel_scope = anyio.CancelScope()
102-
self._cancel_scope.__enter__()
10395
return self
10496

10597
def __exit__(
@@ -108,15 +100,7 @@ def __exit__(
108100
exc_val: BaseException | None,
109101
exc_tb: TracebackType | None,
110102
) -> None:
111-
"""Exit the context manager, performing cleanup and notifying completion."""
112-
try:
113-
if self._completed:
114-
self._on_complete(self)
115-
finally:
116-
self._entered = False
117-
if not self._cancel_scope: # pragma: no cover
118-
raise RuntimeError("No active cancel scope")
119-
self._cancel_scope.__exit__(exc_type, exc_val, exc_tb)
103+
self._entered = False
120104

121105
async def respond(self, response: SendResultT | ErrorData) -> None:
122106
"""Send a response for this request.
@@ -130,37 +114,11 @@ async def respond(self, response: SendResultT | ErrorData) -> None:
130114
if not self._entered: # pragma: no cover
131115
raise RuntimeError("RequestResponder must be used as a context manager")
132116
assert not self._completed, "Request already responded to"
133-
134-
if not self.cancelled: # pragma: no branch
135-
self._completed = True
136-
137-
await self._session._send_response( # type: ignore[reportPrivateUsage]
138-
request_id=self.request_id, response=response
139-
)
140-
141-
async def cancel(self) -> None:
142-
"""Cancel this request and mark it as completed."""
143-
if not self._entered: # pragma: no cover
144-
raise RuntimeError("RequestResponder must be used as a context manager")
145-
if not self._cancel_scope: # pragma: no cover
146-
raise RuntimeError("No active cancel scope")
147-
148-
self._cancel_scope.cancel()
149-
self._completed = True # Mark as completed so it's removed from in_flight
150-
# Send an error response to indicate cancellation
117+
self._completed = True
151118
await self._session._send_response( # type: ignore[reportPrivateUsage]
152-
request_id=self.request_id,
153-
response=ErrorData(code=0, message="Request cancelled"),
119+
request_id=self.request_id, response=response
154120
)
155121

156-
@property
157-
def in_flight(self) -> bool: # pragma: no cover
158-
return not self._completed and not self.cancelled
159-
160-
@property
161-
def cancelled(self) -> bool:
162-
return self._cancel_scope.cancel_called
163-
164122

165123
class BaseSession(
166124
Generic[
@@ -180,7 +138,6 @@ class BaseSession(
180138

181139
_response_streams: dict[RequestId, MemoryObjectSendStream[JSONRPCResponse | JSONRPCError]]
182140
_request_id: int
183-
_in_flight: dict[RequestId, RequestResponder[ReceiveRequestT, SendResultT]]
184141
_progress_callbacks: dict[RequestId, ProgressFnT]
185142

186143
def __init__(
@@ -195,7 +152,6 @@ def __init__(
195152
self._response_streams = {}
196153
self._request_id = 0
197154
self._session_read_timeout_seconds = read_timeout_seconds
198-
self._in_flight = {}
199155
self._progress_callbacks = {}
200156
self._exit_stack = AsyncExitStack()
201157

@@ -347,15 +303,10 @@ async def _handle_session_message(message: SessionMessage) -> None:
347303
request_meta=validated_request.params.meta if validated_request.params else None,
348304
request=validated_request,
349305
session=self,
350-
on_complete=lambda r: self._in_flight.pop(r.request_id, None),
351306
message_metadata=message.metadata,
352307
context=sender_context,
353308
)
354-
self._in_flight[responder.request_id] = responder
355309
await self._received_request(responder)
356-
357-
if not responder._completed: # type: ignore[reportPrivateUsage]
358-
await self._handle_incoming(responder)
359310
except Exception:
360311
# For request validation errors, send a proper JSON-RPC error
361312
# response instead of crashing the server
@@ -375,33 +326,28 @@ async def _handle_session_message(message: SessionMessage) -> None:
375326
message.message.model_dump(by_alias=True, mode="json", exclude_none=True),
376327
by_name=False,
377328
)
378-
# Handle cancellation notifications
379-
if isinstance(notification, CancelledNotification):
380-
cancelled_id = notification.params.request_id
381-
if cancelled_id in self._in_flight: # pragma: no branch
382-
await self._in_flight[cancelled_id].cancel()
383-
else:
384-
# Handle progress notifications callback
385-
if isinstance(notification, ProgressNotification):
386-
progress_token = notification.params.progress_token
387-
# If there is a progress callback for this token,
388-
# call it with the progress information
389-
if progress_token in self._progress_callbacks:
390-
callback = self._progress_callbacks[progress_token]
391-
try:
392-
await callback(
393-
notification.params.progress,
394-
notification.params.total,
395-
notification.params.message,
396-
)
397-
except Exception:
398-
logging.exception("Progress callback raised an exception")
399-
await self._received_notification(notification)
400-
await self._handle_incoming(notification)
329+
# Handle progress notifications callback
330+
if isinstance(notification, ProgressNotification):
331+
progress_token = notification.params.progress_token
332+
# If there is a progress callback for this token,
333+
# call it with the progress information
334+
if progress_token in self._progress_callbacks:
335+
callback = self._progress_callbacks[progress_token]
336+
try:
337+
await callback(
338+
notification.params.progress,
339+
notification.params.total,
340+
notification.params.message,
341+
)
342+
except Exception:
343+
logging.exception("Progress callback raised an exception")
344+
await self._received_notification(notification)
345+
await self._handle_incoming(notification)
401346
except Exception:
402347
# For other validation errors, log and continue
403-
logging.warning( # pragma: no cover
404-
f"Failed to validate notification:. Message was: {message.message}",
348+
logging.warning(
349+
"Failed to validate notification: %s",
350+
message.message,
405351
exc_info=True,
406352
)
407353
else: # Response or error

‎tests/client/test_session.py‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
from __future__ import annotations
22

3+
from collections.abc import AsyncIterator
4+
from contextlib import asynccontextmanager
5+
from typing import Any
6+
37
import anyio
8+
import anyio.streams.memory
49
import pytest
510

611
from mcp import types
@@ -10,12 +15,14 @@
1015
from mcp.shared.session import RequestResponder
1116
from mcp.shared.version import SUPPORTED_PROTOCOL_VERSIONS
1217
from mcp.types import (
18+
INVALID_PARAMS,
1319
LATEST_PROTOCOL_VERSION,
1420
CallToolResult,
1521
Implementation,
1622
InitializedNotification,
1723
InitializeRequest,
1824
InitializeResult,
25+
JSONRPCError,
1926
JSONRPCNotification,
2027
JSONRPCRequest,
2128
JSONRPCResponse,
@@ -26,6 +33,29 @@
2633
client_request_adapter,
2734
)
2835

36+
_SendToClient = anyio.streams.memory.MemoryObjectSendStream[SessionMessage | Exception]
37+
_RecvFromClient = anyio.streams.memory.MemoryObjectReceiveStream[SessionMessage]
38+
39+
40+
@asynccontextmanager
41+
async def raw_client_session(
42+
**kwargs: Any,
43+
) -> AsyncIterator[tuple[ClientSession, _SendToClient, _RecvFromClient]]:
44+
"""Yield `(session, send_to_client, recv_from_client)` with the receive loop running.
45+
46+
`send_to_client` accepts `SessionMessage | Exception` so tests can inject
47+
transport-level exceptions. No initialize handshake is performed.
48+
"""
49+
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](32)
50+
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage](32)
51+
async with ClientSession(s2c_recv, c2s_send, **kwargs) as session:
52+
try:
53+
with anyio.fail_after(5):
54+
yield session, s2c_send, c2s_recv
55+
finally:
56+
s2c_send.close()
57+
c2s_recv.close()
58+
2959

3060
@pytest.mark.anyio
3161
async def test_client_session_initialize():
@@ -705,3 +735,96 @@ async def mock_server():
705735
await session.initialize()
706736

707737
await session.call_tool(name=mocked_tool.name, arguments={"foo": "bar"}, meta=meta)
738+
739+
740+
@pytest.mark.anyio
741+
async def test_receive_loop_answers_malformed_inbound_request_with_invalid_params():
742+
"""A request that fails ServerRequest validation gets an INVALID_PARAMS error response."""
743+
async with raw_client_session() as (_session, to_client, from_client):
744+
await to_client.send(
745+
SessionMessage(JSONRPCRequest(jsonrpc="2.0", id=7, method="sampling/createMessage", params={"broken": 1}))
746+
)
747+
out = await from_client.receive()
748+
assert isinstance(out.message, JSONRPCError)
749+
assert out.message.id == 7
750+
assert out.message.error.code == INVALID_PARAMS
751+
752+
753+
@pytest.mark.anyio
754+
async def test_receive_loop_answers_invalid_params_when_sampling_callback_raises():
755+
"""Same boundary catches exceptions from the request handler itself."""
756+
757+
async def boom(ctx: object, params: object) -> types.CreateMessageResult:
758+
raise RuntimeError("sampling boom")
759+
760+
params = types.CreateMessageRequestParams(
761+
messages=[types.SamplingMessage(role="user", content=types.TextContent(type="text", text="hi"))],
762+
max_tokens=10,
763+
).model_dump(by_alias=True, mode="json", exclude_none=True)
764+
async with raw_client_session(sampling_callback=boom) as (_session, to_client, from_client):
765+
await to_client.send(
766+
SessionMessage(JSONRPCRequest(jsonrpc="2.0", id=8, method="sampling/createMessage", params=params))
767+
)
768+
out = await from_client.receive()
769+
assert isinstance(out.message, JSONRPCError)
770+
assert out.message.error.code == INVALID_PARAMS
771+
772+
773+
@pytest.mark.anyio
774+
async def test_receive_loop_logs_and_drops_malformed_notification(caplog: pytest.LogCaptureFixture):
775+
"""A notification that fails ServerNotification validation is logged and dropped."""
776+
seen: list[object] = []
777+
delivered = anyio.Event()
778+
779+
async def handler(msg: object) -> None:
780+
seen.append(msg)
781+
delivered.set()
782+
783+
async with raw_client_session(message_handler=handler) as (_session, to_client, _):
784+
await to_client.send(SessionMessage(JSONRPCNotification(jsonrpc="2.0", method="not/a/spec/notification")))
785+
# Follow with a valid notification so we know the loop is still alive.
786+
await to_client.send(
787+
SessionMessage(JSONRPCNotification(jsonrpc="2.0", method="notifications/tools/list_changed"))
788+
)
789+
await delivered.wait()
790+
assert isinstance(seen[0], types.ToolListChangedNotification)
791+
assert "Failed to validate notification" in caplog.text
792+
793+
794+
@pytest.mark.anyio
795+
async def test_receive_loop_forwards_transport_exception_to_message_handler():
796+
seen: list[object] = []
797+
delivered = anyio.Event()
798+
799+
async def handler(msg: object) -> None:
800+
seen.append(msg)
801+
delivered.set()
802+
803+
async with raw_client_session(message_handler=handler) as (_session, to_client, _):
804+
exc = ValueError("bad bytes")
805+
await to_client.send(exc)
806+
await delivered.wait()
807+
assert seen == [exc]
808+
809+
810+
@pytest.mark.anyio
811+
async def test_receive_loop_swallows_progress_callback_exception(caplog: pytest.LogCaptureFixture):
812+
delivered = anyio.Event()
813+
814+
async def boom(progress: float, total: float | None, message: str | None) -> None:
815+
raise RuntimeError("progress boom")
816+
817+
async def handler(msg: object) -> None:
818+
delivered.set()
819+
820+
async with raw_client_session(message_handler=handler) as (session, to_client, _):
821+
# Register the callback under a known token without sending a request.
822+
session._progress_callbacks[42] = boom # pyright: ignore[reportPrivateUsage]
823+
params = {"progressToken": 42, "progress": 0.5}
824+
await to_client.send(
825+
SessionMessage(JSONRPCNotification(jsonrpc="2.0", method="notifications/progress", params=params))
826+
)
827+
# The progress notification also reaches the message handler after the
828+
# callback runs, so this fires once the callback's exception is handled.
829+
await delivered.wait()
830+
assert "Progress callback raised an exception" in caplog.text

‎tests/server/test_validation.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,16 @@ def test_validate_tool_use_result_messages_raises_when_tool_result_without_previ
120120
validate_tool_use_result_messages(messages)
121121

122122

123+
def test_validate_tool_use_result_messages_raises_when_previous_message_has_no_tool_use() -> None:
124+
"""Raises when tool_result follows a message that has content but no tool_use."""
125+
messages = [
126+
SamplingMessage(role="assistant", content=TextContent(type="text", text="just text")),
127+
SamplingMessage(role="user", content=ToolResultContent(type="tool_result", tool_use_id="tool-1")),
128+
]
129+
with pytest.raises(ValueError, match="do not match any tool_use in the previous message"):
130+
validate_tool_use_result_messages(messages)
131+
132+
123133
def test_validate_tool_use_result_messages_raises_when_tool_result_ids_dont_match_tool_use() -> None:
124134
"""Raises when tool_result IDs don't match tool_use IDs."""
125135
messages = [

0 commit comments

Comments
 (0)