FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix: Handle Exception case in lowlevel server _handle_message by samsonkolge · Pull Request #1200 · modelcontextprotocol/python-sdk · GitHub

Fix: Handle Exception case in lowlevel server _handle_message - #1200

Closed
samsonkolge wants to merge 2 commits into
modelcontextprotocol:mainfrom
samsonkolge:fix/exception-handling-lowlevel-server
Closed

Fix: Handle Exception case in lowlevel server _handle_message#1200
samsonkolge wants to merge 2 commits into
modelcontextprotocol:mainfrom
samsonkolge:fix/exception-handling-lowlevel-server

Conversation

Copy link
Copy Markdown

Fix: Handle Exception case in lowlevel server _handle_message

Problem

The _handle_message method in src/mcp/server/lowlevel/server.py had a critical unhandled case for Exception types. Despite the function signature explicitly accepting Exception as a valid message type, the match statement was missing this case, potentially leading to silent failures in production environments.

Critical TODO was present:

# TODO(Marcelo): We should be checking if message is Exception here.
match message:  # type: ignore[reportMatchNotExhaustive]

Solution

🔧 Fixed the unhandled Exception case by:

  1. Added proper Exception handling in the match statement:

    • Added case Exception() as error: to handle Exception instances
    • Used logger.exception() following CLAUDE.md guidelines for proper exception logging with traceback
    • Respected the raise_exceptions parameter to maintain backward compatibility
  2. Removed the TODO comment and type ignore directive as the issue is now resolved

  3. Added comprehensive test coverage in tests/server/test_exception_handling.py:

    • Test for proper exception logging when raise_exceptions=False
    • Test for exception re-raising when raise_exceptions=True
    • Test to ensure exceptions don't crash the server when handled gracefully

Files Changed

  • src/mcp/server/lowlevel/server.py - Fixed Exception handling in _handle_message
  • tests/server/test_exception_handling.py - Added comprehensive test coverage

Commit Info

  • Branch: fix/exception-handling-lowlevel-server
  • Files changed: 2 files, 126 insertions(+), 2 deletions(-)

Reviewers: @jerome3o-anthropic @jspahrsummers

This PR resolves the most critical outstanding TODO in the codebase and significantly improves the robustness of the MCP server implementation.

- Add missing Exception case in _handle_message match statement
- Use logger.exception() following CLAUDE.md guidelines
- Respect raise_exceptions parameter for backward compatibility
- Add comprehensive tests for exception handling scenarios
- Remove TODO comment as issue is resolved

This fixes a critical gap where Exception instances passed to _handle_message
were not properly handled, potentially causing silent failures in production.
samsonkolge requested review from a team and ihrpr July 26, 2025 09:14

Copy link
Copy Markdown
Contributor

Closing this PR as this is a duplicate of #786

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL