fix(mcp,langchain): Add mechanism to captured exceptions - #7226
Open
gmassello wants to merge 3 commits into
Open
fix(mcp,langchain): Add mechanism to captured exceptions#7226gmassello wants to merge 3 commits into
gmassello wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f5fcb50. Configure here.
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.

Fixes #5242
What
Errors captured by the MCP and LangChain instrumentation reach Sentry with the
default mechanism
{"type": "generic", "handled": True}.The issue title says the mechanism is missing. It isn't — it's wrong, in two
separate ways:
type: "generic"— the error isn't attributed to the integration thatcaptured it, which is what the issue asks for: "so capture data is
available, and the popularity of integrations can be determined."
handled: True— all six MCP call sites re-raise (mcp.pylines 398,494, 619, 768, 923, 988), so the exception reaches the user's code. Marking
it handled keeps these errors out of the unhandled-issue signals and out of
the crash-free rate. This part isn't mentioned in the issue.
How
A module-level
_capture_exceptionhelper in each integration, following thepattern already used by the other eight AI integrations (
openai.py:151,anthropic.py:202,cohere.py:86,huggingface_hub.py:66,google_genai/utils.py:157,openai_agents/utils.py:42,pydantic_ai/utils.py:253, and inline inlitellm.py:311). The seven capturesites route through it.
handled=Falseat all seven: the MCP sites re-raise, and the LangChain_handle_errorcallback is a notification point, not a swallow point — theexisting test wraps the call in
pytest.raises(ValueError).flask.py:240sets the precedent for errors a framework later turns into a response. I did
not add a
handledparameter: the onlyhandled=Trueamong the AIintegrations (
pydantic_ai/patches/tools.py:93,166) is gated behind an optionthat has no equivalent here.
The MCP call sites are wrapped in
capture_internal_exceptions().Scope.capture_exceptionwrapped itscapture_eventcall intry/except → capture_internal_exception(scope.py:1593-1596); the helperdrops that, and the sibling integrations restore it at the call site
(
openai.py:845-846). Without it, a failure inside the SDK would replace theuser's exception, which the integration contract forbids.
langchain.pydoesn't need it —
_handle_erroralready runs inside that context manager.Tests
Assertions added to the error tests that already existed, plus one new test.
test_langchain_tool_errorcoverson_tool_error(langchain.py:783), whichreaches the capture but no test exercised — the only tool in the file
never raises. Coverage confirms it hits
on_tool_errorand the capture atlangchain.py:296, and neitheron_llm_errornoron_chat_model_error.Coverage over the six MCP call sites, measured rather than assumed:
_tool_handler_wrapper_instrument_v2_tool_call_prompt_handler_wrapper_instrument_v2_prompt_get_resource_handler_wrapper_instrument_v2_resource_readv1 and v2 are mutually exclusive paths, so both versions are needed to cover
all six.
Suites run green:
mcp-v1.29.0(100),mcp-v2.0.0(105),langchain-base-v1.3.14(626),langgraph-v0.6.11(146),fastmcp-v1.0(88),fastmcp-v4.0.0b2(42).mypy sentry_sdkclean,ruffclean.test_graph_bubble_up_ignored(langgraph) still passes: the_ignored_exceptionsbranch never reaches the capture, so ignored exceptionsstill produce no event.
Two things found along the way, both out of scope here
FastMCP v4 bypasses the MCP instrumentation. With
fastmcp==4.0.0b2+mcp==2.0.0, a failing tool never reaches any of the six capture sites(measured: zero executed). fastmcp catches and logs it itself, and the only
reason Sentry sees the error is
LoggingIntegrationpicking up thatlogger.error— the event arrives withmechanism.type == "logging". Thisalso means
test_fastmcp_tool_with_erroris not currently testing MCP errorcapture on that env, and its
assert len(error_events) >= 1hides it. I lefttest_fastmcp.pyuntouched for that reason: adding the mechanism assertionsthere would fail permanently on that env for an unrelated cause.
The integration's patches fire when it's disabled.
MCPIntegration.setup_oncepatches
Server.call_tool/get_prompt/read_resource, theServer.__init__middleware and
StreamableHTTPServerTransport.handle_requestglobally andpermanently per process, and
get_integration(MCPIntegration)is only checkedafter the
try/except(e.g.mcp.py:404,:500,:625). So errors arecaptured even when the integration isn't enabled. This PR is neutral on
that —
sentry_sdk.capture_exception(e)was equally unguarded, only themechanism payload changes. A real fix moves a guard to the top of all six
wrappers, which also changes span emission; happy to open a separate issue.
Note on process
CONTRIBUTING asks to discuss the approach with a maintainer first. I wrote this
for the DEV Bug Smash challenge and its deadline didn't allow for that, so I'm
opening it as a draft and left a comment on the issue. Happy to close it or
rework it if the approach doesn't fit.