eschutho opened a new pull request, #44246:
URL: https://github.com/apache/superset/pull/44246
### SUMMARY
**Problem**: In `SqlaTable.get_fetch_values_predicate`, the
`template_processor.process_template()` call sits *outside* the `try:` block
whose `except (TemplateError, SupersetSyntaxErrorException)` clause was written
to catch Jinja template errors from it. A bare
`jinja2.exceptions.UndefinedError` (a `TemplateError` subclass) raised by
attribute access on an undefined variable (e.g. `{{ foo.bar }}`) escapes
uncaught, propagating as an opaque 500 (`GENERIC_BACKEND_ERROR`) instead of the
intended 400 with a helpful "Error in jinja expression in fetch values
predicate" message.
**Fix**: Move the `process_template` call inside the existing `try:` block
so the pre-existing except clause actually covers it. No new except clauses, no
refactoring — just correcting the scope of the existing handler.
**Precedent**: This is the same bug class as the fix in #42401 (`catch
TemplateError alongside SupersetSyntaxErrorException in models.py`) —
`UndefinedError` escaping `process_template` due to misscoped or missing error
handling. The existing test
`test_get_dataset_include_rendered_sql_handles_undefined_error` covers the
analogous fix in the `datasets/api.py` call path; this PR covers the
`get_fetch_values_predicate` call path.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only error handling fix.
### TESTING INSTRUCTIONS
1. New unit test `test_get_fetch_values_predicate_wraps_undefined_error` in
`tests/unit_tests/connectors/sqla/models_test.py`:
- Mocks `process_template` to raise `UndefinedError("'foo' is undefined")`
- Asserts that `get_fetch_values_predicate` wraps it in
`QueryObjectValidationError`
- **Verified fail-before/pass-after**: test fails on unmodified code (raw
`UndefinedError` escapes), passes after the fix
2. Full test file (85 tests) passes with no regressions
3. `ruff check`, `ruff format --check`, and `mypy` all pass
### 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]
