modelcontextprotocol / modelcontextprotocol/python-sdk

ClientSession receive loop swallows callback exceptions, replying "Invalid request parameters" to the server instead of propagating to the call_tool awaiter

Đang mở
#2,673 4 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

enhancement needs decision P3
Ngôn ngữ chính
Python
Star
24.3k
Fork
4k
Merge trung bình
1 ngày 1 giờ
Pull request đã merge (30 ngày)
31

Mô tả

Initial Checks
Description

I hit this while wiring an MCP tool that uses ctx.elicit(...) into a
LangGraph agent — the natural pattern of calling interrupt() from the
client elicitation callback does not work because the callback's exception
is swallowed by _receive_loop and replied to the server as
Invalid request parameters. The same limitation blocks any framework that
relies on exception-based control flow on the awaiter's task (custom
async-cancellation strategies, Trio nursery patterns, retry/backoff
libraries, any human-in-the-loop integration), so a small SDK-level escape
hatch would unblock a broad class of MCP client developers, not just this
one use case.

Any exception raised inside a ClientSession callback
(elicitation_callback, sampling_callback, list_roots_callback) is
caught by the blanket except Exception in BaseSession._receive_loop
(src/mcp/shared/session.py) and turned into a JSON-RPC
Invalid request parameters reply to the server:

except Exception:
    logging.warning("Failed to validate request", exc_info=True)
    error_response = JSONRPCError(
        jsonrpc="2.0",
        id=message.message.id,
        error=ErrorData(code=INVALID_PARAMS, message="Invalid request parameters", data=""),
    )
    await self._write_stream.send(SessionMessage(message=error_response))

Consequences:

  1. The server is blamed for "invalid params" when the failure was on the
    client. The original callback traceback is lost from the awaiter of
    session.call_tool(...).
  2. Callbacks have no way to abort back to the awaiter. Frameworks that use
    exception-based flow control on the caller's task (LangGraph's
    interrupt() for human-in-the-loop pause/resume is the motivating
    case) cannot be wired through MCP without per-framework workarounds.

Expected: an opt-in escape hatch so a callback can raise an exception
that propagates out of the in-flight session.call_tool(...) instead of
becoming a JSON-RPC error to the server. Defaulting opt-in to off preserves
existing behaviour.

Proposed minimal fix (single file, ~30 lines, three coordinated edits):

# src/mcp/shared/session.py
#
# (1) BaseSession.__init__: add a per-session stash for marked exceptions.
self._propagate_errors: dict[RequestId, BaseException] = {}

# (2) Inside _receive_loop's request-handler `except Exception as e:` block,
#     branch on the marker before the existing INVALID_PARAMS path.
if getattr(e, "__mcp_propagate__", False):
    # Notify the peer so their request doesn't hang.
    error_response = JSONRPCError(
        jsonrpc="2.0",
        id=message.message.root.id,
        error=ErrorData(code=INTERNAL_ERROR, message="Handler raised", data=""),
    )
    await self._write_stream.send(SessionMessage(message=JSONRPCMessage(error_response)))
    # Surface to the awaiter of any in-flight outgoing request on this session.
    for in_flight_id, stream in list(self._response_streams.items()):
        self._propagate_errors[in_flight_id] = e
        await stream.aclose()
    continue
# ...existing INVALID_PARAMS path unchanged below this point.

# (3) Inside send_request, alongside the existing `except TimeoutError:`,
#     consume the stash on EndOfStream and re-raise the original exception.
except anyio.EndOfStream:
    propagate = self._propagate_errors.pop(request_id, None)
    if propagate is not None:
        raise propagate from None
    raise

I've already implemented and tested this locally against v1.x (and the
same shape on main for the V2 rework): ~33 lines added to
src/mcp/shared/session.py plus regression tests in
tests/shared/test_session.py. The patch is strictly additive — without the
__mcp_propagate__ marker, behaviour is byte-identical to today. End-to-end
verified by surfacing a LangGraph interrupt() from an elicitation callback
as a normal __interrupt__ on the agent's first invoke. ruff / pyright
clean, existing session tests still pass. Happy to open a PR once the issue
is accepted and you've confirmed the direction.

Example Code
# Self-contained reproduction. No third-party agent framework needed.
#
# Terminal A:  python repro_server.py
# Terminal B:  python repro_client.py
#
# Observed in A: WARNING:root:Failed to validate request: CallbackBoom(...)
# Observed in B: tool result.isError = True, CallbackBoom was NEVER raised
#                on the awaiter — silently converted to INVALID_PARAMS.

# --- repro_server.py ---
from mcp.server.fastmcp import Context, FastMCP
from pydantic import BaseModel

class Answer(BaseModel):
    value: str

server = FastMCP(name="repro", port=8765)

@server.tool()
async def ask(ctx: Context) -> str:
    result = await ctx.elicit(message="hello?", schema=Answer)
    return f"got: {result}"

if __name__ == "__main__":
    server.run(transport="streamable-http")


# --- repro_client.py ---
import asyncio
from mcp import ClientSession
from mcp.client.streamable_http import streamablehttp_client

class CallbackBoom(Exception):
    """Stand-in for any framework's flow-control exception (e.g. GraphInterrupt)."""

async def on_elicit(ctx, params):
    raise CallbackBoom("callback wants to abort back to the awaiter")

async def main():
    async with streamablehttp_client("http://127.0.0.1:8765/mcp") as (r, w, _):
        async with ClientSession(r, w, elicitation_callback=on_elicit) as session:
            await session.initialize()
            try:
                result = await session.call_tool("ask", {})
            except CallbackBoom:
                print("OK: CallbackBoom propagated to the awaiter")
                return
            print("BUG: awaiter did not raise; result.isError =", result.isError)
            print("     content =", result.content)

asyncio.run(main())
Python & MCP Python SDK
Python: 3.13
mcp:    1.27.1

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với src/mcp/shared/session.py, đặc biệt là BaseSession._receive_loop và send_request, sau đó xem lại các bài kiểm thử hồi quy trong tests/shared/test_session.py. Tái hiện hành vi của các ngoại lệ callback và xác minh rằng các ngoại lệ callback được đánh dấu sẽ đến được awaiter của lệnh gọi call_tool đang thực thi, trong khi các ngoại lệ không được đánh dấu vẫn giữ nguyên hành vi phản hồi hiện có.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
python
Lĩnh vực
api, backend-api-design
Loại issue
Lỗi
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
68/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.