eschutho opened a new pull request, #42917:
URL: https://github.com/apache/superset/pull/42917
### SUMMARY
`SqlLabRestApi.format_sql()` (POST `/api/v1/sqllab/format_sql/`, SQL Lab's
"Format SQL" button) calls `process_template()` with no catch for jinja2
exceptions. When an undefined Jinja variable is accessed via
attribute/subscript (e.g. `{{ tbl.name }}`, not a bare `{{ tbl }}`),
`BaseTemplateProcessor.process_template()`'s bare-raise fallback re-raises the
raw `jinja2.exceptions.UndefinedError` instead of converting it to a Superset
exception. Since neither of `format_sql()`'s existing `except` clauses
(`json.JSONDecodeError`, and the outer `ValidationError`) catches it, the raw
exception escapes to Flask's global `@app.errorhandler(Exception)` catch-all as
an opaque 500 (`GENERIC_BACKEND_ERROR`) instead of a typed, user-actionable 4xx.
This is the same bug class fixed at 6 other `process_template()` call sites
in this series: #42366, #42401, #42714, #42757, #42802, #42851.
`superset/sqllab/api.py` had not previously been touched by any of them.
### BEFORE/AFTER
**Before:** posting SQL with an undefined-attribute Jinja reference to
`/api/v1/sqllab/format_sql/` returns a `500 GENERIC_BACKEND_ERROR` with a raw
traceback-derived message.
**After:** the same request returns a `400` with a typed
`SupersetErrorException` (`GENERIC_COMMAND_ERROR`) whose message names the
undefined variable.
### FIX
Added `except TemplateError as ex: raise
SupersetErrorException(SupersetError(message=str(ex),
error_type=GENERIC_COMMAND_ERROR, level=ERROR), status=400) from ex` after the
existing `except json.JSONDecodeError` clause, mirroring
`QueryEstimationCommand.run()` (#42757) exactly. `SupersetErrorException` is
handled by the global `@app.errorhandler(SupersetErrorException)`, the same
mechanism `estimate_query_cost()` (a few lines above, same file) already relies
on for its own `SupersetErrorException` — consistent with this file's existing
pattern, not a new one. Additive-only; no existing behavior changes for
well-formed templates.
### TESTING INSTRUCTIONS
1. `POST /api/v1/sqllab/format_sql/` with `sql: "select * from {{ tbl.name
}}"`, `template_params: "{}"` (or any non-empty dict that doesn't define
`tbl`), and a valid `database_id`.
2. Before this fix: `500` with a raw jinja2 traceback message.
3. After this fix: `400` with a structured error body naming `tbl` as
undefined.
New regression test:
`test_format_sql_request_with_undefined_jinja_attribute` in
`tests/integration_tests/sql_lab/api_tests.py`, right after the existing
`test_format_sql_request_with_jinja`. Confirmed it fails on pre-fix code (`git
stash` on the source fix, raw `UndefinedError` traceback, `assert
rv.status_code == 400` fails with `500`) and passes post-fix.
### ADDITIONAL INFORMATION
- [x] Has associated issue: matches the bug class from #42366
- [x] Required feature flags: N/A
- [ ] Changes UI
- [ ] Includes DB migration
- [ ] Confirm bespoke/custom deployment/upgrade steps required to prepare
and deploy this change
**Tradeoffs:** none — this is an additive `except` clause converting an
unhandled 500 into a typed 400; no existing success-path behavior changes.
Related: #42366, #42401, #42714, #42757, #42802, #42851 (same bug class,
other `process_template()` 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]
