Skip to content

fix(mcp): wrap LoggingMiddleware.on_message event_logger in try/except#38560

Merged
Antonio-RiveroMartnez merged 8 commits intoapache:masterfrom
aminghadersohi:amin/fix-mcp-logging-middleware-app-context
Mar 10, 2026
Merged

fix(mcp): wrap LoggingMiddleware.on_message event_logger in try/except#38560
Antonio-RiveroMartnez merged 8 commits intoapache:masterfrom
aminghadersohi:amin/fix-mcp-logging-middleware-app-context

Conversation

@aminghadersohi
Copy link
Copy Markdown
Contributor

@aminghadersohi aminghadersohi commented Mar 10, 2026

SUMMARY

After registering LoggingMiddleware in the MCP server startup, the MCP server started failing the initialize handshake with clients (e.g. Claude Code) with:

WARNING:root:Failed to validate request: Working outside of application context.

Root cause: LoggingMiddleware fires on ALL MCP messages including the protocol-level initialize handshake and the on_call_tool() finally block (which runs after mcp_auth_hook pops Flask context). Both on_message() and on_call_tool() call event_logger.log() which requires Flask app context, but:

  1. During initialize there is no Flask app context (mcp_auth_hook only pushes context for tool calls, not protocol messages)
  2. In on_call_tool()'s finally block, the Flask app context has already been popped by mcp_auth_hook

Fix: Use flask.has_app_context() guard check before event_logger.log() calls in both on_call_tool() and on_message(). This is explicit about intent, avoids exception-driven control flow, and won't accidentally swallow unrelated RuntimeErrors.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A - backend fix

TESTING INSTRUCTIONS

  1. Start the MCP server locally: superset mcp run --port 5008 --debug
  2. Connect with an MCP client (e.g. Claude Code /mcp)
  3. Verify the initialize handshake completes successfully (no Working outside of application context error)
  4. Verify all 19 MCP tools work correctly (health_check, list_charts, list_dashboards, list_datasets, get_instance_info, get_schema, get_chart_info, get_dashboard_info, get_dataset_info, execute_sql, open_sql_lab_with_context, generate_explore_link, generate_chart, get_chart_data, get_chart_preview, update_chart, update_chart_preview, generate_dashboard, add_chart_to_existing_dashboard)
  5. Verify tool calls still log properly via event_logger when Flask app context is available

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

LoggingMiddleware.on_message() fires on all MCP messages including the
initialize handshake, but event_logger.log() requires Flask app context
which isn't available during protocol-level messages. This causes the
MCP server to reject the initialize request with "Working outside of
application context".

Wrap the event_logger.log() call with try/except RuntimeError to
gracefully skip logging when no Flask context is available.
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review bot commented Mar 10, 2026

Code Review Agent Run #27be06

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: b30959d..b30959d
    • superset/mcp_service/middleware.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:S This PR changes 10-29 lines, ignoring generated files label Mar 10, 2026
@dosubot dosubot bot added the change:backend Requires changing the backend label Mar 10, 2026
@codeant-ai-for-open-source
Copy link
Copy Markdown
Contributor

Sequence Diagram

This PR updates MCP message logging so that missing Flask application context during the initialize handshake no longer breaks the flow, allowing the handshake to succeed while still logging messages when context is available.

sequenceDiagram
    participant Client
    participant MCPServer
    participant Middleware
    participant EventLogger

    Client->>MCPServer: Send initialize message
    MCPServer->>Middleware: on_message with context
    Middleware->>EventLogger: Log MCP message

    alt Flask context available
        EventLogger-->>Middleware: Log entry recorded
    else No Flask context
        EventLogger-->>Middleware: RuntimeError raised
        Middleware->>Middleware: Catch error and skip logging
    end

    Middleware->>MCPServer: call_next and continue handling
    MCPServer-->>Client: Initialize response success
Loading

Generated by CodeAnt AI

…lask context

The on_call_tool finally block runs event_logger.log() AFTER
mcp_auth_hook pops the Flask app context, causing RuntimeError
that destroys the successful tool result. Wrap all event_logger
calls with try/except RuntimeError.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.44%. Comparing base (bf55f1e) to head (ef63a81).
⚠️ Report is 26 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/middleware.py 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #38560      +/-   ##
==========================================
- Coverage   64.85%   64.44%   -0.42%     
==========================================
  Files        1817     2529     +712     
  Lines       72183   128780   +56597     
  Branches    22975    29677    +6702     
==========================================
+ Hits        46815    82986   +36171     
- Misses      25368    44351   +18983     
- Partials        0     1443    +1443     
Flag Coverage Δ
hive 40.80% <0.00%> (?)
mysql 61.97% <0.00%> (?)
postgres 62.04% <0.00%> (?)
presto 40.81% <0.00%> (?)
python 63.67% <0.00%> (?)
sqlite 61.67% <0.00%> (?)
unit 100.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Replace exception-driven control flow with explicit Flask context check.
This is cleaner because:
- Explicit intent: only log when Flask context exists
- Won't swallow unrelated RuntimeErrors
- Avoids exception-driven control flow anti-pattern
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review bot commented Mar 10, 2026

Code Review Agent Run #4e5e81

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: b30959d..acbfcd6
    • superset/mcp_service/middleware.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@Antonio-RiveroMartnez Antonio-RiveroMartnez merged commit 6d7cfac into apache:master Mar 10, 2026
65 checks passed
bschreder pushed a commit to bschreder/superset that referenced this pull request Mar 11, 2026
michael-s-molina pushed a commit that referenced this pull request Mar 17, 2026
aminghadersohi added a commit to aminghadersohi/superset that referenced this pull request Mar 17, 2026
MallikarjunaReddyN pushed a commit to MallikarjunaReddyN/superset that referenced this pull request Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend size/M size:S This PR changes 10-29 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants