Skip to content

Commit 0a54692

Browse files
committed
Tidy naming and doc nits from review
- Rename _SHIELDED_WRITE_TIMEOUT to _ABANDON_WRITE_TIMEOUT; the timed-out arm passes it unshielded, so the old name lied there - Document that on_stream_exception is awaited inline in the read loop - Note in run()'s docstring that the dispatcher is single-shot - Import ProgressFnT from mcp.shared.dispatcher (its home) instead of the mcp.shared.session compat shim - Migration guide: scope the optional-fields section to ServerRequestContext, and correct the null-id bullet (v1 surfaced the transport's ValidationError, not an MCPError)
1 parent 04778b6 commit 0a54692

5 files changed

Lines changed: 19 additions & 17 deletions

File tree

docs/migration.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,9 +1120,9 @@ async def handle_call_tool(ctx: ServerRequestContext, params: CallToolRequestPar
11201120
)
11211121
```
11221122

1123-
### `RequestContext`: request-specific fields are now optional
1123+
### `ServerRequestContext`: request-specific fields are now optional
11241124

1125-
The `RequestContext` class now uses optional fields for request-specific data (`request_id`, `meta`, etc.) so it can be used for both request and notification handlers. In notification handlers, these fields are `None`.
1125+
`ServerRequestContext` now uses optional fields for request-specific data (`request_id`, `meta`, etc.) so it can be used for both request and notification handlers. In notification handlers, these fields are `None`.
11261126

11271127
```python
11281128
from mcp.server import ServerRequestContext
@@ -1177,7 +1177,7 @@ Behavior changes:
11771177
- **Session shutdown now answers in-flight server-initiated requests with `CONNECTION_CLOSED` (-32000)**; v1 left them unanswered. The write is bounded (~1s) so closing stays fast.
11781178
- **Notification callbacks are concurrent.** `logging_callback`, `progress_callback`, and `message_handler` deliveries start in arrival order but each runs as its own task: they may interleave, and a `progress_callback` delivery may finish after the request it reports on has returned. Callbacks that need strict sequencing must coordinate themselves, and there is no built-in bound on concurrent deliveries (v1's inline loop processed one message at a time).
11791179
- **Transport-level `Exception` items are delivered to `message_handler` the same way** — as their own task, without blocking the receive loop — and a `message_handler` that raises on one is logged, not fatal to the session.
1180-
- **Stray responses are no longer surfaced to `message_handler`.** Responses with an unknown id are ignored (as the spec asks; v1 surfaced a `RuntimeError`), and error responses with a null `id` — a peer reporting a parse error — are dropped with a debug log (v1 surfaced an `MCPError`).
1180+
- **Stray responses are no longer surfaced to `message_handler`.** Responses with an unknown id are ignored (as the spec asks; v1 surfaced a `RuntimeError`), and error responses with a null `id` — a peer reporting a parse error — are dropped with a debug log (v1 surfaced the transport's `ValidationError`).
11811181
- **A raising request callback** is answered with `code=0` and the exception text; v1 flattened every callback exception to `INVALID_PARAMS`. For a specific error response, return `ErrorData` (unchanged) or raise `MCPError`. One carve-out: pydantic's `ValidationError` is still answered with `INVALID_PARAMS`, as in v1.
11821182
- **`send_request` before entering the context manager** raises `RuntimeError` immediately; v1 wrote to the transport and hung until the timeout. After the connection has closed it raises `MCPError` (`CONNECTION_CLOSED`) instead. `send_notification` before entry still works.
11831183
- **`send_notification` no longer takes `related_request_id`, and `send_request` no longer accepts `ServerMessageMetadata`.** No client transport ever serialized these hints; progress and response correlation via `progressToken` and the request id is unaffected.

src/mcp/client/client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from mcp.client.streamable_http import streamable_http_client
1313
from mcp.server import Server
1414
from mcp.server.mcpserver import MCPServer
15-
from mcp.shared.session import ProgressFnT
15+
from mcp.shared.dispatcher import ProgressFnT
1616
from mcp.types import (
1717
CallToolResult,
1818
CompleteResult,

src/mcp/client/session.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
from mcp import types
1616
from mcp.client._transport import ReadStream, WriteStream
1717
from mcp.shared._compat import resync_tracer
18-
from mcp.shared.dispatcher import CallOptions, DispatchContext, Dispatcher
18+
from mcp.shared.dispatcher import CallOptions, DispatchContext, Dispatcher, ProgressFnT
1919
from mcp.shared.exceptions import MCPError
2020
from mcp.shared.jsonrpc_dispatcher import JSONRPCDispatcher
2121
from mcp.shared.message import ClientMessageMetadata, SessionMessage
22-
from mcp.shared.session import ProgressFnT, RequestResponder
22+
from mcp.shared.session import RequestResponder
2323
from mcp.shared.transport_context import TransportContext
2424
from mcp.shared.version import SUPPORTED_PROTOCOL_VERSIONS
2525
from mcp.types import RequestId, RequestParamsMeta

src/mcp/shared/jsonrpc_dispatcher.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@
5151

5252
logger = logging.getLogger(__name__)
5353

54-
_SHIELDED_WRITE_TIMEOUT: float = 5
55-
"""Bound for courtesy abandon-path writes; without it a wedged transport
56-
would turn the shielded write into an uncancellable hang."""
54+
_ABANDON_WRITE_TIMEOUT: float = 5
55+
"""Bound for courtesy-cancel writes on the abandon paths; the caller-cancel
56+
arm shields its write, so a wedged transport would otherwise hang it uncancellably."""
5757

5858
_SHUTDOWN_WRITE_TIMEOUT: float = 1
5959
"""Tighter bound for the shutdown-arm error write so a wedged transport can't hold session close."""
@@ -232,7 +232,8 @@ def __init__(
232232
message is dequeued (e.g. `initialize`); an inline handler
233233
that awaits the peer deadlocks the parked loop.
234234
on_stream_exception: Observer for `Exception` items on the read
235-
stream; without it they are debug-logged and dropped.
235+
stream; without it they are debug-logged and dropped. Awaited
236+
inline in the read loop, so a slow observer stalls dispatch.
236237
"""
237238
self._read_stream = read_stream
238239
self._write_stream = write_stream
@@ -332,7 +333,7 @@ async def send_raw_request(
332333
_related_request_id,
333334
),
334335
shield=False,
335-
timeout=_SHIELDED_WRITE_TIMEOUT,
336+
timeout=_ABANDON_WRITE_TIMEOUT,
336337
describe=f"courtesy cancel for timed-out request {request_id!r}",
337338
)
338339
raise MCPError(code=REQUEST_TIMEOUT, message=f"Request {method!r} timed out") from None
@@ -343,7 +344,7 @@ async def send_raw_request(
343344
await self._final_write(
344345
partial(self._cancel_outbound, request_id, "caller cancelled", _related_request_id),
345346
shield=True,
346-
timeout=_SHIELDED_WRITE_TIMEOUT,
347+
timeout=_ABANDON_WRITE_TIMEOUT,
347348
describe=f"courtesy cancel for caller-cancelled request {request_id!r}",
348349
)
349350
raise
@@ -382,6 +383,7 @@ async def run(
382383
"""Drive the receive loop until the read stream closes.
383384
384385
`task_status.started()` fires once `send_raw_request` is usable.
386+
Single-shot: once the loop ends the dispatcher stays closed and cannot be restarted.
385387
"""
386388
try:
387389
# LIFO exits: the write stream closes only after the task-group join, so teardown writes still land.

tests/shared/test_jsonrpc_dispatcher.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ async def test_caller_cancel_courtesy_write_is_bounded_when_the_transport_is_wed
727727
caplog: pytest.LogCaptureFixture,
728728
):
729729
"""A wedged transport write cannot turn caller cancellation into an unbounded shielded hang:
730-
`_SHIELDED_WRITE_TIMEOUT` abandons the courtesy-cancel write (SDK-defined bound). On regression
730+
`_ABANDON_WRITE_TIMEOUT` abandons the courtesy-cancel write (SDK-defined bound). On regression
731731
the test hangs rather than failing fast - fail_after cannot cancel through the shield."""
732732
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](0)
733733
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](0)
@@ -745,7 +745,7 @@ async def caller() -> None:
745745
gave_up.set()
746746

747747
try:
748-
# Both bounds exceed the in-loop _SHIELDED_WRITE_TIMEOUT (5s); the virtual clock makes them instant.
748+
# Both bounds exceed the in-loop _ABANDON_WRITE_TIMEOUT (5s); the virtual clock makes them instant.
749749
with anyio.fail_after(30):
750750
async with anyio.create_task_group() as tg: # pragma: no branch
751751
await tg.start(client.run, on_request, on_notify)
@@ -775,7 +775,7 @@ async def test_timeout_courtesy_cancel_write_is_bounded_when_the_transport_is_we
775775
caplog: pytest.LogCaptureFixture,
776776
):
777777
"""A wedged transport write cannot delay the REQUEST_TIMEOUT error indefinitely (SDK-defined
778-
bound): `_SHIELDED_WRITE_TIMEOUT` abandons the courtesy cancel so the error still surfaces."""
778+
bound): `_ABANDON_WRITE_TIMEOUT` abandons the courtesy cancel so the error still surfaces."""
779779
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](0)
780780
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](0)
781781
client: JSONRPCDispatcher[TransportContext] = JSONRPCDispatcher(s2c_recv, c2s_send)
@@ -799,7 +799,7 @@ async def caller() -> None:
799799
request = await c2s_recv.receive()
800800
assert isinstance(request, SessionMessage)
801801
assert isinstance(request.message, JSONRPCRequest)
802-
# Exceeds the request timeout (1s) plus _SHIELDED_WRITE_TIMEOUT (5s); virtual clock, no wall time.
802+
# Exceeds the request timeout (1s) plus _ABANDON_WRITE_TIMEOUT (5s); virtual clock, no wall time.
803803
with anyio.fail_after(10):
804804
await gave_up.wait()
805805
tg.cancel_scope.cancel()
@@ -835,7 +835,7 @@ async def on_notify(ctx: DCtx, method: str, params: Mapping[str, Any] | None) ->
835835
raise NotImplementedError
836836

837837
try:
838-
# 3s sits between _SHUTDOWN_WRITE_TIMEOUT (1s) and _SHIELDED_WRITE_TIMEOUT (5s): pins the tighter bound.
838+
# 3s sits between _SHUTDOWN_WRITE_TIMEOUT (1s) and _ABANDON_WRITE_TIMEOUT (5s): pins the tighter bound.
839839
with anyio.fail_after(3):
840840
async with anyio.create_task_group() as tg: # pragma: no branch
841841
await tg.start(server.run, park, on_notify)

0 commit comments

Comments
 (0)