You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/08/05 23:22:16 UTC
(superset) branch master updated: fix(mcp): supply required DBEventLogger args so mcp_tool_error events are logged (#42579) (#42730)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3f011edabba fix(mcp): supply required DBEventLogger args so mcp_tool_error events are logged (#42579) (#42730)
3f011edabba is described below
commit 3f011edabba499ce8cb8a0c49eed25fef88f67f5
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Wed Aug 5 16:22:10 2026 -0700
fix(mcp): supply required DBEventLogger args so mcp_tool_error events are logged (#42579) (#42730)
Co-authored-by: Claude Code <no...@anthropic.com>
---
superset/mcp_service/middleware.py | 15 ++++++++++
tests/unit_tests/mcp_service/test_middleware.py | 38 +++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/superset/mcp_service/middleware.py b/superset/mcp_service/middleware.py
index 61ec04f1b87..9fbb5f3f9a2 100644
--- a/superset/mcp_service/middleware.py
+++ b/superset/mcp_service/middleware.py
@@ -912,7 +912,10 @@ class GlobalErrorHandlerMiddleware(Middleware):
event_logger.log(
user_id=user_id,
action="mcp_tool_error",
+ dashboard_id=None,
duration_ms=duration_ms,
+ slice_id=None,
+ referrer=None,
curated_payload={
"tool": tool_name,
"error_type": type(error).__name__,
@@ -1179,6 +1182,10 @@ class ResponseSizeGuardMiddleware(Middleware):
event_logger.log(
user_id=user_id,
action="mcp_response_truncated",
+ dashboard_id=None,
+ duration_ms=None,
+ slice_id=None,
+ referrer=None,
curated_payload={
"tool": tool_name,
"original_tokens": estimated_tokens,
@@ -1251,6 +1258,10 @@ class ResponseSizeGuardMiddleware(Middleware):
event_logger.log(
user_id=user_id,
action="mcp_response_truncated",
+ dashboard_id=None,
+ duration_ms=None,
+ slice_id=None,
+ referrer=None,
curated_payload={
"tool": tool_name,
"original_tokens": estimated_tokens,
@@ -1315,6 +1326,10 @@ class ResponseSizeGuardMiddleware(Middleware):
event_logger.log(
user_id=user_id,
action="mcp_response_size_exceeded",
+ dashboard_id=None,
+ duration_ms=None,
+ slice_id=None,
+ referrer=None,
curated_payload={
"tool": tool_name,
"estimated_tokens": estimated_tokens,
diff --git a/tests/unit_tests/mcp_service/test_middleware.py b/tests/unit_tests/mcp_service/test_middleware.py
index d3a49866695..28291f112fc 100644
--- a/tests/unit_tests/mcp_service/test_middleware.py
+++ b/tests/unit_tests/mcp_service/test_middleware.py
@@ -45,6 +45,7 @@ from superset.mcp_service.middleware import (
ResponseSizeGuardMiddleware,
StructuredContentStripperMiddleware,
)
+from superset.utils.log import DBEventLogger
class TestResponseSizeGuardMiddleware:
@@ -1525,6 +1526,43 @@ class TestGlobalErrorHandlerLogLevels:
assert "Internal error" not in str(exc.value)
+ @pytest.mark.asyncio
+ async def test_error_event_reaches_the_real_event_logger(self) -> None:
+ """
+ Regression for #42579: ``_handle_error`` calls
+ ``event_logger.log(user_id=..., action=..., duration_ms=...,
+ curated_payload=...)`` without the ``dashboard_id``/``slice_id``/
+ ``referrer`` arguments ``DBEventLogger.log`` requires (they have no
+ defaults), so the call raises ``TypeError`` on every single MCP
+ error, silently swallowed by the surrounding ``except Exception``.
+
+ Every other test in this class patches ``event_logger`` with a bare
+ ``MagicMock``, which accepts any kwargs and would never catch this --
+ this test uses the real ``DBEventLogger`` instead (with the DB
+ session calls stubbed out, so it doesn't need a live database) to
+ actually exercise the argument binding that's broken today.
+ """
+ middleware = GlobalErrorHandlerMiddleware()
+
+ context = MagicMock()
+ context.message.name = "execute_sql"
+ context.method = "tools/call"
+
+ call_next = AsyncMock(side_effect=ValueError("bad param"))
+
+ with (
+ patch("superset.mcp_service.middleware.get_user_id", return_value=1),
+ patch("superset.mcp_service.middleware.event_logger", DBEventLogger()),
+ patch("superset.db") as mock_db,
+ pytest.raises(ToolError),
+ ):
+ await middleware.on_message(context, call_next)
+
+ # If event_logger.log() actually bound its arguments successfully,
+ # it would go on to persist a Log row. Today the TypeError is raised
+ # (and swallowed) before it ever gets that far.
+ mock_db.session.bulk_save_objects.assert_called_once()
+
class TestRBACToolVisibilityMiddleware:
"""Tests for RBACToolVisibilityMiddleware.on_list_tools."""