Contain a raising progress callback on the in-process dispatcher

[maxisbey]

Fixes #3434.

What was wrong

With the in-process Client(server) connection, a progress_callback that raised took the server's tool down with it. report_progress re-raised the client's exception inside the tool, so call_tool came back as an error blaming the tool. Over stdio and HTTP, and with mode="legacy", the same callback failure is logged and the tool finishes.

What changes

• DirectDispatcher catches an exception raised by the caller's on_progress callback and logs it as progress callback raised, the same message JSONRPCDispatcher uses.
• The handler carries on and the request returns its normal result.
• Only Exception is caught, so cancellation and request timeouts still propagate.
• No names, parameters or defaults change, and nothing changes on the wire.

What users will notice

• In-process Client(server) in any mode other than "legacy": a progress_callback that raises no longer fails the call. The traceback goes to the mcp.shared.direct_dispatcher logger.
• Tests that relied on a raising or asserting progress callback failing the call will now pass and log instead.
  • With MCPServer, the failure used to be an is_error result naming the tool.
  • With a lowlevel Server, it used to reach the caller as an MCPError.
• Other connections are unchanged; they already behaved this way.

Not included

• DirectDispatcher.notify() still propagates an exception raised by the peer's notification handler.
  • ClientSession and the server runner contain raising notification callbacks at their own layer, so this is only reachable from a hand-wired dispatcher pair with its own raising handler.

How it was checked

• The JSON-RPC-only test of this behaviour moved into the dispatcher contract tests in tests/shared/test_dispatcher.py, so it runs against both dispatchers.
  • The direct case fails on main and passes with the change.
  • The jsonrpc case passes on both.
• The scenario from the issue returns done in the default mode, with mode="2026-07-28" and with mode="legacy".
• A progress callback still blocked when the request timeout fires gives REQUEST_TIMEOUT, and cancelling the caller still cancels the request.
• ./scripts/test passes with 100% coverage; ruff and pyright are clean.

_{AI Disclaimer}

[cubic-dev-ai]

No issues found across 3 files

_{Re-trigger cubic}

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked whether the moved contract test could flake on the jsonrpc parametrization (callback spawned as a task, caplog.text read after the response) — it is the same body as the test it replaces in tests/shared/test_jsonrpc_dispatcher.py, which already ran this way, and the progress notification is dispatched before the response message is dequeued, so nothing new is introduced. Also confirmed the catch is Exception only, so anyio cancellation (a BaseException) still propagates through ctx.progress.

Extended reasoning...

The change is a four-line try/except in _DirectDispatchContext.progress in src/mcp/shared/direct_dispatcher.py plus a test moved from the JSON-RPC-only file into the shared dispatcher contract tests; no security-sensitive surface is touched. Inline findings (undocumented behaviour change, bare except Exception: convention, and the sibling uncontained ctx.notify path) remain for the author, so this is not an approval.

One optional note from this repository's REVIEW.md or CLAUDE.md checks was not posted as a comment, over this review's limit for such notes; it is on this commit's check card.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/mcp/shared/direct_dispatcher.py — Server handlers running over an in-process DirectDispatcher have their own request failed when the peer's notification handler raises, because ctx.notify at src/mcp/shared/direct_dispatcher.py:79 awaits _back_notify inline with no containment. The PR fixes the identical pattern for ctx.progress three lines later (lines 93-96) but not here. Fix: contain a raising peer on_notify at the dispatcher boundary (either in _DirectDispatchContext.notify or in _dispatch_notify at line 307) with try/except Exception + logger.exception, matching _contained_notify in JSONRPCDispatcher.

  Why this was flagged

  A request handler on one side of a DirectDispatcher pair calls ctx.notify(...) (src/mcp/shared/direct_dispatcher.py:78-79); this awaits peer._dispatch_notify (line 210, 295-307) which awaits the peer's on_notify inline. If that on_notify raises an Exception, it propagates up through line 79 into the handler, and _dispatch_request at line 276-284 converts it to an MCPError(INTERNAL_ERROR) blaming the request handler, exactly the shape of bug #3434 for progress. Over JSONRPCDispatcher the notification is spawned under _contained_notify (src/mcp/shared/jsonrpc_dispatcher.py:659) and the handler's request succeeds. In this checkout the only SDK-wired on_notify callables (ClientSession._on_notify at src/mcp/client/session.py:1482-1491 and ServerRunner at src/mcp/server/runner.py:284-289) contain their own exceptions, so the exposed population is any user or test that wires its own on_notify to the public dispatcher. Pre-existing on the base commit; same root mechanism as L3 but observed from the sending request handler's side.

  Verification: Pre-existing; acknowledged in diff. _DirectDispatchContext.notify at src/mcp/shared/direct_dispatcher.py:78-79 is await self._back_notify(method, params) with no try/except. test_notification_handler_exception_is_contained runs against the jsonrpc pair only. Line 79 is untouched by this diff; the base branch already fails the same way for the hand-wired case.

[claude]

🟡 nit (optional): maintainers get a new bare except Exception: in library code, which AGENTS.md forbids outside top-level handlers. The catch at src/mcp/shared/direct_dispatcher.py:95 contains a caller-supplied on_progress callback, so narrowing is not practical, but nothing marks it as a boundary. Fix: keep the broad catch only as an explicitly documented containment boundary, e.g. a one-line comment or docstring saying the user callback is isolated here, matching the docstring _shielded_progress carries at src/mcp/shared/jsonrpc_dispatcher.py:201, so the exemption is visible to readers and linters. [also at: src/mcp/shared/direct_dispatcher.py:95 - nit: AGENTS.md forbids except Exception: outside top-level handlers: _DirectDispatchContext.progress() now wraps the caller's on_progress in a bare except Exception: inside a context method that sits mid-stack in the dispatcher, not in a top-level handler.]

Why this was flagged

The diff adds try: await self._on_progress(...) except Exception: logger.exception("progress callback raised") at src/mcp/shared/direct_dispatcher.py:93-96, inside _DirectDispatchContext.progress, which is a mid-stack method called from the server handler's task, not a top-level handler. AGENTS.md states FORBIDDEN except Exception: - unless in top-level handlers. The equivalent JSON-RPC code at src/mcp/shared/jsonrpc_dispatcher.py:200-209 wraps the same catch in _shielded_progress with a docstring explaining why the boundary exists; the direct version has no such marker, so a reader cannot tell this is the sanctioned boundary case rather than an accidental broad catch. Nothing breaks at runtime; the logging form (logger.exception, no exception text in the message) complies with the convention. The base branch had no catch here at all, so this is new code the instruction applies to.

Verification: nit. Triggering condition: any maintainer reading the new catch. AGENTS.md:166: "- FORBIDDEN except Exception: - unless in top-level handlers". The diff adds at src/mcp/shared/direct_dispatcher.py:93-96 except Exception: inside _DirectDispatchContext.progress, not a top-level handler. Unlike _shielded_progress at jsonrpc_dispatcher.py:200-207, the new catch carries no boundary marker.

[claude]

🟡 nit (optional): users reading the docs will not learn that a raising progress_callback is now logged and the call still succeeds, even though the PR itself lists this as a user-visible change for in-process Client(server). AGENTS.md asks that user-visible behaviour changes update the relevant docs/ page in the same PR, and no page is touched. Fix: add a line to docs/handlers/progress.md (the info box at docs/handlers/progress.md:51-56 already contrasts in-process and wire delivery) stating that a callback exception is logged under the dispatcher logger and never fails call_tool, on every connection kind. [also at: src/mcp/shared/direct_dispatcher.py:96 - nit: AGENTS.md asks that a user-visible behaviour change update the relevant docs/ page in the same PR: the PR describes a user-visible change (in-process Client(server): a raising progress_callback no longer fails the call, traceback goes to the mcp.shared.direct_dispatcher logger) but touches no docs.]

Why this was flagged

The change at src/mcp/shared/direct_dispatcher.py:93-96 alters observable behaviour for in-process Client(server) users: a progress_callback that raises used to surface as an error on call_tool (via the handler's except Exception at src/mcp/shared/direct_dispatcher.py:276-284 on the base) and now is logged and swallowed. The PR description itself lists this under what users will notice. AGENTS.md:144-145 requires the relevant docs/ page to be updated in the same PR for user-visible behaviour changes. The diff touches no file under docs/; docs/handlers/progress.md describes callback timing for in-process vs wire connections at lines 51-56 but says nothing about how a raising callback is treated, so readers have no documented contract for it. Nothing fails at runtime; this is the repository's documentation instruction not being followed.

Verification: nit. A reader of docs/handlers/progress.md wants to know what happens when their progress_callback raises on an in-process Client(server). src/mcp/shared/direct_dispatcher.py:93-96 now wraps await self._on_progress(...) in try/except Exception, whereas on the base the raise propagated into MCPError(INTERNAL_ERROR, ...). Nothing under docs/ changed, though AGENTS.md:144-145 requires it.