eschutho opened a new pull request, #42714: URL: https://github.com/apache/superset/pull/42714
### SUMMARY `AlertCommand._execute_query()` in `superset/commands/report/alert.py` calls `sql_template.process_template(self._report_schedule.sql)` *before* the `try:` block a few lines below it, even though that block's `except Exception as ex: raise AlertQueryError(...) from ex` is clearly meant to cover the whole query-execution flow (the docstring documents `:raises AlertQueryError: SQL query is not valid`). Because the Jinja render call sits outside the `try`, any exception raised while templating the alert's SQL — `SupersetSyntaxErrorException`, `SupersetTemplateException`, or a bare `jinja2.exceptions.TemplateError`/`UndefinedError` re-raised by `process_template()`'s own fallback path in `superset/jinja_context.py` — propagates unhandled instead of becoming the documented `AlertQueryError`. Alert SQL is a normal user-editable field with documented Jinja templating support, so a user-authored alert query referencing an undefined Jinja variable currently crashes the alert-execution task with a raw exception instead of surfacing the typed, expected error. Same bug class (raw system/library exception escaping instead of a proper Superset exception) as #42366 and #42401. ### PROBLEM A raw `jinja2.exceptions.TemplateError` (or a raw `SupersetSyntaxErrorException`/`SupersetTemplateException`) can propagate out of `AlertCommand._execute_query()` uncaught, because the Jinja-rendering line is physically placed one statement above the `try:` block that's supposed to wrap the whole alert-query execution. ### FIX Move `rendered_sql = sql_template.process_template(self._report_schedule.sql)` to be the first statement inside the existing `try:` block, so it's covered by the same `except Exception as ex: raise AlertQueryError(...) from ex` that already wraps the rest of the query-execution flow. `sql_template = jinja_context.get_template_processor(...)` is left in place — this is a 2-line, additive-only change with no behavior change for any alert query that renders successfully today. ### TESTING INSTRUCTIONS - Added `tests/unit_tests/commands/report/alert_test.py::test_execute_query_wraps_template_rendering_error`, which mocks `process_template` to raise a `jinja2.exceptions.TemplateError` and asserts `_execute_query()` raises `AlertQueryError` instead of the raw error. - Confirmed empirically that this test fails on pre-fix code (raises the raw `TemplateError` uncaught) and passes post-fix. - `pytest tests/unit_tests/commands/report/alert_test.py` — 25 passed. - `ruff check` / `ruff format --check` on both changed files — clean. - `mypy` on changed files — no new errors. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API **Tradeoffs:** None — this is an additive exception-handling fix only. No change to the failure mode for any query that renders/executes successfully today; only affects the (previously-uncaught) failure path when the alert's SQL template fails to render. Related: #42366, #42401 (same bug class: raw Jinja/system exceptions propagating instead of typed Superset exceptions). -- 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]
