fix: add raise ... from e in all except handlers#2777
Open
kiankhooban wants to merge 2 commits into
Open
Conversation
Preserves exception chaining so the original traceback is always visible when exceptions are re-raised inside except blocks. Fixes 11 sites across 6 files — follow-up to modelcontextprotocol#2542.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Improve debuggability by preserving original exceptions when re-raising errors across resources, prompts, and client/auth flows.
Changes:
- Add explicit exception chaining (
raise ... from e) to preserve original tracebacks. - Apply the same pattern across resource reading/creation, prompt rendering, tool result validation, and OAuth response parsing.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mcp/server/mcpserver/resources/types.py | Chain underlying exceptions for resource/file/directory read/list errors. |
| src/mcp/server/mcpserver/resources/templates.py | Chain underlying exceptions when template-based resource creation fails. |
| src/mcp/server/mcpserver/resources/resource_manager.py | Chain underlying exceptions when resolving resources via templates fails. |
| src/mcp/server/mcpserver/prompts/base.py | Chain underlying exceptions when prompt rendering fails. |
| src/mcp/client/session.py | Chain schema/validation exceptions when validating tool structured output. |
| src/mcp/client/auth/utils.py | Chain validation exceptions when parsing OAuth registration/token responses fails. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+320
to
+321
| except SchemaError as e: # pragma: no cover | ||
| raise RuntimeError(f"Invalid schema for tool {name}: {e}") # pragma: no cover | ||
| raise RuntimeError(f"Invalid schema for tool {name}: {e}") from e # pragma: no cover |
| return client_info | ||
| except ValidationError as e: # pragma: no cover | ||
| raise OAuthRegistrationError(f"Invalid registration response: {e}") | ||
| raise OAuthRegistrationError(f"Invalid registration response: {e}") from e |
| return token_response | ||
| except ValidationError as e: # pragma: no cover | ||
| raise OAuthTokenError(f"Invalid token response: {e}") | ||
| raise OAuthTokenError(f"Invalid token response: {e}") from e |
- Remove duplicate pragma: no cover from raise line in session.py
- Strip {e} from OAuth error messages to avoid leaking token data;
the chained exception already carries full details for debugging
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2542.
Adds
from eto all remainingraisestatements insideexcept ... as e:blocks that were missing exception chaining. This ensures the original traceback is preserved when exceptions are re-raised, making debugging significantly easier.Files changed (11 sites across 6 files):
src/mcp/client/session.py(2)src/mcp/client/auth/utils.py(2)src/mcp/server/mcpserver/resources/types.py(4)src/mcp/server/mcpserver/resources/resource_manager.py(1)src/mcp/server/mcpserver/resources/templates.py(1)src/mcp/server/mcpserver/prompts/base.py(1)