eschutho opened a new pull request, #43226:
URL: https://github.com/apache/superset/pull/43226
### SUMMARY
`StreamingSqlResultExportCommand.validate()` in
`superset/commands/sql_lab/streaming_export_command.py` calls
`self._query.raise_for_access()` guarded **only** by
`except SupersetSecurityException`. `raise_for_access()` re-parses the
query's
SQL via `process_jinja_sql()` and can raise a **raw**
`jinja2.exceptions.TemplateError` — e.g. a `TemplateSyntaxError` from
malformed
Jinja-like content in the SQL. Because that exception type wasn't caught
here,
it would surface as an opaque **HTTP 500** when a client streamed a CSV
export
for a SQL Lab query whose SQL triggers this path.
This is the third sibling site with the identical gap, after
`superset/commands/sql_lab/results.py` (fixed in #43145, merged) and
`superset/sqllab/api.py` / `superset/commands/sql_lab/estimate.py` (already
correct, the precedent this mirrors). `#43145`'s own PR body called out
`streaming_export_command.py` explicitly as one of two sibling files
intentionally left for follow-up; this PR ships that follow-up for the
streaming-export site (the CSV `export.py` follow-up is tracked separately).
This is an additive catch — it only narrows a previously-uncaught exception
into the existing structured-error path. There is no change to any
success-path or existing failure-mode semantics, so no Tradeoffs section is
warranted.
### PROBLEM
- `StreamingSqlResultExportCommand.validate()` → `raise_for_access()` →
`process_jinja_sql()` raises a raw `TemplateError` subclass on malformed
Jinja-like SQL content.
- Only `SupersetSecurityException` was caught, so the raw `TemplateError`
escaped the command and became an opaque HTTP 500 instead of a clean 4xx.
### FIX
Add, alongside the existing `except SupersetSecurityException` clause in
`validate()`, the same guard used verbatim by the sibling sites:
```python
except TemplateError as ex:
raise SupersetErrorException(
SupersetError(
message=str(ex),
error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
level=ErrorLevel.ERROR,
),
status=400,
) from ex
```
plus the missing `from jinja2.exceptions import TemplateError` import. The
fix
is surgical: one method, one bug class, nothing else in the file touched.
### TESTING INSTRUCTIONS
Added `test_validate_jinja_template_error` to
`tests/unit_tests/commands/sql_lab/streaming_export_command_test.py`, modeled
on the existing `test_validate_access_denied` in the same file: it mocks
`Query.raise_for_access` to raise `TemplateSyntaxError('unexpected end of
template', lineno=1)` (a `TemplateError` subclass, confirmed via
`issubclass(TemplateSyntaxError, TemplateError) == True`) and verifies the
command raises `SupersetErrorException` with `error_type ==
GENERIC_COMMAND_ERROR` and `status == 400`.
> **Note on local execution:** this shared clone's integration-test harness
> could not be brought up (`ModuleNotFoundError: flask_talisman` on the
> `tests/conftest.py` import chain — a pre-existing environment issue, not
> caused by this change), so the new unit test was added but not executed via
> `pytest` locally; CI will be its first full run. The fix logic was verified
> independently via a standalone control-flow repro replicating the exact
> try/except shapes: the pre-fix shape lets `TemplateSyntaxError` propagate
> unhandled, the post-fix shape catches it and converts it to a 400
> `SupersetErrorException`.
Manual verification: stream a CSV export (`StreamingSqlResultExportCommand`)
for a SQL Lab query whose SQL contains malformed Jinja-like content; the
response is a structured 400 instead of an opaque 500.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: internal Shortcut sc-117475
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Follows the same SQL Lab error-cleanup pattern as #42366 (precedent), #42401,
and #43145 (merged, the sibling `results.py` guard this mirrors).
--
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]
