(superset) 01/01: test(mcp): pin mcp_tool_error event actually reaches the event logger (#42579)

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch tdd/issue-42579-mcp-error-event-logging-crash
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 8783e4c1d2e971ff09f6f54ee6bc7b2ed2fd3448
Author: Claude Code <no...@anthropic.com>
AuthorDate: Mon Aug 3 16:00:56 2026 -0700

    test(mcp): pin mcp_tool_error event actually reaches the event logger (#42579)
    
    GlobalErrorHandlerMiddleware._handle_error calls event_logger.log() without
    the dashboard_id/slice_id/referrer arguments DBEventLogger.log() requires
    (no defaults), so the call raises TypeError on every MCP tool error,
    silently swallowed by the surrounding except Exception. Every other test in
    this file patches event_logger with a bare MagicMock, which accepts any
    kwargs and would never catch this -- this test uses the real DBEventLogger
    (with DB session calls stubbed out) to actually exercise the broken
    argument binding. Red: fails today with the exact TypeError from the issue
    report.
---
 tests/unit_tests/mcp_service/test_middleware.py | 38 +++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tests/unit_tests/mcp_service/test_middleware.py b/tests/unit_tests/mcp_service/test_middleware.py
index 3f0957fd9b8..c0c0f174317 100644
--- a/tests/unit_tests/mcp_service/test_middleware.py
+++ b/tests/unit_tests/mcp_service/test_middleware.py
@@ -44,6 +44,7 @@ from superset.mcp_service.middleware import (
     RBACToolVisibilityMiddleware,
     ResponseSizeGuardMiddleware,
 )
+from superset.utils.log import DBEventLogger
 
 
 class TestResponseSizeGuardMiddleware:
@@ -1491,6 +1492,43 @@ class TestGlobalErrorHandlerLogLevels:
         mock_logger.warning.assert_called()
         mock_logger.error.assert_not_called()
 
+    @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."""