You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/10/20 18:37:47 UTC
(superset) branch master updated: fix(semantic-layers): report async chart query validation errors as failures (#45525)
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 036d1dafe2c fix(semantic-layers): report async chart query validation errors as failures (#45525)
036d1dafe2c is described below
commit 036d1dafe2c9f8c1418c538306a83d9413dd0cb9
Author: Mike Bridge <mi...@preset.io>
AuthorDate: Tue Oct 20 12:37:38 2026 -0600
fix(semantic-layers): report async chart query validation errors as failures (#45525)
---
docs/developer_docs/semantic-query-errors.md | 6 +-
superset/tasks/async_queries.py | 19 ++++-
superset/tasks/schemas.py | 9 ++
.../semantic_layers/query_errors_test.py | 24 ++++--
tests/unit_tests/tasks/test_async_queries.py | 95 +++++++++++++++++++++-
tests/unit_tests/tasks/test_schemas.py | 62 ++++++++++++++
6 files changed, 203 insertions(+), 12 deletions(-)
diff --git a/docs/developer_docs/semantic-query-errors.md b/docs/developer_docs/semantic-query-errors.md
index 843c83b3982..8d2b7945d18 100644
--- a/docs/developer_docs/semantic-query-errors.md
+++ b/docs/developer_docs/semantic-query-errors.md
@@ -68,8 +68,10 @@ HTTP 500.
An asynchronous totals-prerequisite query whose payload is marked FAILED fails
its task with that payload's error. It does not publish a cache key for dependent
-contribution queries to read as if totals had been successfully cached. Other
-asynchronous chart queries retain their structured-error readback behavior.
+contribution queries to read as if totals had been successfully cached.
+Typed query validation failures also fail ordinary asynchronous chart tasks,
+without publishing a cache key or retrying the rejected query synchronously.
+Ordinary SQL engine errors retain their structured-error readback behavior.
Host-classified client errors (`QueryObjectValidationError` and subclasses) keep
their status. The shared SDK module also defines `SemanticResultCompletenessError`
diff --git a/superset/tasks/async_queries.py b/superset/tasks/async_queries.py
index 98e1865ed5f..7b2dcc5f828 100644
--- a/superset/tasks/async_queries.py
+++ b/superset/tasks/async_queries.py
@@ -23,6 +23,7 @@ from uuid import UUID
from flask import current_app
from flask_appbuilder.security.sqla.models import User
+from flask_babel import gettext as _
from superset_core.tasks.types import TaskOptions, TaskScope
from superset.common.db_query_status import QueryStatus
@@ -32,7 +33,12 @@ from superset.common.query_serialization import (
SerializedQuery,
)
from superset.constants import CacheRegion
-from superset.exceptions import SemanticResultCompletenessError, SupersetException
+from superset.exceptions import (
+ QueryObjectValidationError,
+ QueryObjectValidationMessage,
+ SemanticResultCompletenessError,
+ SupersetException,
+)
from superset.extensions import (
security_manager,
)
@@ -283,8 +289,8 @@ def execute_chart_query(
the existing per-query execution/caching path so a re-request reads the same
DATA-cache entry. The query runs under ``_capture_query_cancellation`` so an
abort/timeout can cancel it on engines that support query cancellation.
- Failed totals prerequisites must fail before advertising a cache key; ordinary
- tasks retain synchronous readback for structured errors.
+ Typed validation failures and failed totals prerequisites fail before advertising
+ a cache key; ordinary SQL engine errors retain structured-error readback.
"""
from superset.charts.data.form_data import set_query_context_form_data
@@ -332,6 +338,13 @@ def execute_chart_query(
payload={"semantic_result_error": error.reason}, immediate=True
)
raise
+ validation_error: object = result.payload.get("error")
+ if result.payload.get("status") == QueryStatus.FAILED and isinstance(
+ validation_error, QueryObjectValidationMessage
+ ):
+ raise QueryObjectValidationError(
+ _("Error: %(error)s", error=validation_error)
+ ) from validation_error.cause
if (
is_totals_prerequisite
and result.payload.get("status") == QueryStatus.FAILED
diff --git a/superset/tasks/schemas.py b/superset/tasks/schemas.py
index 3d12e19d6e8..f23a947298e 100644
--- a/superset/tasks/schemas.py
+++ b/superset/tasks/schemas.py
@@ -23,6 +23,9 @@ from marshmallow import fields, Schema
from marshmallow.fields import Method
from superset_core.tasks.types import TaskProperties
+from superset.tasks.async_queries import CHART_QUERY_TASK
+from superset.utils.error_sanitization import sanitize_error_message
+
# RISON/JSON schemas for query parameters
get_delete_ids_schema = {
"type": "array",
@@ -154,6 +157,12 @@ class TaskResponseSchema(Schema):
if not task_internals_visible():
properties.pop("private", None)
+ message: object = properties.get("error_message")
+ if getattr(obj, "task_type", None) == CHART_QUERY_TASK and isinstance(
+ message, str
+ ):
+ properties["error_message"] = sanitize_error_message(message)
+
return properties
def get_duration(self, obj: object) -> float | None:
diff --git a/tests/unit_tests/semantic_layers/query_errors_test.py b/tests/unit_tests/semantic_layers/query_errors_test.py
index 9e49721ada8..ef3d95bab1d 100644
--- a/tests/unit_tests/semantic_layers/query_errors_test.py
+++ b/tests/unit_tests/semantic_layers/query_errors_test.py
@@ -33,7 +33,7 @@ from superset.charts.schemas import ChartDataQueryContextSchema
from superset.common.query_context import QueryContext
from superset.common.query_object import QueryObject
from superset.common.query_serialization import serialize_query
-from superset.exceptions import SupersetException
+from superset.exceptions import QueryObjectValidationError
from superset.semantic_layers.mapper import get_results
from superset.semantic_layers.models import SemanticView
from superset.tasks.async_queries import execute_chart_query
@@ -207,10 +207,14 @@ def test_metadata_discovery_fault_http(
date_view.get_table.assert_not_called()
-def test_totals_prerequisite_refusal_does_not_publish_cache_key(
- app_context: None, date_view: MagicMock, mocker: MockerFixture
+@pytest.mark.parametrize("is_totals_prerequisite", [False, True])
+def test_async_validation_failure_does_not_publish_cache_key(
+ app_context: None,
+ date_view: MagicMock,
+ mocker: MockerFixture,
+ is_totals_prerequisite: bool,
) -> None:
- """A failed totals payload fails its task without advertising a cache entry."""
+ """Snowflake-shaped DATE refusals fail ordinary and totals workers as 400s."""
context: QueryContext = ChartDataQueryContextSchema().load(
{
"datasource": {"id": 7, "type": "semantic_view"},
@@ -234,10 +238,18 @@ def test_totals_prerequisite_refusal_does_not_publish_cache_key(
mocker.patch(
"superset.tasks.async_queries.load_serialized_query", return_value=context
)
- with pytest.raises(SupersetException, match="Invalid date value 'not-a-date'"):
+ captured: pytest.ExceptionInfo[QueryObjectValidationError]
+ with pytest.raises(QueryObjectValidationError) as captured:
execute_chart_query.func(
- serialize_query(context, 0), user_id=7, is_totals_prerequisite=True
+ serialize_query(context, 0),
+ user_id=7,
+ is_totals_prerequisite=is_totals_prerequisite,
)
+ assert captured.value.status == 400
+ assert str(captured.value) == (
+ "Error: Invalid date value 'not-a-date' for filter column ORDER_DATE"
+ )
+ assert isinstance(captured.value.__cause__, QueryObjectValidationError)
task_context.update_task.assert_not_called()
date_view.get_table.assert_not_called()
diff --git a/tests/unit_tests/tasks/test_async_queries.py b/tests/unit_tests/tasks/test_async_queries.py
index aaa48d3675b..068b6803059 100644
--- a/tests/unit_tests/tasks/test_async_queries.py
+++ b/tests/unit_tests/tasks/test_async_queries.py
@@ -17,16 +17,19 @@
"""Unit tests for the GTF chart-data fan-out orchestrator."""
from decimal import Decimal
-from typing import Literal, TYPE_CHECKING
+from typing import Any, Literal, TYPE_CHECKING
from unittest import mock
from uuid import uuid4
import pandas as pd
import pytest
from pytest_mock import MockerFixture
+from superset_core.tasks.types import TaskStatus
from superset.common.db_query_status import QueryStatus
from superset.common.query_serialization import SerializedQuery
+from superset.exceptions import QueryObjectValidationError, QueryObjectValidationMessage
+from superset.tasks.utils import error_update
if TYPE_CHECKING:
from superset_core.tasks.types import TaskProperties
@@ -187,6 +190,96 @@ def test_failed_non_totals_sql_query_preserves_readback(
)
+@pytest.mark.parametrize("validation_failure", [False, True])
+def test_chart_worker_scheduler_records_validation_failure(
+ app_context: None, mocker: MockerFixture, validation_failure: bool
+) -> None:
+ """The real executor records typed worker failures, not a success transition."""
+ # Avoid app-init regression: scheduler imports task models before the app fixture.
+ from superset.tasks.async_queries import execute_chart_query
+ from superset.tasks.scheduler import _execute_task_body
+
+ query_context: mock.MagicMock = mocker.MagicMock()
+ query_context.queries = [mocker.MagicMock()]
+ query_context.get_df_payload_result.return_value.payload = {
+ "status": QueryStatus.FAILED if validation_failure else QueryStatus.SUCCESS,
+ "error": QueryObjectValidationMessage(
+ QueryObjectValidationError("Invalid filter")
+ )
+ if validation_failure
+ else None,
+ "cache_key": "worker-key",
+ }
+ task: mock.MagicMock = mocker.MagicMock()
+ task.uuid = uuid4()
+ task.properties_dict = {}
+ task_context: mock.MagicMock = mocker.MagicMock()
+ task_context.aborting_in_flight = False
+ task_context.fence_triggered = False
+ task_context._abort_detected = False
+ task_context.timeout_triggered = False
+ task_context.error_properties.side_effect = error_update
+
+ def transition_status(**kwargs: Any) -> mock.MagicMock:
+ """Keep the fake DAO row in sync with the executor's actual transitions."""
+ task.status = kwargs["new_status"].value
+ return mock.MagicMock(run=mock.MagicMock(return_value=True))
+
+ transition: mock.MagicMock = mocker.patch(
+ "superset.commands.tasks.internal_update.InternalStatusTransitionCommand",
+ side_effect=transition_status,
+ )
+ mocker.patch("superset.tasks.scheduler.TaskContext", return_value=task_context)
+ mocker.patch("superset.tasks.scheduler.TaskDAO.find_one_or_none", return_value=task)
+ mocker.patch(
+ "superset.tasks.scheduler.TaskRegistry.get_executor",
+ return_value=execute_chart_query.func,
+ )
+ mocker.patch("superset.tasks.scheduler.TaskManager")
+ mocker.patch("superset.tasks.async_queries.get_context", return_value=task_context)
+ mocker.patch("superset.tasks.async_queries._resolve_user")
+ mocker.patch("superset.tasks.async_queries.override_user")
+ mocker.patch(
+ "superset.tasks.async_queries.load_serialized_query", return_value=query_context
+ )
+
+ result: dict[str, Any] = _execute_task_body(
+ task,
+ task.uuid,
+ "superset.query_object_v1",
+ (_serialized_query(),),
+ {},
+ mocker.MagicMock(),
+ )
+ assert result["status"] == (
+ TaskStatus.FAILURE.value if validation_failure else TaskStatus.SUCCESS.value
+ )
+ statuses: list[TaskStatus] = [
+ call.kwargs["new_status"] for call in transition.call_args_list
+ ]
+ assert statuses == [
+ TaskStatus.IN_PROGRESS,
+ TaskStatus.FAILURE if validation_failure else TaskStatus.SUCCESS,
+ ]
+ query_context.get_df_payload_result.assert_called_once()
+ if validation_failure:
+ assert (
+ transition.call_args.kwargs["properties"]["error_message"]
+ == "Error: Invalid filter"
+ )
+ assert (
+ transition.call_args.kwargs["properties"]["private"]["framework"][
+ "exception_type"
+ ]
+ == "QueryObjectValidationError"
+ )
+ task_context.update_task.assert_not_called()
+ else:
+ task_context.update_task.assert_called_once_with(
+ payload={"cache_key": "worker-key"}, immediate=True
+ )
+
+
def test_execute_chart_query_publishes_cache_key_payload(
mocker: MockerFixture,
) -> None:
diff --git a/tests/unit_tests/tasks/test_schemas.py b/tests/unit_tests/tasks/test_schemas.py
index 916d53906e4..123f147c288 100644
--- a/tests/unit_tests/tasks/test_schemas.py
+++ b/tests/unit_tests/tasks/test_schemas.py
@@ -14,11 +14,73 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from datetime import datetime
from types import SimpleNamespace
+from typing import Any
+from uuid import uuid4
+import pytest
+from flask.testing import FlaskClient
from pytest_mock import MockerFixture
+from werkzeug.test import TestResponse
from superset.tasks.schemas import TaskResponseSchema
+from superset.utils.error_sanitization import GENERIC_ERROR_MESSAGE
+
+
+@pytest.mark.parametrize("guest", [False, True])
+@pytest.mark.parametrize("task_type", ["superset.query_object_v1", "other_task"])
+def test_task_properties_sanitize_chart_errors_per_reader(
+ mocker: MockerFixture, guest: bool, task_type: str
+) -> None:
+ """List and detail serialization redact chart errors without changing storage."""
+ task: SimpleNamespace = _task_with_private()
+ task.task_type = task_type
+ mocker.patch("superset.tasks.utils.current_app").debug = False
+ mocker.patch("superset.security_manager.is_guest_user", return_value=guest)
+ schema: TaskResponseSchema = TaskResponseSchema(only=["properties"])
+ detail: dict[str, Any] = schema.dump(task)
+ listing: list[dict[str, Any]] = schema.dump([task], many=True)
+ expected: str = (
+ str(GENERIC_ERROR_MESSAGE)
+ if guest and task_type == "superset.query_object_v1"
+ else "boom"
+ )
+ assert detail["properties"]["error_message"] == expected
+ assert listing[0]["properties"]["error_message"] == expected
+ assert task.properties_dict["error_message"] == "boom"
+
+
+@pytest.mark.parametrize("guest", [False, True])
+@pytest.mark.parametrize(
+ "app", [{"FEATURE_FLAGS": {"GLOBAL_TASK_FRAMEWORK": True}}], indirect=True
+)
+def test_task_detail_chart_validation_message(
+ client: FlaskClient,
+ full_api_access: None,
+ mocker: MockerFixture,
+ guest: bool,
+) -> None:
+ """The detail endpoint uses the viewing principal for chart error redaction."""
+ # Avoid app-init regression: task models require the app fixture.
+ from superset.models.tasks import Task
+
+ task: Task = Task(
+ uuid=uuid4(),
+ task_type="superset.query_object_v1",
+ status="failure",
+ created_on=datetime(2026, 10, 20),
+ properties='{"error_message": "Error: Invalid filter on private_column"}',
+ )
+ mocker.patch("superset.daos.tasks.TaskDAO.find_one_or_none", return_value=task)
+ mocker.patch("superset.security_manager.is_guest_user", return_value=guest)
+ response: TestResponse = client.get(f"/api/v1/task/{task.uuid}")
+ assert response.status_code == 200
+ assert response.json["result"]["properties"]["error_message"] == (
+ str(GENERIC_ERROR_MESSAGE)
+ if guest
+ else "Error: Invalid filter on private_column"
+ )
def _task_with_private() -> SimpleNamespace: