eschutho opened a new pull request, #44202: URL: https://github.com/apache/superset/pull/44202
### SUMMARY Fixes Sentry issue **SUPERSET-PYTHON-ZY6** (https://preset-inc.sentry.io/issues/7291702862/) — `UndefinedTemplateFunctionException: 'ref' is undefined`, **204+ events** since 2026-02-25, culprit `DatabaseRestApi.validate_sql`. **Root cause.** `ValidateSQLCommand.run()` in `superset/commands/database/validate_sql.py` wraps template rendering + validation in a try/except chain. `UndefinedTemplateFunctionException` (defined in `superset/jinja_context.py`) is a **subclass of `SupersetTemplateException`**, so it fell into the generic `except SupersetTemplateException` branch and was logged at **ERROR with a full stack trace** (`exc_info=True`) — even though it already returns a clean 400 to the client. It fires specifically when a user calls an undefined Jinja *function* (e.g. a dbt-style `ref(...)` macro Superset doesn't provide) in a SQL Lab "Validate" request: `jinja_context.process_template` catches `UndefinedError`, detects the undefined name is being *called* as a function, and raises `UndefinedTemplateFunctionException` to distinguish "user called an undefined function" from other undefined-variable errors. **This is a user input mistake, not a system fault** — and that classification is already established elsewhere in the same codebase. `superset/sqllab/query_render.py`'s `SqlQueryRenderImpl.render()` (the actual SQL Lab *execution* path) catches this exact exception and silently falls back to the raw un-rendered SQL rather than treating it as an error: ```python except Exception as ex: from superset.jinja_context import UndefinedTemplateFunctionException if isinstance(ex, UndefinedTemplateFunctionException): return query_model.sql.strip().strip(";") raise ``` So `validate_sql.py`'s ERROR-level logging of this specific exception was inconsistent with the rest of the codebase and was the sole source of this Sentry noise. **The fix.** Add a dedicated `except UndefinedTemplateFunctionException` branch **before** the existing `except SupersetTemplateException` branch (except-clause order matters — the subclass must be caught first). It logs at `logger.warning(...)` without `exc_info` (mirroring the sibling `SupersetSyntaxErrorException` branch), and still raises the same `ValidatorSQL400Error` with the same `SupersetError` shape/message the generic branch produced. ### TRADEOFFS **There is NO client-facing / behavior change.** The response is byte-for-byte identical to before: the same `ValidatorSQL400Error` with the same `"Template processing failed: %(ex)s"` message, `GENERIC_COMMAND_ERROR` error type, and `ERROR` error level. Failure-mode semantics (what the API returns, the HTTP status, the payload) are unchanged. The *only* change is server-side **log severity**: ERROR-with-traceback → WARNING-without-traceback for this one expected user-input case. The Sentry issue auto-resolves on merge via `Fixes SUPERSET-PYTHON-ZY6` — it is not being resolved manually. ### TESTING INSTRUCTIONS Added `test_validate_sql_undefined_template_function` in `tests/unit_tests/commands/databases/validate_sql_test.py` (directly analogous to the existing `test_validate_sql_template_processing_error`). It: - sets `process_template.side_effect = UndefinedTemplateFunctionException("'ref' is undefined")`, - asserts a `ValidatorSQL400Error` is raised with `"Template processing failed"` and `"'ref' is undefined"` in the message (unchanged response shape), and - **asserts the log goes out at WARNING, not ERROR, and without a traceback** — mocking `logger.warning`/`logger.error` and checking `logger_error.assert_not_called()` plus that no `exc_info` kwarg is passed. This is the whole point of the fix; a test that didn't check log level would be vacuous (it would pass pre-fix too). Verified manually that the new test **fails on the pre-fix code** (stashed the source change, ran the test, it failed) and **passes after** the fix. Full file: 7 passed. `ruff check` + `ruff format` clean; `pre-commit run` (mypy, ruff, pylint) passes on both changed files. Refs Shortcut story [SC-120409](https://app.shortcut.com/preset/story/120409). ### 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
