fix(chart/data): handle QueryObjectValidationError in _get_data_response

[eschutho]

Summary

ChartDataRestApi._get_data_response() catches ChartDataCacheLoadError and ChartDataQueryFailedError but leaves QueryObjectValidationError unhandled. When this exception escapes command.execute() through paths outside get_df_payload_result()'s try/except — such as ensure_totals_available(), the invalid-result-type check in get_query_results_with_timing(), or preparers like _prepare_drill_detail_query() — Flask's global handler returns HTTP 500 instead of 400.

QueryObjectValidationError is already imported in this file and has status = 400. This PR adds the missing except clause, consistent with the equivalent handler in the command-setup block above.

Testing

• pytest tests/unit_tests/charts/data/test_api.py
• pytest tests/integration_tests/charts/charts_test.py

Tracking: sc-117145

[bito-code-review]

Code Review Agent Run #495cdd

Actionable Suggestions - 0

Review Details

• Files reviewed - 1 · Commit Range: 92c4854..92c4854

  • superset/charts/data/api.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

[codeant-ai-for-open-source]

Suggestion: The new handling only covers _get_data_response, but the GLOBAL_ASYNC_QUERIES path calls command.execute(force_cached=True) directly in _run_async, where only ChartDataCacheLoadError is caught. If execution-time validation in get_payload_result() raises QueryObjectValidationError during the cache lookup, it escapes to Flask and returns HTTP 500 instead of 400. Add equivalent handling to the async cache-hit path before scheduling the background job. [incomplete implementation]

Severity Level: Major ⚠️

- ❌ Async chart requests can return HTTP 500 for invalid queries.
- ⚠️ Clients receive server errors instead of validation feedback.
- ⚠️ GLOBAL_ASYNC_QUERIES cache-hit handling remains inconsistent.

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset/charts/data/api.py
**Line:** 649:650
**Comment:**
	*Incomplete Implementation: The new handling only covers `_get_data_response`, but the `GLOBAL_ASYNC_QUERIES` path calls `command.execute(force_cached=True)` directly in `_run_async`, where only `ChartDataCacheLoadError` is caught. If execution-time validation in `get_payload_result()` raises `QueryObjectValidationError` during the cache lookup, it escapes to Flask and returns HTTP 500 instead of 400. Add equivalent handling to the async cache-hit path before scheduling the background job.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

[bito-code-review]

The flagged issue is correct. The GLOBAL_ASYNC_QUERIES path in _run_async currently lacks the QueryObjectValidationError handling that was added to _get_data_response. To resolve this, you should wrap the command.execute(force_cached=True) call in a try-except block that catches QueryObjectValidationError and returns a 400 response, similar to the implementation in _get_data_response.

superset/charts/data/api.py

try:
            result = command.execute(force_cached=True)
        except QueryObjectValidationError as exc:
            return self.response_400(message=sanitize_error_message(exc.message))
        except ChartDataCacheLoadError:
            # ... existing logic ...

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.62%. Comparing base (8bd25b3) to head (45b643a).
⚠️ Report is 20 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/charts/data/api.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #43081      +/-   ##
==========================================
- Coverage   66.77%   66.62%   -0.15%     
==========================================
  Files        2862     2866       +4     
  Lines      161657   162547     +890     
  Branches    37287    37447     +160     
==========================================
+ Hits       107943   108301     +358     
- Misses      51669    52155     +486     
- Partials     2045     2091      +46     

Flag	Coverage Δ
hive	38.25% <0.00%> (-0.19%)	⬇️
mysql	57.90% <0.00%> (-0.20%)	⬇️
postgres	57.93% <0.00%> (-0.21%)	⬇️
presto	40.21% <0.00%> (-0.21%)	⬇️
python	59.32% <0.00%> (-0.22%)	⬇️
sqlite	57.56% <0.00%> (-0.19%)	⬇️
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.

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	45b643a
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a7d047b52b8880008ca93e0
😎 Deploy Preview	https://deploy-preview-43081--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.