eschutho opened a new pull request, #42401:
URL: https://github.com/apache/superset/pull/42401
### SUMMARY
Four call sites in `superset/connectors/sqla/models.py` that render Jinja
expressions only caught `SupersetSyntaxErrorException`, missing
`jinja2.exceptions.TemplateError`. A raw `TemplateError` (e.g.
`UndefinedError`) from these sites propagated as an unhandled 500 instead of a
clear, actionable `QueryObjectValidationError`. This mirrors the same bug class
fixed for `get_rendered_sql()` in #42366, and this file already has two sibling
call sites (RLS filters, `get_fetch_values_predicate`) that catch the error
correctly — this PR brings the remaining four in line with that existing
pattern.
### PROBLEM
`TableColumn.get_sqla_col`, `TableColumn.get_timestamp_expression`,
`SqlMetric.get_sqla_col`, and
`SqlaTable._render_adhoc_expression_for_metadata_lookup` each wrap
`template_processor.process_template(...)` in a `try`/`except
SupersetSyntaxErrorException`. Since `jinja2.exceptions.UndefinedError` is a
subclass of `TemplateError`, not `SupersetSyntaxErrorException`, a
column/metric/adhoc-column Jinja expression that references an undefined
variable (e.g. an arithmetic operation on an undefined name) raises a raw
`UndefinedError` that these `except` clauses don't catch, surfacing as an
unhandled 500 during chart explore/render.
### FIX
Widen the `except` clause at all four sites to `except (TemplateError,
SupersetSyntaxErrorException) as ex:` and extract the message via `getattr(ex,
"message", str(ex))`, exactly matching the two already-correct sibling call
sites in this same file. `TemplateError` was already imported at the top of the
file. No other exception handling was touched, and `superset/models/helpers.py`
(already fixed in #42366) was left untouched.
### TESTING INSTRUCTIONS
- Added
`tests/unit_tests/connectors/sqla/models_test.py::test_get_sqla_col_wraps_raw_jinja_undefined_error`,
which constructs a `TableColumn` with the Jinja expression `{{ nonexistent_var
+ 1 }}` and asserts `get_sqla_col()` raises `QueryObjectValidationError`
instead of a raw `UndefinedError`. Verified the test fails on pre-fix code and
passes after.
- Ran `pytest tests/unit_tests/connectors/sqla/models_test.py` (44 passed)
and `pytest tests/unit_tests/jinja_context_test.py` (101 passed, no
regressions).
- Ran `ruff check` and `mypy` on the changed files — no new issues.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
