Skip to content

Commit 200a607

Browse files
committed
feat: JSONRPCDispatcher exception boundary (chunk c)
_handle_request is now the single exception-to-wire boundary: - MCPError -> JSONRPCError(e.error) - pydantic ValidationError -> INVALID_PARAMS - Exception -> INTERNAL_ERROR(str(e)), logged, optionally re-raised - outer-cancel (run() TG shutdown) -> shielded REQUEST_CANCELLED write, re-raise - peer-cancel (notifications/cancelled) -> scope swallows, no response written dctx.close() runs in an inner finally so the back-channel shuts the moment the handler exits. _write_result/_write_error swallow Broken/ClosedResourceError so a dropped connection during the response write doesn't crash the dispatcher. All 22 contract tests now pass against both DirectDispatcher and JSONRPCDispatcher; chunk-c xfail markers removed.
1 parent 136a22d commit 200a607

3 files changed

Lines changed: 52 additions & 30 deletions

File tree

src/mcp/shared/jsonrpc_dispatcher.py

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import anyio
3030
import anyio.abc
3131
from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream
32+
from pydantic import ValidationError
3233

3334
from mcp.shared._stream_protocols import ReadStream, WriteStream
3435
from mcp.shared.dispatcher import CallOptions, Dispatcher, OnNotify, OnRequest, ProgressFnT
@@ -42,6 +43,9 @@
4243
from mcp.shared.transport_context import TransportContext
4344
from mcp.types import (
4445
CONNECTION_CLOSED,
46+
INTERNAL_ERROR,
47+
INVALID_PARAMS,
48+
REQUEST_CANCELLED,
4549
REQUEST_TIMEOUT,
4650
ErrorData,
4751
JSONRPCError,
@@ -473,17 +477,43 @@ async def _handle_request(
473477
) -> None:
474478
"""Run ``on_request`` for one inbound request and write its response.
475479
476-
Chunk (b): happy-path only. The full exception-to-wire boundary
477-
(MCPError, ValidationError, INTERNAL_ERROR scrubbing, peer-cancel
478-
no-response) lands in chunk (c).
480+
This is the single exception-to-wire boundary: handler exceptions are
481+
caught here and serialized to ``JSONRPCError``. Nothing above this in
482+
the stack constructs wire errors.
479483
"""
480484
try:
481485
with scope:
482-
result = await on_request(dctx, req.method, req.params)
483-
await self._write(JSONRPCResponse(jsonrpc="2.0", id=req.id, result=result))
486+
try:
487+
result = await on_request(dctx, req.method, req.params)
488+
finally:
489+
# Close the back-channel the moment the handler exits
490+
# (success or raise), before the response write — a handler
491+
# spawning detached work that later calls
492+
# `dctx.send_request()` should see `NoBackChannelError`.
493+
dctx.close()
494+
await self._write_result(req.id, result)
495+
# Peer-cancel: `_dispatch_notification` cancelled this scope. anyio
496+
# swallows a scope's *own* cancel at __exit__, so the result write
497+
# (or the handler) is interrupted and execution lands here without
498+
# reaching the `except cancelled` arm below. Spec SHOULD: send no
499+
# response — fall through to `finally`.
500+
except anyio.get_cancelled_exc_class():
501+
# Outer-cancel: run()'s task group is shutting down. Any bare
502+
# `await` here re-raises immediately, so shield the courtesy write.
503+
with anyio.CancelScope(shield=True):
504+
await self._write_error(req.id, ErrorData(code=REQUEST_CANCELLED, message="Request cancelled"))
505+
raise
506+
except MCPError as e:
507+
await self._write_error(req.id, e.error)
508+
except ValidationError as e:
509+
await self._write_error(req.id, ErrorData(code=INVALID_PARAMS, message=str(e)))
510+
except Exception as e:
511+
logger.exception("handler for %r raised", req.method)
512+
await self._write_error(req.id, ErrorData(code=INTERNAL_ERROR, message=str(e)))
513+
if self._raise_handler_exceptions:
514+
raise
484515
finally:
485516
self._in_flight.pop(req.id, None)
486-
dctx.close()
487517

488518
def _allocate_id(self) -> int:
489519
self._next_id += 1
@@ -492,6 +522,18 @@ def _allocate_id(self) -> int:
492522
async def _write(self, message: JSONRPCMessage, metadata: MessageMetadata = None) -> None:
493523
await self._write_stream.send(SessionMessage(message=message, metadata=metadata))
494524

525+
async def _write_result(self, request_id: RequestId, result: dict[str, Any]) -> None:
526+
try:
527+
await self._write(JSONRPCResponse(jsonrpc="2.0", id=request_id, result=result))
528+
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
529+
logger.debug("dropped result for %r: write stream closed", request_id)
530+
531+
async def _write_error(self, request_id: RequestId, error: ErrorData) -> None:
532+
try:
533+
await self._write(JSONRPCError(jsonrpc="2.0", id=request_id, error=error))
534+
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
535+
logger.debug("dropped error for %r: write stream closed", request_id)
536+
495537
async def _cancel_outbound(self, request_id: RequestId, reason: str) -> None:
496538
try:
497539
await self.notify("notifications/cancelled", {"requestId": request_id, "reason": reason})

tests/shared/conftest.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,4 @@ def pair_factory(request: pytest.FixtureRequest) -> PairFactory:
5858
return request.param
5959

6060

61-
def xfail_jsonrpc_chunk_c(request: pytest.FixtureRequest, factory: PairFactory) -> None:
62-
"""Apply a strict xfail when running against the JSON-RPC dispatcher.
63-
64-
Use for contract tests that require `_handle_request`'s exception boundary
65-
(PR2 chunk c). Remove once that lands.
66-
"""
67-
if factory is jsonrpc_pair:
68-
request.applymarker(
69-
pytest.mark.xfail(strict=True, reason="needs JSONRPCDispatcher._handle_request exception boundary")
70-
)
71-
72-
73-
__all__ = ["PairFactory", "direct_pair", "jsonrpc_pair", "xfail_jsonrpc_chunk_c"]
61+
__all__ = ["PairFactory", "direct_pair", "jsonrpc_pair"]

tests/shared/test_dispatcher.py

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from mcp.shared.transport_context import TransportContext
2020
from mcp.types import INTERNAL_ERROR, INVALID_PARAMS, INVALID_REQUEST, REQUEST_TIMEOUT
2121

22-
from .conftest import PairFactory, direct_pair, xfail_jsonrpc_chunk_c
22+
from .conftest import PairFactory, direct_pair
2323

2424

2525
class Recorder:
@@ -82,11 +82,7 @@ async def test_send_raw_request_returns_result_from_peer_on_request(pair_factory
8282

8383

8484
@pytest.mark.anyio
85-
async def test_send_raw_request_reraises_mcperror_from_handler_unchanged(
86-
pair_factory: PairFactory, request: pytest.FixtureRequest
87-
):
88-
xfail_jsonrpc_chunk_c(request, pair_factory)
89-
85+
async def test_send_raw_request_reraises_mcperror_from_handler_unchanged(pair_factory: PairFactory):
9086
async def on_request(
9187
ctx: DispatchContext[TransportContext], method: str, params: Mapping[str, Any] | None
9288
) -> dict[str, Any]:
@@ -140,11 +136,7 @@ async def server_on_request(
140136

141137

142138
@pytest.mark.anyio
143-
async def test_ctx_send_raw_request_raises_nobackchannelerror_when_transport_disallows(
144-
pair_factory: PairFactory, request: pytest.FixtureRequest
145-
):
146-
xfail_jsonrpc_chunk_c(request, pair_factory)
147-
139+
async def test_ctx_send_raw_request_raises_nobackchannelerror_when_transport_disallows(pair_factory: PairFactory):
148140
async def server_on_request(
149141
ctx: DispatchContext[TransportContext], method: str, params: Mapping[str, Any] | None
150142
) -> dict[str, Any]:

0 commit comments

Comments
 (0)