eschutho opened a new pull request, #42802:
URL: https://github.com/apache/superset/pull/42802
### SUMMARY
`DatasetRestApi.render_dataset_fields()` (used by `GET
/api/v1/dataset/<id_or_uuid>?include_rendered_sql=true`) caught
`(TemplateSyntaxError, SupersetSyntaxErrorException)` when rendering Jinja
macros in a dataset's `sql`/`metrics`/`columns` fields, but not the raw
`jinja2.exceptions.UndefinedError` that can also propagate from
`process_template()`. This widens the catch to `TemplateError` (jinja2's base
class covering both), converting the previously-unhandled case into the
existing typed 422.
### PROBLEM
`superset/jinja_context.py::BaseTemplateProcessor.process_template()`
converts most Jinja errors to typed exceptions, but has a bare-`raise` fallback
for `UndefinedError` when an undefined variable is accessed via
attribute/subscript syntax (e.g. `{{ foo.bar }}`) rather than a function call —
the regex heuristic that maps to `UndefinedTemplateFunctionException` only
matches function-call syntax. This is the same gap already fixed at other
`process_template()` call sites in this codebase: #42366, #42401, #42714,
#42757.
`jinja2.exceptions.UndefinedError`'s MRO is `UndefinedError ->
TemplateRuntimeError -> TemplateError -> Exception` — it is **not** a subclass
of `TemplateSyntaxError`. So a dataset with a malformed Jinja expression in its
`sql`, a metric, or a calculated column (e.g. referencing `{{
some_undefined.attr }}`) would raise a raw `UndefinedError` that escaped both
`render_dataset_fields`'s own catch and the calling view method's `except
SupersetTemplateException`, surfacing as an opaque 500 instead of the intended
typed 4xx.
### FIX
Widened the `except` clause in `render_dataset_fields()` from
`(TemplateSyntaxError, SupersetSyntaxErrorException)` to `(TemplateError,
SupersetSyntaxErrorException)`, and updated the import accordingly.
Additive-only: `SupersetSyntaxErrorException`/`SupersetTemplateException`
(Superset's own exception hierarchy) are not `jinja2.exceptions.TemplateError`
subclasses, so no currently-working path changes behavior — this only adds
coverage for the raw-`UndefinedError`-via-attribute/subscript-access case.
### TESTING INSTRUCTIONS
Added `test_get_dataset_include_rendered_sql_handles_undefined_error` to
`tests/unit_tests/datasets/api_tests.py`, mocking `get_template_processor` to
raise a raw `jinja2.exceptions.UndefinedError` and asserting `GET
/api/v1/dataset/{id}?include_rendered_sql=true` returns 422 (not 500).
- Confirmed the test fails on pre-fix code: `git stash` of `api.py` (test
kept) reproduces `assert 500 == 422`, with the raw `UndefinedError` traceback
surfacing through Flask's error handler.
- Post-fix: full `tests/unit_tests/datasets/api_tests.py` passes (4/4).
- `ruff check` / `ruff format --check`: pass. `pre-commit run` (mypy, ruff,
ruff-format, pylint): all hooks pass. Diffed `mypy superset/datasets/api.py`
output before/after — byte-identical (only pre-existing, unrelated errors),
zero new errors.
### ADDITIONAL INFORMATION
- [x] Has associated tests
- [x] Confirmed the regression test fails on pre-fix code and passes post-fix
**Tradeoffs**: none — additive-only exception-handling widening, no behavior
change to any currently-working path.
Related: #42366, #42401, #42714, #42757 (same `process_template()`
bare-raise-fallback bug class, different call sites).
--
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]
