fix(mcp): stop masking dashboard lookup DB errors as not-found

[aminghadersohi]

SUMMARY

update_dashboard's _find_and_authorize_dashboard helper caught DashboardNotFoundError and SQLAlchemyError in the same except clause, returning a DashboardNotFound response for both. A real database/infrastructure failure during the dashboard lookup was therefore misreported to the caller as "dashboard not found," with zero server-side logging of the underlying error — invisible to operators and misleading to the MCP client.

This splits the handling:

• DashboardNotFoundError still returns the existing DashboardNotFound structured error.
• SQLAlchemyError is now logged server-side via logger.exception and returns a distinct DatabaseError with a generic message. The raw exception text is never included in the response, since str(exc) on a SQLAlchemyError can leak table/column/constraint names — same leak-avoidance pattern already used in generate_dashboard.py.

Audited the rest of the dashboard domain (add_chart_to_existing_dashboard.py, remove_chart_from_dashboard.py, duplicate_dashboard.py, manage_native_filters.py) and the wider mcp_service package for the same "SQLAlchemyError collapsed into not-found" pattern; no other occurrence exists.

BEFORE/AFTER

Before: A transient DB error during update_dashboard's lookup returned {"error": "Dashboard not found: ...", "error_type": "DashboardNotFound"} with no log line anywhere.

After: The same failure returns {"error": "Failed to look up dashboard due to a database error.", "error_type": "DatabaseError"} and is logged server-side via logger.exception (with traceback) for observability.

TESTING INSTRUCTIONS

• pytest tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py -q — 15 passed, including a new test asserting the DB-error path returns error_type="DatabaseError", never leaks the raw exception text, and calls logger.exception.
• pytest tests/unit_tests/mcp_service/dashboard/ -q — 265 passed (no regressions).

ADDITIONAL INFORMATION

• Has associated tests

[codecov]

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.74%. Comparing base (0ff52d4) to head (d28c894).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...set/mcp_service/dashboard/tool/update_dashboard.py	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #41919      +/-   ##
==========================================
- Coverage   64.74%   64.74%   -0.01%     
==========================================
  Files        2738     2738              
  Lines      152950   152953       +3     
  Branches    35055    35055              
==========================================
  Hits        99032    99032              
- Misses      52018    52021       +3     
  Partials     1900     1900              

Flag	Coverage Δ
hive	38.98% <0.00%> (-0.01%)	⬇️
mysql	57.93% <0.00%> (-0.01%)	⬇️
postgres	57.98% <0.00%> (-0.01%)	⬇️
presto	40.95% <0.00%> (-0.01%)	⬇️
python	59.37% <0.00%> (-0.01%)	⬇️
sqlite	57.59% <0.00%> (-0.01%)	⬇️
unit	100.00% <ø> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

[Copilot]

Pull request overview

This PR fixes error classification/observability for the update_dashboard MCP tool by ensuring database/infrastructure failures during dashboard lookup are not misreported as “not found”, and are instead logged server-side with a distinct structured error response.

Changes:

• Split _find_and_authorize_dashboard exception handling so DashboardNotFoundError returns DashboardNotFound, while SQLAlchemyError logs via logger.exception and returns DatabaseError with a generic message.
• Added a unit test asserting the DB-error path returns error_type="DatabaseError", does not include the raw exception text, and triggers server-side logging.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
superset/mcp_service/dashboard/tool/update_dashboard.py	Separates not-found vs DB lookup failures; logs SQLAlchemy exceptions and returns a distinct DatabaseError without leaking DB details in the response.
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py	Adds coverage to ensure DB lookup failures aren’t masked as not-found and are logged.

[bito-code-review]

Code Review Agent Run #4dc1e5

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: d28c894..d28c894

  • superset/mcp_service/dashboard/tool/update_dashboard.py
  • tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ 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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by