Skip to content

Conversation

@jlowin
Copy link
Owner

@jlowin jlowin commented Dec 10, 2025

When exceptions occur during client initialization (e.g., McpError from server middleware), they were being swallowed and converted to "generator didn't yield" errors. This specifically affected in-memory transport where the server lifespan (including Docket's background worker context managers) is nested inside the client's task group - the complex cleanup chain was suppressing exceptions.

Root cause: anyio task groups suppress exceptions when cancel_scope.cancel() is called during cleanup.

Fix: Capture exceptions before task group cleanup and re-raise them after the task group exits cleanly:

exception_to_raise: BaseException | None = None
async with anyio.create_task_group() as tg:
    try:
        async with ClientSession(...) as session:
            yield session
    except BaseException as e:
        exception_to_raise = e
    finally:
        tg.cancel_scope.cancel()

if exception_to_raise is not None:
    raise exception_to_raise

Also preserves McpError type in _connect() so callers can catch protocol-level errors:

async with Client(server) as client:
    ...
# Now raises McpError instead of RuntimeError wrapping McpError

Relevant to fixing #2531

anyio task groups suppress exceptions when cancel_scope.cancel() is
called during cleanup. Capture exceptions before cleanup and re-raise
after task group exits cleanly.

Also preserve McpError type in client _connect() so callers can catch
protocol-level errors specifically.
@marvin-context-protocol marvin-context-protocol bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. client Related to the FastMCP client SDK or client-side functionality. labels Dec 10, 2025
@jlowin jlowin merged commit 95e58e8 into main Dec 10, 2025
14 of 15 checks passed
@jlowin jlowin deleted the fix-exception-propagation-in-transport branch December 10, 2025 20:37
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

The pull request enhances exception handling in the FastMCP client across two files. In client.py, the error handling logic is updated to import and preserve McpError exceptions alongside httpx.HTTPStatusError when session tasks report failures. In transports.py, a new exception capture mechanism is introduced in the connect_session method that catches exceptions during task group execution and re-raises them after cleanup is complete. These changes ensure exceptions are properly propagated to callers rather than being suppressed during task group cancellation.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: preserve exception propagation through transport cleanup' clearly summarizes the main change: fixing exception propagation in transport cleanup logic.
Description check ✅ Passed PR description clearly explains the problem, provides code examples, and lists the changes made. All required sections from the template are addressed.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-exception-propagation-in-transport

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. client Related to the FastMCP client SDK or client-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants