fix(mcp,langchain): Add mechanism to captured exceptions - #7226
fix(mcp,langchain): Add mechanism to captured exceptions#7226gmassello wants to merge 2 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.
| assert len(error_events) == 1 | ||
| assert error_events[0]["exception"]["values"][0]["type"] == "ValueError" | ||
| assert error_events[0]["exception"]["values"][0]["mechanism"]["type"] == "langchain" | ||
| assert not error_events[0]["exception"]["values"][0]["mechanism"]["handled"] |
There was a problem hiding this comment.
Missing LangChain version skip
Medium Severity
test_langchain_tool_error uses create_agent, which is only imported for LangChain 1.0+, but it lacks the @pytest.mark.skipif(LANGCHAIN_VERSION < (1,), ...) guard that sibling tests such as test_langchain_create_agent and test_tool_execution_span already apply. On tox envs still in the matrix (langchain-base-v0.1.20, v0.3.30), collection succeeds while the test raises NameError at runtime.
Reviewed by Cursor Bugbot for commit f5fcb50. Configure here.


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.