eschutho opened a new pull request, #43556: URL: https://github.com/apache/superset/pull/43556
### SUMMARY `get_dataset_id_from_context()` (used by the `metric()` Jinja macro when no explicit `dataset_id` is passed) read the ambient Flask request via a raw ```python payload = request.get_json(cache=True) if request.is_json else None ``` `request.is_json` only inspects the `Content-Type` header -- it doesn't verify the body actually parses as JSON. When `metric()` is evaluated while rendering a chart's SQL from the MCP `get_chart_data` tool, the ambient Flask request is the raw MCP transport request (`Content-Type: application/json`, but not a chart-data JSON payload), so `request.get_json()` raises an unhandled Werkzeug `BadRequest` instead of falling through to this function's own `form_data`/`g.form_data` fallbacks. **Sentry**: [SUPERSET-PYTHON-1516](https://preset-inc.sentry.io/issues/7637766668/) -- 614 events since `firstSeen` 2026-07-28, still firing daily (MCP culprit hosts), 0 tracked users but a real broken tool call for anyone hitting it. This exact bug class was already fixed for the sibling function `get_form_data()` in #42196 (merged 2026-07-28 -- the same day this issue's `firstSeen` starts), which added `views/utils.get_request_json_body()` specifically to coerce a non-JSON body to `{}` instead of raising, for this exact MCP-tool-call shape. `get_dataset_id_from_context()` has its own separate inline copy of the old unguarded pattern and was missed by that fix. ### BEFORE/AFTER **Before**: a `metric()` macro call without an explicit `dataset_id`, evaluated from an MCP tool call, raises an unhandled 400/500 `BadRequest` from deep inside `request.get_json()`. **After**: falls through gracefully to the function's existing fallbacks (`request.form`/`request.args`/`g.form_data`), and if none of those resolve a dataset ID either, raises the same pre-existing, intentional `SupersetTemplateException("Please specify the Dataset ID for the ``%(name)s`` metric in the Jinja macro.")` that callers already handle. ### TESTING INSTRUCTIONS Added `test_metric_macro_no_dataset_id_non_json_body_with_json_content_type` in `tests/unit_tests/jinja_context_test.py`, mirroring #42196's own test shape (JSON content-type + non-JSON body via `current_app.test_request_context(data=..., content_type="application/json")`). - Verified fail-before/pass-after via `git stash` (pre-fix: raw `BadRequest` escapes; post-fix: falls through to the existing `SupersetTemplateException`). - Full `tests/unit_tests/jinja_context_test.py`: 136/137 passed. The 1 failure (`test_get_time_filter[...trino://mydb...]`) is a pre-existing, unrelated `sqlalchemy.databases` import error in the local trino dialect package -- reproduces identically on a clean `master` checkout, untouched by this diff. - `ruff check` / `ruff format --check` clean. - `mypy` (1.15.0) clean on the changed source file. ### TRADEOFFS None -- this restores the function's own existing "please specify the dataset ID" fallback path instead of an unhandled exception; no new failure-mode semantics. ### ADDITIONAL INFORMATION - [x] Has associated tests Shortcut: https://app.shortcut.com/preset/story/118456 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
