From 98b80cd103c70f56501a869826c3c571274c60f8 Mon Sep 17 00:00:00 2001 From: Gabriel Torres Ruiz Date: Thu, 21 Aug 2025 16:11:36 -0300 Subject: [PATCH 1/5] fix(error): jinja2 error handling improvements --- superset/charts/schemas.py | 7 +- superset/jinja_context.py | 85 ++++- superset/utils/jinja_template_validator.py | 164 ++++++++++ tests/unit_tests/jinja_template_error_test.py | 296 ++++++++++++++++++ .../utils/test_jinja_template_validator.py | 111 +++++++ 5 files changed, 657 insertions(+), 6 deletions(-) create mode 100644 superset/utils/jinja_template_validator.py create mode 100644 tests/unit_tests/jinja_template_error_test.py create mode 100644 tests/unit_tests/utils/test_jinja_template_validator.py diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index e794f1809109..692d1715ccda 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -37,6 +37,7 @@ PostProcessingBoxplotWhiskerType, PostProcessingContributionOrientation, ) +from superset.utils.jinja_template_validator import validate_params_json_with_jinja if TYPE_CHECKING: from superset.common.query_context import QueryContext @@ -208,7 +209,7 @@ class ChartPostSchema(Schema): params = fields.String( metadata={"description": params_description}, allow_none=True, - validate=utils.validate_json, + validate=validate_params_json_with_jinja, ) query_context = fields.String( metadata={"description": query_context_description}, @@ -269,7 +270,9 @@ class ChartPutSchema(Schema): ) owners = fields.List(fields.Integer(metadata={"description": owners_description})) params = fields.String( - metadata={"description": params_description}, allow_none=True + metadata={"description": params_description}, + allow_none=True, + validate=validate_params_json_with_jinja, ) query_context = fields.String( metadata={"description": query_context_description}, allow_none=True diff --git a/superset/jinja_context.py b/superset/jinja_context.py index 42fc5abf5d7f..670b0153e84a 100644 --- a/superset/jinja_context.py +++ b/superset/jinja_context.py @@ -18,6 +18,7 @@ from __future__ import annotations +import logging import re from dataclasses import dataclass from datetime import datetime @@ -27,7 +28,8 @@ import dateutil from flask import current_app, g, has_request_context, request from flask_babel import gettext as _ -from jinja2 import DebugUndefined, Environment +from jinja2 import DebugUndefined, Environment, TemplateSyntaxError +from jinja2.exceptions import SecurityError, UndefinedError from jinja2.sandbox import SandboxedEnvironment from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.sql.expression import bindparam @@ -37,7 +39,11 @@ from superset.commands.dataset.exceptions import DatasetNotFoundError from superset.common.utils.time_range_utils import get_since_until_from_time_range from superset.constants import LRU_CACHE_MAX_SIZE, NO_TIME_RANGE -from superset.exceptions import SupersetTemplateException +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.exceptions import ( + SupersetSyntaxErrorException, + SupersetTemplateException, +) from superset.extensions import feature_flag_manager from superset.sql.parse import Table from superset.utils import json @@ -56,6 +62,8 @@ from superset.models.core import Database from superset.models.sql_lab import Query +logger = logging.getLogger(__name__) + NONE_TYPE = type(None).__name__ ALLOWED_TYPES = ( NONE_TYPE, @@ -688,10 +696,79 @@ def process_template(self, sql: str, **kwargs: Any) -> str: >>> process_template(sql) "SELECT '2017-01-01T00:00:00'" """ - template = self.env.from_string(sql) - kwargs.update(self._context) + try: + template = self.env.from_string(sql) + except ( + TemplateSyntaxError, + SecurityError, + UndefinedError, + UnicodeError, + UnicodeDecodeError, + UnicodeEncodeError, + ) as ex: + error_msg = str(ex) + exception_type = type(ex).__name__ + + message = ( + f"Jinja2 template error ({exception_type}). " + "Please check your template syntax and variable references. " + f"Original error: {error_msg}" + ) + + line_number = getattr(ex, "lineno", None) + + logger.warning( + "Jinja2 template client error", + extra={ + "error_message": error_msg, + "template_snippet": sql[:200] if sql else None, + "template_length": len(sql) if sql else 0, + "line_number": line_number, + "error_type": "CLIENT_TEMPLATE_ERROR", + "exception_type": exception_type, + }, + exc_info=False, + ) + + error = SupersetError( + message=message, + error_type=SupersetErrorType.GENERIC_COMMAND_ERROR, + level=ErrorLevel.ERROR, + extra={ + "template": sql[:500], + "line": line_number, + "exception_type": exception_type, + }, + ) + + raise SupersetSyntaxErrorException([error]) from ex + except Exception as ex: + error_msg = str(ex) + exception_type = type(ex).__name__ + message = ( + "Internal error processing Jinja2 template. " + "Please contact your administrator. " + f"Error: {error_msg}" + ) + + logger.error( + "Jinja2 template server error", + extra={ + "error_message": error_msg, + "template_snippet": sql[:200] if sql else None, + "template_length": len(sql) if sql else 0, + "error_type": "SERVER_TEMPLATE_ERROR", + "exception_type": exception_type, + }, + exc_info=True, + ) + + raise SupersetTemplateException(message) from ex + + kwargs.update(self._context) context = validate_template_context(self.engine, kwargs) + try: return template.render(context) except RecursionError as ex: diff --git a/superset/utils/jinja_template_validator.py b/superset/utils/jinja_template_validator.py new file mode 100644 index 000000000000..90002700ea95 --- /dev/null +++ b/superset/utils/jinja_template_validator.py @@ -0,0 +1,164 @@ +""" +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +""" + +import re +from typing import Any, Dict, Optional, Tuple + +from jinja2 import TemplateSyntaxError +from jinja2.sandbox import SandboxedEnvironment +from marshmallow import ValidationError + +from superset.utils import json + + +def validate_jinja_template(template_str: str) -> Tuple[bool, Optional[str]]: + """ + Validates Jinja2 template syntax. + + Returns: + Tuple of (is_valid, error_message) + If valid: (True, None) + If invalid: (False, "Error description") + """ + if not template_str: + return True, None + + try: + env = SandboxedEnvironment() + env.from_string(template_str) + + return True, None + + except TemplateSyntaxError as e: + error_msg = str(e) + + if "expected token 'end of print statement'" in error_msg: + if "such" in template_str.lower(): + return False, ( + "Invalid Jinja2 syntax. Found text after variable name. " + "Use {{ variable }} for simple variables or " + "{{ variable | default('value') }} for defaults. " + f"Error: {error_msg}" + ) + return False, ( + "Invalid Jinja2 syntax. " + "Check that all {{ }} blocks are properly closed. " + f"Error: {error_msg}" + ) + elif "unexpected end of template" in error_msg: + return False, ( + "Unclosed Jinja2 block. " + "Make sure all {{ and {% blocks have closing }} and %. " + f"Error: {error_msg}" + ) + elif "expected token" in error_msg: + return False, (f"Invalid Jinja2 syntax. {error_msg}") + else: + return False, f"Template syntax error: {error_msg}" + + except Exception as e: + return False, f"Template validation error: {str(e)}" + + +def sanitize_jinja_template(template_str: str) -> str: + """ + Attempts to fix common Jinja2 template mistakes. + + This is a best-effort function that tries to fix obvious errors. + """ + if not template_str: + return template_str + + pattern = r"\{\{\s*(\w+)\s+such\s+as\s+([^}]+?)\s*\}\}" + template_str = re.sub(pattern, r"{{ \1 | default(\2) }}", template_str) + + return template_str + + +def _check_filter_sql_expression(sql_expression: str) -> None: + """Check SQL expression for valid Jinja2.""" + is_valid, error_msg = validate_jinja_template(sql_expression) + if not is_valid: + raise ValidationError(f"Invalid Jinja2 template in SQL filter: {error_msg}") + + +def _check_filter_clause(clause: str) -> None: + """Check filter clause for valid Jinja2.""" + if "{{" in clause: + is_valid, error_msg = validate_jinja_template(clause) + if not is_valid: + raise ValidationError( + f"Invalid Jinja2 template in WHERE clause: {error_msg}" + ) + + +def _check_filter_dict(filter_dict: Dict[str, Any]) -> None: + """Check SQL expressions in a filter dictionary.""" + if not isinstance(filter_dict, dict): + return + + # Check the sqlExpression field for custom SQL filters + if sql_expression := filter_dict.get("sqlExpression"): + _check_filter_sql_expression(sql_expression) + + # Check the clause field (for WHERE clauses) + if clause := filter_dict.get("clause"): + _check_filter_clause(clause) + + +def validate_jinja_template_in_params(params: Dict[str, Any]) -> None: + """ + Validates Jinja2 templates in chart parameters. + This function checks adhoc_filters and other fields that might contain Jinja2. + """ + # Check adhoc_filters + if adhoc_filters := params.get("adhoc_filters", []): + for filter_item in adhoc_filters: + _check_filter_dict(filter_item) + + # Check extra_filters + if extra_filters := params.get("extra_filters", []): + for filter_item in extra_filters: + _check_filter_dict(filter_item) + + # Check where clause + if where_clause := params.get("where"): + _check_filter_clause(where_clause) + + +def validate_params_json_with_jinja(value: str | None) -> None: + """ + Validates that params is valid JSON and contains valid Jinja2 templates. + """ + if value is None: + return + + # First validate JSON + try: + params = json.loads(value) + except (json.JSONDecodeError, TypeError) as ex: + raise ValidationError("Invalid JSON") from ex + + # Then validate Jinja2 templates within the params + try: + validate_jinja_template_in_params(params) + except ValidationError: + raise + except Exception as ex: + raise ValidationError(f"Template validation error: {str(ex)}") from ex diff --git a/tests/unit_tests/jinja_template_error_test.py b/tests/unit_tests/jinja_template_error_test.py new file mode 100644 index 000000000000..9aa906b674b8 --- /dev/null +++ b/tests/unit_tests/jinja_template_error_test.py @@ -0,0 +1,296 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from unittest.mock import MagicMock + +import pytest + +from superset.errors import SupersetErrorType +from superset.exceptions import SupersetSyntaxErrorException +from superset.jinja_context import BaseTemplateProcessor +from superset.utils import json + + +def create_mock_processor(): + """Create a BaseTemplateProcessor with mocked database""" + mock_database = MagicMock() + mock_database.db_engine_spec = MagicMock() + return BaseTemplateProcessor(database=mock_database) + + +def test_jinja2_syntax_error_general(): + """Test handling of general Jinja2 syntax errors""" + processor = create_mock_processor() + template = "SELECT * WHERE column = {{ variable such as 'default' }}" + + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + # Check the exception details + exception = exc_info.value + assert len(exception.errors) == 1 + error = exception.errors[0] + + # Verify error message contains helpful guidance + assert "Jinja2 template error" in error.message + assert "TemplateSyntaxError" in error.message + assert "check your template syntax" in error.message + assert "Original error:" in error.message + + # Verify error type and status + assert error.error_type == SupersetErrorType.GENERIC_COMMAND_ERROR + assert exception.status == 422 + + # Verify extra data includes template snippet + assert "template" in error.extra + assert error.extra["template"][:50] == template[:50] + + +def test_jinja2_syntax_error_unclosed_block(): + """Test handling of unclosed Jinja2 blocks""" + processor = create_mock_processor() + template = "SELECT * WHERE column = {{ variable " + + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + error = exception.errors[0] + + # Should get general syntax error with helpful guidance + assert "Jinja2 template error" in error.message + assert "TemplateSyntaxError" in error.message + assert "check your template syntax" in error.message + + +def test_jinja2_syntax_error_unexpected_end(): + """Test handling of unexpected end of template""" + processor = create_mock_processor() + template = "SELECT * WHERE {% if condition" + + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + error = exception.errors[0] + + # Should get general syntax error with helpful guidance + assert "Jinja2 template error" in error.message + assert "TemplateSyntaxError" in error.message + + +def test_datadog_structured_logging_data(): + """Test that errors include structured data for Datadog logging""" + # We can't easily mock the logger due to module-level instantiation, + # but we can test that the error includes the structured data we need + processor = create_mock_processor() + template = "SELECT * WHERE column = {{ variable such as 'default' }}" + + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + error = exception.errors[0] + + # Verify error contains structured data that would be useful for Datadog + assert error.error_type == SupersetErrorType.GENERIC_COMMAND_ERROR + assert "template" in error.extra + assert "line" in error.extra + assert error.extra["template"] == template[:500] # Truncated for logging + + # The actual logging happens inside the function - we can see it in captured logs + # This is sufficient to verify the functionality is working + + +def test_valid_jinja2_template_passes(): + """Test that valid templates work correctly""" + processor = create_mock_processor() + valid_templates = [ + "SELECT * WHERE column = {{ variable }}", + "SELECT * WHERE column = {{ variable | default('value') }}", + "SELECT * WHERE column = {{ variable or 'value' }}", + "{% if condition %}SELECT * {% endif %}", + ] + + for template in valid_templates: + # Should not raise any exception + result = processor.process_template(template) + assert result is not None + + +def test_api_response_format(): + """Test that the API response format is correct for template errors""" + processor = create_mock_processor() + template = "SELECT * WHERE column = {{ variable such as 'default' }}" + + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + + # Simulate what the API error handler would do + errors_dict = [ + { + "message": error.message, + "error_type": error.error_type.name, + "level": error.level.name, + "extra": error.extra, + } + for error in exception.errors + ] + + # This is what would be sent to the client + response_data = {"errors": errors_dict} + + # Verify the structure + assert "errors" in response_data + assert len(response_data["errors"]) == 1 + + error_data = response_data["errors"][0] + assert "message" in error_data + assert "error_type" in error_data + assert "level" in error_data + assert "extra" in error_data + + # Verify it's JSON serializable (important for API responses) + json_str = json.dumps(response_data) + assert json_str is not None + + +def test_chart_params_validation(): + """Test that chart params with invalid Jinja2 are rejected""" + from marshmallow import ValidationError + + from superset.utils.jinja_template_validator import ( + validate_jinja_template_in_params, + ) + + # Invalid template in adhoc_filters + invalid_params = { + "adhoc_filters": [ + { + "sqlExpression": "column = {{ variable such as 'value' }}", + "clause": "WHERE", + } + ] + } + + with pytest.raises(ValidationError) as exc_info: + validate_jinja_template_in_params(invalid_params) + + error_msg = str(exc_info.value) + assert "Invalid Jinja2 template" in error_msg + + +def test_chart_params_validation_passes_valid(): + """Test that valid Jinja2 templates pass validation""" + from superset.utils.jinja_template_validator import ( + validate_jinja_template_in_params, + ) + + valid_params = { + "adhoc_filters": [ + { + "sqlExpression": "column = {{ variable | default('value') }}", + "clause": "WHERE", + } + ], + "where": "date >= {{ from_date }}", + } + + # Should not raise any exception + validate_jinja_template_in_params(valid_params) + + +def test_jinja2_client_template_error(): + """Test handling of client-side Jinja2 template errors""" + from unittest.mock import patch + + from jinja2 import UndefinedError + + processor = create_mock_processor() + template = "SELECT * FROM table" + + # Mock the Environment.from_string to raise a client error + with patch.object( + processor.env, "from_string", side_effect=UndefinedError("Variable not defined") + ): + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + error = exception.errors[0] + + # Should get client error message (422) + assert "Jinja2 template error" in error.message + assert "UndefinedError" in error.message + assert "check your template syntax" in error.message + assert "Variable not defined" in error.message + assert exception.status == 422 + + # Verify it includes exception type info + assert "UndefinedError" in error.extra.get("exception_type", "") + + +def test_jinja2_security_error(): + """Test handling of SecurityError as client error with proper exception type""" + from unittest.mock import patch + + from jinja2.exceptions import SecurityError + + processor = create_mock_processor() + template = "SELECT * FROM table" + + # Mock the Environment.from_string to raise a security error + with patch.object( + processor.env, "from_string", side_effect=SecurityError("Access denied") + ): + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + error = exception.errors[0] + + # Should get client error message with SecurityError type + assert "Jinja2 template error" in error.message + assert "SecurityError" in error.message + assert "Access denied" in error.message + assert exception.status == 422 + + +def test_jinja2_server_template_error(): + """Test handling of server-side Jinja2 template errors""" + from unittest.mock import patch + + from superset.exceptions import SupersetTemplateException + + processor = create_mock_processor() + template = "SELECT * FROM table" + + # Mock the Environment.from_string to raise a server error + with patch.object( + processor.env, "from_string", side_effect=MemoryError("Out of memory") + ): + with pytest.raises(SupersetTemplateException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + + # Should get server error message (500) + assert "Internal error processing Jinja2 template" in str(exception) + assert "contact your administrator" in str(exception) + assert "Out of memory" in str(exception) diff --git a/tests/unit_tests/utils/test_jinja_template_validator.py b/tests/unit_tests/utils/test_jinja_template_validator.py new file mode 100644 index 000000000000..14a0e6873b3e --- /dev/null +++ b/tests/unit_tests/utils/test_jinja_template_validator.py @@ -0,0 +1,111 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import pytest + +from superset.utils.jinja_template_validator import ( + sanitize_jinja_template, + validate_jinja_template, + validate_params_json_with_jinja, +) + + +def test_validate_jinja_template_valid(): + """Test valid Jinja2 templates.""" + # Simple variable + is_valid, error = validate_jinja_template("{{ variable }}") + assert is_valid is True + assert error is None + + # With filter + is_valid, error = validate_jinja_template("{{ variable | default('value') }}") + assert is_valid is True + assert error is None + + # SQL with Jinja2 + is_valid, error = validate_jinja_template("WHERE date = {{ date }}") + assert is_valid is True + assert error is None + + # Empty string + is_valid, error = validate_jinja_template("") + assert is_valid is True + assert error is None + + +def test_validate_jinja_template_invalid(): + """Test invalid Jinja2 templates with common mistakes.""" + # The "such as" mistake + is_valid, error = validate_jinja_template("{{ variable such as 'value' }}") + assert is_valid is False + assert "Invalid Jinja2 syntax" in error + assert "Use {{ variable }}" in error or "default" in error + + # Unclosed block + is_valid, error = validate_jinja_template("{{ variable ") + assert is_valid is False + assert "syntax" in error.lower() or "unclosed" in error.lower() + + # Text after closing brace + is_valid, error = validate_jinja_template("{{ variable }} extra text {{") + assert is_valid is False + assert "syntax" in error.lower() or "unexpected" in error.lower() + + +def test_sanitize_jinja_template(): + """Test template sanitization for common mistakes.""" + # Fix "such as" pattern + result = sanitize_jinja_template("{{ variable such as 'value' }}") + assert result == "{{ variable | default('value') }}" + + # Leave valid templates unchanged + result = sanitize_jinja_template("{{ variable | default('value') }}") + assert result == "{{ variable | default('value') }}" + + # Handle empty string + result = sanitize_jinja_template("") + assert result == "" + + +def test_validate_params_json_with_jinja(): + """Test the combined JSON + Jinja2 validation function.""" + from marshmallow import ValidationError + + from superset.utils import json + + # Valid JSON with valid templates + valid_params = { + "adhoc_filters": [ + {"sqlExpression": "column = {{ variable | default('value') }}"} + ] + } + # Should not raise any exception + validate_params_json_with_jinja(json.dumps(valid_params)) + + # Invalid JSON + with pytest.raises(ValidationError, match="Invalid JSON"): + validate_params_json_with_jinja("{invalid json") + + # Valid JSON with invalid Jinja2 + invalid_params = { + "adhoc_filters": [{"sqlExpression": "column = {{ variable such as 'value' }}"}] + } + with pytest.raises(ValidationError, match="Invalid Jinja2 template"): + validate_params_json_with_jinja(json.dumps(invalid_params)) + + # None value should pass + validate_params_json_with_jinja(None) From 9dc2fa44acdbef5fdc2d915ea9eb7a98ef052a1e Mon Sep 17 00:00:00 2001 From: Gabriel Torres Ruiz Date: Sun, 24 Aug 2025 17:31:52 -0300 Subject: [PATCH 2/5] fix(error): address PR comments + fixes --- superset/jinja_context.py | 12 +- superset/utils/jinja_template_validator.py | 89 ++---- tests/unit_tests/jinja_context_test.py | 131 ++++++++ tests/unit_tests/jinja_template_error_test.py | 296 ------------------ .../utils/test_jinja_template_validator.py | 64 ++-- 5 files changed, 184 insertions(+), 408 deletions(-) delete mode 100644 tests/unit_tests/jinja_template_error_test.py diff --git a/superset/jinja_context.py b/superset/jinja_context.py index 670b0153e84a..d43744fac255 100644 --- a/superset/jinja_context.py +++ b/superset/jinja_context.py @@ -709,11 +709,7 @@ def process_template(self, sql: str, **kwargs: Any) -> str: error_msg = str(ex) exception_type = type(ex).__name__ - message = ( - f"Jinja2 template error ({exception_type}). " - "Please check your template syntax and variable references. " - f"Original error: {error_msg}" - ) + message = f"Jinja2 template error ({exception_type}): {error_msg}" line_number = getattr(ex, "lineno", None) @@ -746,11 +742,7 @@ def process_template(self, sql: str, **kwargs: Any) -> str: error_msg = str(ex) exception_type = type(ex).__name__ - message = ( - "Internal error processing Jinja2 template. " - "Please contact your administrator. " - f"Error: {error_msg}" - ) + message = f"Internal Jinja2 template error ({exception_type}): {error_msg}" logger.error( "Jinja2 template server error", diff --git a/superset/utils/jinja_template_validator.py b/superset/utils/jinja_template_validator.py index 90002700ea95..6e5d1396fb33 100644 --- a/superset/utils/jinja_template_validator.py +++ b/superset/utils/jinja_template_validator.py @@ -17,8 +17,7 @@ under the License. """ -import re -from typing import Any, Dict, Optional, Tuple +from typing import Any, Dict from jinja2 import TemplateSyntaxError from jinja2.sandbox import SandboxedEnvironment @@ -27,85 +26,55 @@ from superset.utils import json -def validate_jinja_template(template_str: str) -> Tuple[bool, Optional[str]]: +class JinjaValidationError(Exception): + """Exception raised when Jinja2 template validation fails.""" + + def __init__(self, message: str): + self.message = message + super().__init__(message) + + +def validate_jinja_template(template_str: str) -> None: """ Validates Jinja2 template syntax. - Returns: - Tuple of (is_valid, error_message) - If valid: (True, None) - If invalid: (False, "Error description") + Args: + template_str: The template string to validate + + Raises: + JinjaValidationError: If the template syntax is invalid """ if not template_str: - return True, None + return try: env = SandboxedEnvironment() env.from_string(template_str) - - return True, None - except TemplateSyntaxError as e: - error_msg = str(e) - - if "expected token 'end of print statement'" in error_msg: - if "such" in template_str.lower(): - return False, ( - "Invalid Jinja2 syntax. Found text after variable name. " - "Use {{ variable }} for simple variables or " - "{{ variable | default('value') }} for defaults. " - f"Error: {error_msg}" - ) - return False, ( - "Invalid Jinja2 syntax. " - "Check that all {{ }} blocks are properly closed. " - f"Error: {error_msg}" - ) - elif "unexpected end of template" in error_msg: - return False, ( - "Unclosed Jinja2 block. " - "Make sure all {{ and {% blocks have closing }} and %. " - f"Error: {error_msg}" - ) - elif "expected token" in error_msg: - return False, (f"Invalid Jinja2 syntax. {error_msg}") - else: - return False, f"Template syntax error: {error_msg}" - + raise JinjaValidationError(f"Invalid Jinja2 template syntax: {str(e)}") from e except Exception as e: - return False, f"Template validation error: {str(e)}" - - -def sanitize_jinja_template(template_str: str) -> str: - """ - Attempts to fix common Jinja2 template mistakes. - - This is a best-effort function that tries to fix obvious errors. - """ - if not template_str: - return template_str - - pattern = r"\{\{\s*(\w+)\s+such\s+as\s+([^}]+?)\s*\}\}" - template_str = re.sub(pattern, r"{{ \1 | default(\2) }}", template_str) - - return template_str + raise JinjaValidationError(f"Template validation error: {str(e)}") from e def _check_filter_sql_expression(sql_expression: str) -> None: """Check SQL expression for valid Jinja2.""" - is_valid, error_msg = validate_jinja_template(sql_expression) - if not is_valid: - raise ValidationError(f"Invalid Jinja2 template in SQL filter: {error_msg}") + try: + validate_jinja_template(sql_expression) + except JinjaValidationError as e: + raise ValidationError( + f"Invalid Jinja2 template in SQL filter: {e.message}" + ) from e def _check_filter_clause(clause: str) -> None: """Check filter clause for valid Jinja2.""" if "{{" in clause: - is_valid, error_msg = validate_jinja_template(clause) - if not is_valid: + try: + validate_jinja_template(clause) + except JinjaValidationError as e: raise ValidationError( - f"Invalid Jinja2 template in WHERE clause: {error_msg}" - ) + f"Invalid Jinja2 template in WHERE clause: {e.message}" + ) from e def _check_filter_dict(filter_dict: Dict[str, Any]) -> None: diff --git a/tests/unit_tests/jinja_context_test.py b/tests/unit_tests/jinja_context_test.py index dfadbd00d7cd..30ada6968174 100644 --- a/tests/unit_tests/jinja_context_test.py +++ b/tests/unit_tests/jinja_context_test.py @@ -1435,3 +1435,134 @@ def test_get_time_filter( assert cache.get_time_filter(*args, **kwargs) == time_filter, description assert cache.removed_filters == removed_filters assert cache.applied_filters == applied_filters + + +def test_jinja2_template_syntax_error_handling(mocker: MockerFixture) -> None: + """Test TemplateSyntaxError handling with proper error message and 422 status""" + from superset.errors import SupersetErrorType + from superset.exceptions import SupersetSyntaxErrorException + + database = mocker.MagicMock() + database.db_engine_spec = mocker.MagicMock() + + from superset.jinja_context import BaseTemplateProcessor + + processor = BaseTemplateProcessor(database=database) + + # Test with invalid Jinja2 syntax + template = "SELECT * WHERE column = {{ variable such as 'default' }}" + + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + assert len(exception.errors) == 1 + error = exception.errors[0] + + # Verify error message contains helpful guidance + assert "Jinja2 template error" in error.message + assert "TemplateSyntaxError" in error.message + assert "expected token" in error.message + + # Verify error type and status + assert error.error_type == SupersetErrorType.GENERIC_COMMAND_ERROR + assert exception.status == 422 + + # Verify extra data includes template snippet + assert "template" in error.extra + assert error.extra["template"][:50] == template[:50] + + +def test_jinja2_undefined_error_handling(mocker: MockerFixture) -> None: + """Test that UndefinedError is handled as client error""" + from unittest.mock import patch + + from jinja2.exceptions import UndefinedError + + from superset.exceptions import SupersetSyntaxErrorException + + database = mocker.MagicMock() + database.db_engine_spec = mocker.MagicMock() + + from superset.jinja_context import BaseTemplateProcessor + + processor = BaseTemplateProcessor(database=database) + template = "SELECT * FROM table" + + # Mock the Environment.from_string to raise UndefinedError + with patch.object( + processor.env, "from_string", side_effect=UndefinedError("Variable not defined") + ): + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + error = exception.errors[0] + + # Should get client error message (422) + assert "Jinja2 template error" in error.message + assert "UndefinedError" in error.message + assert "Variable not defined" in error.message + assert exception.status == 422 + + +def test_jinja2_security_error_handling(mocker: MockerFixture) -> None: + """Test that SecurityError is handled as client error""" + from unittest.mock import patch + + from jinja2.exceptions import SecurityError + + from superset.exceptions import SupersetSyntaxErrorException + + database = mocker.MagicMock() + database.db_engine_spec = mocker.MagicMock() + + from superset.jinja_context import BaseTemplateProcessor + + processor = BaseTemplateProcessor(database=database) + template = "SELECT * FROM table" + + # Mock the Environment.from_string to raise SecurityError + with patch.object( + processor.env, "from_string", side_effect=SecurityError("Access denied") + ): + with pytest.raises(SupersetSyntaxErrorException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + error = exception.errors[0] + + # Should get client error message with SecurityError type + assert "Jinja2 template error" in error.message + assert "SecurityError" in error.message + assert "Access denied" in error.message + assert exception.status == 422 + + +def test_jinja2_server_error_handling(mocker: MockerFixture) -> None: + """Test that server errors (like MemoryError) are handled with 500 status""" + from unittest.mock import patch + + from superset.exceptions import SupersetTemplateException + + database = mocker.MagicMock() + database.db_engine_spec = mocker.MagicMock() + + from superset.jinja_context import BaseTemplateProcessor + + processor = BaseTemplateProcessor(database=database) + template = "SELECT * FROM table" + + # Mock the Environment.from_string to raise MemoryError (server error) + with patch.object( + processor.env, "from_string", side_effect=MemoryError("Out of memory") + ): + with pytest.raises(SupersetTemplateException) as exc_info: + processor.process_template(template) + + exception = exc_info.value + + # Should get server error message (500) + assert "Internal Jinja2 template error" in str(exception) + assert "MemoryError" in str(exception) + assert "Out of memory" in str(exception) diff --git a/tests/unit_tests/jinja_template_error_test.py b/tests/unit_tests/jinja_template_error_test.py deleted file mode 100644 index 9aa906b674b8..000000000000 --- a/tests/unit_tests/jinja_template_error_test.py +++ /dev/null @@ -1,296 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -from unittest.mock import MagicMock - -import pytest - -from superset.errors import SupersetErrorType -from superset.exceptions import SupersetSyntaxErrorException -from superset.jinja_context import BaseTemplateProcessor -from superset.utils import json - - -def create_mock_processor(): - """Create a BaseTemplateProcessor with mocked database""" - mock_database = MagicMock() - mock_database.db_engine_spec = MagicMock() - return BaseTemplateProcessor(database=mock_database) - - -def test_jinja2_syntax_error_general(): - """Test handling of general Jinja2 syntax errors""" - processor = create_mock_processor() - template = "SELECT * WHERE column = {{ variable such as 'default' }}" - - with pytest.raises(SupersetSyntaxErrorException) as exc_info: - processor.process_template(template) - - # Check the exception details - exception = exc_info.value - assert len(exception.errors) == 1 - error = exception.errors[0] - - # Verify error message contains helpful guidance - assert "Jinja2 template error" in error.message - assert "TemplateSyntaxError" in error.message - assert "check your template syntax" in error.message - assert "Original error:" in error.message - - # Verify error type and status - assert error.error_type == SupersetErrorType.GENERIC_COMMAND_ERROR - assert exception.status == 422 - - # Verify extra data includes template snippet - assert "template" in error.extra - assert error.extra["template"][:50] == template[:50] - - -def test_jinja2_syntax_error_unclosed_block(): - """Test handling of unclosed Jinja2 blocks""" - processor = create_mock_processor() - template = "SELECT * WHERE column = {{ variable " - - with pytest.raises(SupersetSyntaxErrorException) as exc_info: - processor.process_template(template) - - exception = exc_info.value - error = exception.errors[0] - - # Should get general syntax error with helpful guidance - assert "Jinja2 template error" in error.message - assert "TemplateSyntaxError" in error.message - assert "check your template syntax" in error.message - - -def test_jinja2_syntax_error_unexpected_end(): - """Test handling of unexpected end of template""" - processor = create_mock_processor() - template = "SELECT * WHERE {% if condition" - - with pytest.raises(SupersetSyntaxErrorException) as exc_info: - processor.process_template(template) - - exception = exc_info.value - error = exception.errors[0] - - # Should get general syntax error with helpful guidance - assert "Jinja2 template error" in error.message - assert "TemplateSyntaxError" in error.message - - -def test_datadog_structured_logging_data(): - """Test that errors include structured data for Datadog logging""" - # We can't easily mock the logger due to module-level instantiation, - # but we can test that the error includes the structured data we need - processor = create_mock_processor() - template = "SELECT * WHERE column = {{ variable such as 'default' }}" - - with pytest.raises(SupersetSyntaxErrorException) as exc_info: - processor.process_template(template) - - exception = exc_info.value - error = exception.errors[0] - - # Verify error contains structured data that would be useful for Datadog - assert error.error_type == SupersetErrorType.GENERIC_COMMAND_ERROR - assert "template" in error.extra - assert "line" in error.extra - assert error.extra["template"] == template[:500] # Truncated for logging - - # The actual logging happens inside the function - we can see it in captured logs - # This is sufficient to verify the functionality is working - - -def test_valid_jinja2_template_passes(): - """Test that valid templates work correctly""" - processor = create_mock_processor() - valid_templates = [ - "SELECT * WHERE column = {{ variable }}", - "SELECT * WHERE column = {{ variable | default('value') }}", - "SELECT * WHERE column = {{ variable or 'value' }}", - "{% if condition %}SELECT * {% endif %}", - ] - - for template in valid_templates: - # Should not raise any exception - result = processor.process_template(template) - assert result is not None - - -def test_api_response_format(): - """Test that the API response format is correct for template errors""" - processor = create_mock_processor() - template = "SELECT * WHERE column = {{ variable such as 'default' }}" - - with pytest.raises(SupersetSyntaxErrorException) as exc_info: - processor.process_template(template) - - exception = exc_info.value - - # Simulate what the API error handler would do - errors_dict = [ - { - "message": error.message, - "error_type": error.error_type.name, - "level": error.level.name, - "extra": error.extra, - } - for error in exception.errors - ] - - # This is what would be sent to the client - response_data = {"errors": errors_dict} - - # Verify the structure - assert "errors" in response_data - assert len(response_data["errors"]) == 1 - - error_data = response_data["errors"][0] - assert "message" in error_data - assert "error_type" in error_data - assert "level" in error_data - assert "extra" in error_data - - # Verify it's JSON serializable (important for API responses) - json_str = json.dumps(response_data) - assert json_str is not None - - -def test_chart_params_validation(): - """Test that chart params with invalid Jinja2 are rejected""" - from marshmallow import ValidationError - - from superset.utils.jinja_template_validator import ( - validate_jinja_template_in_params, - ) - - # Invalid template in adhoc_filters - invalid_params = { - "adhoc_filters": [ - { - "sqlExpression": "column = {{ variable such as 'value' }}", - "clause": "WHERE", - } - ] - } - - with pytest.raises(ValidationError) as exc_info: - validate_jinja_template_in_params(invalid_params) - - error_msg = str(exc_info.value) - assert "Invalid Jinja2 template" in error_msg - - -def test_chart_params_validation_passes_valid(): - """Test that valid Jinja2 templates pass validation""" - from superset.utils.jinja_template_validator import ( - validate_jinja_template_in_params, - ) - - valid_params = { - "adhoc_filters": [ - { - "sqlExpression": "column = {{ variable | default('value') }}", - "clause": "WHERE", - } - ], - "where": "date >= {{ from_date }}", - } - - # Should not raise any exception - validate_jinja_template_in_params(valid_params) - - -def test_jinja2_client_template_error(): - """Test handling of client-side Jinja2 template errors""" - from unittest.mock import patch - - from jinja2 import UndefinedError - - processor = create_mock_processor() - template = "SELECT * FROM table" - - # Mock the Environment.from_string to raise a client error - with patch.object( - processor.env, "from_string", side_effect=UndefinedError("Variable not defined") - ): - with pytest.raises(SupersetSyntaxErrorException) as exc_info: - processor.process_template(template) - - exception = exc_info.value - error = exception.errors[0] - - # Should get client error message (422) - assert "Jinja2 template error" in error.message - assert "UndefinedError" in error.message - assert "check your template syntax" in error.message - assert "Variable not defined" in error.message - assert exception.status == 422 - - # Verify it includes exception type info - assert "UndefinedError" in error.extra.get("exception_type", "") - - -def test_jinja2_security_error(): - """Test handling of SecurityError as client error with proper exception type""" - from unittest.mock import patch - - from jinja2.exceptions import SecurityError - - processor = create_mock_processor() - template = "SELECT * FROM table" - - # Mock the Environment.from_string to raise a security error - with patch.object( - processor.env, "from_string", side_effect=SecurityError("Access denied") - ): - with pytest.raises(SupersetSyntaxErrorException) as exc_info: - processor.process_template(template) - - exception = exc_info.value - error = exception.errors[0] - - # Should get client error message with SecurityError type - assert "Jinja2 template error" in error.message - assert "SecurityError" in error.message - assert "Access denied" in error.message - assert exception.status == 422 - - -def test_jinja2_server_template_error(): - """Test handling of server-side Jinja2 template errors""" - from unittest.mock import patch - - from superset.exceptions import SupersetTemplateException - - processor = create_mock_processor() - template = "SELECT * FROM table" - - # Mock the Environment.from_string to raise a server error - with patch.object( - processor.env, "from_string", side_effect=MemoryError("Out of memory") - ): - with pytest.raises(SupersetTemplateException) as exc_info: - processor.process_template(template) - - exception = exc_info.value - - # Should get server error message (500) - assert "Internal error processing Jinja2 template" in str(exception) - assert "contact your administrator" in str(exception) - assert "Out of memory" in str(exception) diff --git a/tests/unit_tests/utils/test_jinja_template_validator.py b/tests/unit_tests/utils/test_jinja_template_validator.py index 14a0e6873b3e..c38860920dce 100644 --- a/tests/unit_tests/utils/test_jinja_template_validator.py +++ b/tests/unit_tests/utils/test_jinja_template_validator.py @@ -18,7 +18,7 @@ import pytest from superset.utils.jinja_template_validator import ( - sanitize_jinja_template, + JinjaValidationError, validate_jinja_template, validate_params_json_with_jinja, ) @@ -26,59 +26,39 @@ def test_validate_jinja_template_valid(): """Test valid Jinja2 templates.""" - # Simple variable - is_valid, error = validate_jinja_template("{{ variable }}") - assert is_valid is True - assert error is None + # Simple variable - should not raise any exception + validate_jinja_template("{{ variable }}") - # With filter - is_valid, error = validate_jinja_template("{{ variable | default('value') }}") - assert is_valid is True - assert error is None + # With filter - should not raise any exception + validate_jinja_template("{{ variable | default('value') }}") - # SQL with Jinja2 - is_valid, error = validate_jinja_template("WHERE date = {{ date }}") - assert is_valid is True - assert error is None + # SQL with Jinja2 - should not raise any exception + validate_jinja_template("WHERE date = {{ date }}") - # Empty string - is_valid, error = validate_jinja_template("") - assert is_valid is True - assert error is None + # Empty string - should not raise any exception + validate_jinja_template("") def test_validate_jinja_template_invalid(): """Test invalid Jinja2 templates with common mistakes.""" # The "such as" mistake - is_valid, error = validate_jinja_template("{{ variable such as 'value' }}") - assert is_valid is False - assert "Invalid Jinja2 syntax" in error - assert "Use {{ variable }}" in error or "default" in error + with pytest.raises(JinjaValidationError) as exc_info: + validate_jinja_template("{{ variable such as 'value' }}") + error = exc_info.value.message + assert "Invalid Jinja2 template syntax" in error + assert "expected token" in error # Unclosed block - is_valid, error = validate_jinja_template("{{ variable ") - assert is_valid is False - assert "syntax" in error.lower() or "unclosed" in error.lower() + with pytest.raises(JinjaValidationError) as exc_info: + validate_jinja_template("{{ variable ") + error = exc_info.value.message + assert "Invalid Jinja2 template syntax" in error # Text after closing brace - is_valid, error = validate_jinja_template("{{ variable }} extra text {{") - assert is_valid is False - assert "syntax" in error.lower() or "unexpected" in error.lower() - - -def test_sanitize_jinja_template(): - """Test template sanitization for common mistakes.""" - # Fix "such as" pattern - result = sanitize_jinja_template("{{ variable such as 'value' }}") - assert result == "{{ variable | default('value') }}" - - # Leave valid templates unchanged - result = sanitize_jinja_template("{{ variable | default('value') }}") - assert result == "{{ variable | default('value') }}" - - # Handle empty string - result = sanitize_jinja_template("") - assert result == "" + with pytest.raises(JinjaValidationError) as exc_info: + validate_jinja_template("{{ variable }} extra text {{") + error = exc_info.value.message + assert "Invalid Jinja2 template syntax" in error def test_validate_params_json_with_jinja(): From d5945f9f2f89c69809a87e894b3d812de24f9856 Mon Sep 17 00:00:00 2001 From: Gabriel Torres Ruiz Date: Mon, 25 Aug 2025 15:28:18 -0300 Subject: [PATCH 3/5] fix(error): fix failing tests on CI --- superset/charts/schemas.py | 5 ++-- superset/connectors/sqla/models.py | 44 +++++++++++++++++++++++++----- superset/connectors/sqla/utils.py | 12 ++++++-- superset/datasets/api.py | 14 +++++++--- superset/models/helpers.py | 11 ++++++-- 5 files changed, 67 insertions(+), 19 deletions(-) diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index 692d1715ccda..2e3dec7fd856 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -37,7 +37,6 @@ PostProcessingBoxplotWhiskerType, PostProcessingContributionOrientation, ) -from superset.utils.jinja_template_validator import validate_params_json_with_jinja if TYPE_CHECKING: from superset.common.query_context import QueryContext @@ -209,7 +208,7 @@ class ChartPostSchema(Schema): params = fields.String( metadata={"description": params_description}, allow_none=True, - validate=validate_params_json_with_jinja, + validate=utils.validate_json, ) query_context = fields.String( metadata={"description": query_context_description}, @@ -272,7 +271,7 @@ class ChartPutSchema(Schema): params = fields.String( metadata={"description": params_description}, allow_none=True, - validate=validate_params_json_with_jinja, + validate=utils.validate_json, ) query_context = fields.String( metadata={"description": query_context_description}, allow_none=True diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index d6fe46ba5f98..d8c2da229824 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -85,6 +85,7 @@ SupersetErrorsException, SupersetGenericDBErrorException, SupersetSecurityException, + SupersetSyntaxErrorException, ) from superset.jinja_context import ( BaseTemplateProcessor, @@ -686,11 +687,12 @@ def get_sqla_row_level_filters( grouped_filters = [or_(*clauses) for clauses in filter_groups.values()] all_filters.extend(grouped_filters) return all_filters - except TemplateError as ex: + except (TemplateError, SupersetSyntaxErrorException) as ex: + msg = ex.message if hasattr(ex, "message") else str(ex) raise QueryObjectValidationError( _( "Error in jinja expression in RLS filters: %(msg)s", - msg=ex.message, + msg=msg, ) ) from ex @@ -893,7 +895,16 @@ def get_sqla_col( type_ = column_spec.sqla_type if column_spec else None if expression := self.expression: if template_processor: - expression = template_processor.process_template(expression) + try: + expression = template_processor.process_template(expression) + except SupersetSyntaxErrorException as ex: + msg = str(ex) + raise QueryObjectValidationError( + _( + "Error in jinja expression in column expression: %(msg)s", + msg=msg, + ) + ) from ex col = literal_column(expression, type_=type_) else: col = column(self.column_name, type_=type_) @@ -931,7 +942,16 @@ def get_timestamp_expression( return self.database.make_sqla_column_compatible(sqla_col, label) if expression := self.expression: if template_processor: - expression = template_processor.process_template(expression) + try: + expression = template_processor.process_template(expression) + except SupersetSyntaxErrorException as ex: + msg = str(ex) + raise QueryObjectValidationError( + _( + "Error in jinja expression in datetime column: %(msg)s", + msg=msg, + ) + ) from ex col = literal_column(expression, type_=type_) else: col = column(self.column_name, type_=type_) @@ -1012,7 +1032,16 @@ def get_sqla_col( label = label or self.metric_name expression = self.expression if template_processor: - expression = template_processor.process_template(expression) + try: + expression = template_processor.process_template(expression) + except SupersetSyntaxErrorException as ex: + msg = str(ex) + raise QueryObjectValidationError( + _( + "Error in jinja expression in metric expression: %(msg)s", + msg=msg, + ) + ) from ex sqla_col: ColumnClause = literal_column(expression) return self.table.database.make_sqla_column_compatible(sqla_col, label) @@ -1356,11 +1385,12 @@ def get_fetch_values_predicate( ) try: return self.text(fetch_values_predicate) - except TemplateError as ex: + except (TemplateError, SupersetSyntaxErrorException) as ex: + msg = ex.message if hasattr(ex, "message") else str(ex) raise QueryObjectValidationError( _( "Error in jinja expression in fetch values predicate: %(msg)s", - msg=ex.message, + msg=msg, ) ) from ex diff --git a/superset/connectors/sqla/utils.py b/superset/connectors/sqla/utils.py index 2e7264964e96..0a9c911de363 100644 --- a/superset/connectors/sqla/utils.py +++ b/superset/connectors/sqla/utils.py @@ -36,6 +36,7 @@ SupersetGenericDBErrorException, SupersetParseError, SupersetSecurityException, + SupersetSyntaxErrorException, ) from superset.models.core import Database from superset.result_set import SupersetResultSet @@ -103,9 +104,14 @@ def get_virtual_table_metadata(dataset: SqlaTable) -> list[ResultSetColumnType]: ) db_engine_spec = dataset.database.db_engine_spec - sql = dataset.get_template_processor().process_template( - dataset.sql, **dataset.template_params_dict - ) + try: + sql = dataset.get_template_processor().process_template( + dataset.sql, **dataset.template_params_dict + ) + except SupersetSyntaxErrorException as ex: + raise SupersetGenericDBErrorException( + message=_("Template processing error: %(error)s", error=str(ex)), + ) from ex try: parsed_script = SQLScript(sql, engine=db_engine_spec.engine) except SupersetParseError as ex: diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 6821b837e3cc..fd755e066b3f 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -76,7 +76,10 @@ GetOrCreateDatasetSchema, openapi_spec_methods_override, ) -from superset.exceptions import SupersetTemplateException +from superset.exceptions import ( + SupersetSyntaxErrorException, + SupersetTemplateException, +) from superset.jinja_context import BaseTemplateProcessor, get_template_processor from superset.utils import json from superset.utils.core import parse_boolean_string @@ -1315,9 +1318,12 @@ def render_item_list(item_list: list[dict[str, Any]]) -> list[dict[str, Any]]: try: data[new_key] = func(data[key]) - except TemplateSyntaxError as ex: - raise SupersetTemplateException( + except (TemplateSyntaxError, SupersetSyntaxErrorException) as ex: + template_exception = SupersetTemplateException( f"Unable to render expression from dataset {item_type}.", - ) from ex + ) + + template_exception.status = 400 + raise template_exception from ex return data diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 07944c25effe..a66e4c4ecea1 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -74,6 +74,7 @@ QueryClauseValidationException, QueryObjectValidationError, SupersetSecurityException, + SupersetSyntaxErrorException, ) from superset.extensions import feature_flag_manager from superset.jinja_context import BaseTemplateProcessor @@ -1082,11 +1083,17 @@ def get_rendered_sql( if template_processor: try: sql = template_processor.process_template(sql) - except TemplateError as ex: + except (TemplateError, SupersetSyntaxErrorException) as ex: + # Extract error message from different exception types + if isinstance(ex, TemplateError): + error_msg = ex.message + else: # SupersetSyntaxErrorException + error_msg = str(ex.errors[0].message if ex.errors else ex) + raise QueryObjectValidationError( _( "Error while rendering virtual dataset query: %(msg)s", - msg=ex.message, + msg=error_msg, ) ) from ex From af551f34e12038968024b46e5a35994cf261db03 Mon Sep 17 00:00:00 2001 From: Gabriel Torres Ruiz Date: Thu, 28 Aug 2025 15:33:32 -0300 Subject: [PATCH 4/5] fix(error): code improvement --- superset/connectors/sqla/models.py | 4 ++-- superset/models/helpers.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index d8c2da229824..ba07a5d148b3 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -688,7 +688,7 @@ def get_sqla_row_level_filters( all_filters.extend(grouped_filters) return all_filters except (TemplateError, SupersetSyntaxErrorException) as ex: - msg = ex.message if hasattr(ex, "message") else str(ex) + msg = getattr(ex, "message", str(ex)) raise QueryObjectValidationError( _( "Error in jinja expression in RLS filters: %(msg)s", @@ -1386,7 +1386,7 @@ def get_fetch_values_predicate( try: return self.text(fetch_values_predicate) except (TemplateError, SupersetSyntaxErrorException) as ex: - msg = ex.message if hasattr(ex, "message") else str(ex) + msg = getattr(ex, "message", str(ex)) raise QueryObjectValidationError( _( "Error in jinja expression in fetch values predicate: %(msg)s", diff --git a/superset/models/helpers.py b/superset/models/helpers.py index a66e4c4ecea1..7d6c846da41f 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -1512,7 +1512,7 @@ def validate_expression( ) except Exception as ex: # Convert any exception to validation error format - error_msg = str(ex.orig) if hasattr(ex, "orig") else str(ex) + error_msg = str(getattr(ex, "orig", ex)) return ValidationResultDict( valid=False, errors=[ From e62695f8be146b3b9d9276a710f6143eee53255b Mon Sep 17 00:00:00 2001 From: Gabriel Torres Ruiz Date: Tue, 2 Sep 2025 17:11:10 -0300 Subject: [PATCH 5/5] fix(error): SupersetTemplateException error status to 422 --- superset/datasets/api.py | 4 +--- superset/exceptions.py | 2 +- tests/integration_tests/datasets/api_tests.py | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/superset/datasets/api.py b/superset/datasets/api.py index fd755e066b3f..92589309e199 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -1178,7 +1178,7 @@ def get(self, id_or_uuid: str, **kwargs: Any) -> Response: response["result"], processor ) except SupersetTemplateException as ex: - return self.response_400(message=str(ex)) + return self.response(ex.status, message=str(ex)) return self.response(200, **response) @@ -1322,8 +1322,6 @@ def render_item_list(item_list: list[dict[str, Any]]) -> list[dict[str, Any]]: template_exception = SupersetTemplateException( f"Unable to render expression from dataset {item_type}.", ) - - template_exception.status = 400 raise template_exception from ex return data diff --git a/superset/exceptions.py b/superset/exceptions.py index 1b3216da0c72..4967ffbfa81c 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -188,7 +188,7 @@ class NullValueException(SupersetException): class SupersetTemplateException(SupersetException): - pass + status = 422 class SpatialException(SupersetException): diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index d9995896281f..6677f19f8ad1 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -548,7 +548,7 @@ def test_get_dataset_render_jinja_exceptions(self): uri = f"api/v1/dataset/{dataset.id}?q=(columns:!(id,sql))&include_rendered_sql=true" # noqa: E501 rv = self.get_assert_metric(uri, "get") - assert rv.status_code == 400 + assert rv.status_code == 422 response = json.loads(rv.data.decode("utf-8")) assert response["message"] == "Unable to render expression from dataset query." @@ -557,7 +557,7 @@ def test_get_dataset_render_jinja_exceptions(self): "&include_rendered_sql=true&multiplier=4" ) rv = self.get_assert_metric(uri, "get") - assert rv.status_code == 400 + assert rv.status_code == 422 response = json.loads(rv.data.decode("utf-8")) assert response["message"] == "Unable to render expression from dataset metric." @@ -566,7 +566,7 @@ def test_get_dataset_render_jinja_exceptions(self): "&include_rendered_sql=true" ) rv = self.get_assert_metric(uri, "get") - assert rv.status_code == 400 + assert rv.status_code == 422 response = json.loads(rv.data.decode("utf-8")) assert ( response["message"]