test: mock ChartDataCommand.run for the ChartDataQueryFailedError no-reraise test (SC-118140)

Automated PR review (bito-code-review) correctly flagged that this test
mocked the schema loader's side effect instead of ChartDataCommand.run,
so it never actually reached the run() call the exception is meant to
simulate failing at. The except clause still catches the exception either
way (same try block), so the assertion was never wrong, but mocking at
the real trigger point matches the sibling ChartDataCacheLoadError test
and the corrected integration test, and is more representative of the
actual failure path.

Co-Authored-By: Claude <noreply@anthropic.com>

.codecov.yml

@@ -3,7 +3,9 @@ codecov:
         after_n_builds: 4
 ignore:
   - "superset/migrations/versions/*.py"
+  - "superset-frontend/packages/superset-ui-demo/**/*"
   - "**/*.stories.tsx"
+  - "**/*.stories.jsx"
 coverage:
   status:
     project:

.github/workflows/mirror-service-images.yml

@@ -93,7 +93,7 @@ jobs:
           password: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0
+        uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4.2.0
 
       - name: Copy image to GHCR
         env:

docs/package.json

@@ -66,7 +66,7 @@
     "caniuse-lite": "^1.0.30001809",
     "docusaurus-plugin-openapi-docs": "^5.2.0",
     "docusaurus-theme-openapi-docs": "^5.2.0",
-    "js-yaml": "^5.3.0",
+    "js-yaml": "^5.2.3",
     "json-bigint": "^1.0.0",
     "prism-react-renderer": "^2.4.1",
     "react": "^18.3.1",

docs/src/pages/community.tsx

@@ -67,7 +67,7 @@ const communityLinks = [
       'Join our monthly virtual meetups and register for any upcoming events on Meetup',
   },
   {
-    url: 'https://superset.apache.org/inTheWild/',
+    url: 'https://github.com/apache/superset/blob/master/RESOURCES/INTHEWILD.md',
     title: 'Organizations',
     description:
       'A list of some of the organizations using Superset in production.',

docs/yarn.lock

@@ -10291,10 +10291,10 @@ js-yaml@4.1.0, js-yaml@=4.3.1, js-yaml@^4.1.0, js-yaml@^4.1.1, js-yaml@^4.2.0, j
   dependencies:
     argparse "^2.0.1"
 
-js-yaml@^5.3.0:
-  version "5.3.0"
-  resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-5.3.0.tgz#526430a6da31065127528ae695ce168cfc5f91f0"
-  integrity sha512-muutsYr+e2+d3rTgUGslq5rxbBlUy3cJ61IsHag2QNDQV+7zXWjkUpmALIajhrlLlrgRUiymj6U3zUr/TMK84Q==
+js-yaml@^5.2.3:
+  version "5.2.3"
+  resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-5.2.3.tgz#0942ae8f507e22eb0e54624871789cd477106e54"
+  integrity sha512-n+mUVyUX5bVv7G/G2zyIHOhdxfuU1dY2NOFzTQUWiMUbFss8b57NFlgCCaggU78wSw5KVS9cllzeLyzyR+n5nw==
   dependencies:
     argparse "^2.0.1"
 

superset-frontend/package-lock.json

@@ -100,7 +100,7 @@
         "geostyler-style": "11.0.2",
         "geostyler-wfs-parser": "^3.0.1",
         "google-auth-library": "^11.0.2",
-        "immer": "^11.1.17",
+        "immer": "^11.1.16",
         "interweave": "^13.1.1",
         "jquery": "^4.0.0",
         "js-levenshtein": "^1.1.6",
@@ -23907,9 +23907,9 @@
       "license": "MIT"
     },
     "node_modules/immer": {
-      "version": "11.1.17",
-      "resolved": "https://registry.npmjs.org/immer/-/immer-11.1.17.tgz",
-      "integrity": "sha512-8Vu44Y0MuMBlTQz/jQ8HEMYNq/bBqk87MnBwYR5mC8AthfhEXidZ5aT/oA/CUqboa8THKltnD9L3xyqhU/Sy1Q==",
+      "version": "11.1.16",
+      "resolved": "https://registry.npmjs.org/immer/-/immer-11.1.16.tgz",
+      "integrity": "sha512-Xs7H9rBc+kti1J6RueUvbEBkmOz7jqj11XYgf+YMXAYzu8EeE7hwZ9poLXdVfVnGmJu7QAf41T7H2KuF6QoK6Q==",
       "license": "MIT",
       "funding": {
         "type": "opencollective",

superset-frontend/package.json

@@ -177,7 +177,7 @@
     "geostyler-style": "11.0.2",
     "geostyler-wfs-parser": "^3.0.1",
     "google-auth-library": "^11.0.2",
-    "immer": "^11.1.17",
+    "immer": "^11.1.16",
     "interweave": "^13.1.1",
     "jquery": "^4.0.0",
     "js-levenshtein": "^1.1.6",

superset-frontend/src/SqlLab/components/SaveDatasetActionButton/SaveDatasetActionButton.test.tsx

@@ -43,23 +43,6 @@ describe('SaveDatasetActionButton', () => {
     expect(saveDatasetBtn).toBeVisible();
   });
 
-  test('disables only the dataset button when canSaveDataset is false', () => {
-    const onSaveAsExplore = jest.fn();
-    render(
-      <SaveDatasetActionButton
-        setShowSave={() => true}
-        onSaveAsExplore={onSaveAsExplore}
-        canSaveDataset={false}
-      />,
-    );
-
-    // Saving the query needs no results.
-    expect(screen.getByRole('button', { name: 'Save' })).toBeEnabled();
-    expect(
-      screen.getByRole('button', { name: /save dataset/i }),
-    ).toBeDisabled();
-  });
-
   test('disables the save dataset button when the query did not run successfully', async () => {
     render(
       <SaveDatasetActionButton

superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx

@@ -19,14 +19,12 @@
 import { act, type ComponentProps } from 'react';
 import {
   cleanup,
-  createStore,
   fireEvent,
   render,
   screen,
   userEvent,
   waitFor,
 } from 'spec/helpers/testing-library';
-import reducerIndex from 'spec/helpers/reducerIndex';
 import fetchMock from 'fetch-mock';
 import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
 import { createDatasource } from 'src/SqlLab/actions/sqlLab';
@@ -65,12 +63,6 @@ beforeEach(() => {
   cleanup();
 });
 
-afterEach(() => {
-  // In-body restores are skipped when an assertion throws, leaking a
-  // configured spy into later tests.
-  jest.restoreAllMocks();
-});
-
 // Mock createDatasource to return a thunk that resolves with the dataset's
 // new id. The test's mock store includes redux-thunk middleware (from RTK's
 // getDefaultMiddleware), so dispatch(createDatasource(...)) properly unwraps
@@ -526,39 +518,6 @@ describe('SaveDatasetModal', () => {
     });
   });
 
-  test('surfaces the error and keeps the modal open when saving fails', async () => {
-    // The chart-payload step's toast was built but never dispatched, so a
-    // failure there was silent.
-    const postFormData = jest.spyOn(
-      require('src/explore/exploreUtils/formData'),
-      'postFormData',
-    );
-    postFormData.mockRejectedValue(new Error('Boom'));
-    const onHide = jest.fn();
-    const store = createStore({ user }, reducerIndex);
-
-    render(<SaveDatasetModal {...mockedProps} onHide={onHide} />, { store });
-
-    fireEvent.change(screen.getByDisplayValue(/unimportant/i), {
-      target: { value: 'my dataset' },
-    });
-    userEvent.click(screen.getByRole('button', { name: /save/i }));
-
-    // `createStore` builds its reducer map at runtime, so state isn't typed.
-    const toasts = () =>
-      (
-        store.getState() as unknown as {
-          messageToasts: { toastType: string }[];
-        }
-      ).messageToasts;
-
-    await waitFor(() => {
-      expect(toasts()).toHaveLength(1);
-    });
-    expect(toasts()[0].toastType).toBe('DANGER_TOAST');
-    expect(onHide).not.toHaveBeenCalled();
-  });
-
   test('clearDatasetCache is imported and available', () => {
     const { clearDatasetCache } = require('src/utils/cachedSupersetGet');
 

superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx

@@ -61,9 +61,6 @@ import type Subject from 'src/types/Subject';
 import { openInNewTab, redirect } from 'src/utils/navigationUtils';
 import { mapSubjectValuesToIds } from 'src/features/subjects/SubjectPicker';
 
-// Derived so it can't drift from what `getClientErrorObject` accepts.
-type SaveErrorSource = Parameters<typeof getClientErrorObject>[0];
-
 interface QueryDatabase {
   id?: number;
 }
@@ -394,18 +391,9 @@ export const SaveDatasetModal = ({
         setDatasetName(getDefaultDatasetName());
         onHide();
       })
-      .catch((error?: SaveErrorSource) => {
+      .catch(() => {
         setLoading(false);
-        // `createDatasource` already toasted the server's message and rejects
-        // with nothing; only the chart-payload step needs its own.
-        if (!error) {
-          return;
-        }
-        getClientErrorObject(error).then(e =>
-          dispatch(
-            addDangerToast(e.error || t('An error occurred saving dataset')),
-          ),
-        );
+        addDangerToast(t('An error occurred saving dataset'));
       });
   };
 

superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.tsx

@@ -27,8 +27,6 @@ import {
 import SaveQuery from 'src/SqlLab/components/SaveQuery';
 import { initialState, databases } from 'src/SqlLab/fixtures';
 
-const RESULT_COLUMNS = [{ column_name: 'col', type: 'STRING' }];
-
 const mockedProps = {
   queryEditorId: '123',
   animation: false,
@@ -37,6 +35,7 @@ const mockedProps = {
   onSave: () => {},
   saveQueryWarning: null,
   columns: [],
+  canSaveDataset: true,
 };
 
 const mockState = {
@@ -61,31 +60,8 @@ const splitSaveBtnProps = {
     ...mockedProps.database,
     allows_virtual_table_explore: true,
   },
-  columns: RESULT_COLUMNS,
 };
 
-const EDITOR_SQL = 'SELECT * FROM t';
-
-const stateWithLatestQuery = ({
-  id,
-  state,
-  sql = EDITOR_SQL,
-}: {
-  id: string;
-  state: string;
-  sql?: string;
-}) => ({
-  ...mockState,
-  sqlLab: {
-    ...mockState.sqlLab,
-    queryEditors: mockState.sqlLab.queryEditors.map(qe => ({
-      ...qe,
-      latestQueryId: id,
-    })),
-    queries: { [id]: { id, state, sql } },
-  },
-});
-
 const middlewares = [thunk];
 const mockStore = configureStore(middlewares);
 
@@ -121,71 +97,6 @@ describe('SavedQuery', () => {
     expect(saveBtn).toBeVisible();
   });
 
-  test('blocks "Save dataset" until the query has run successfully', () => {
-    // Without a successful run the save can only fail server-side.
-    render(<SaveQuery {...splitSaveBtnProps} />, {
-      useRedux: true,
-      store: mockStore(stateWithLatestQuery({ id: 'qid-1', state: 'failed' })),
-    });
-
-    expect(
-      screen.getByRole('button', { name: /save dataset/i }),
-    ).toBeDisabled();
-    // Saving the query itself is unaffected.
-    expect(screen.getByRole('button', { name: 'Save' })).toBeEnabled();
-  });
-
-  test('blocks "Save dataset" when no query has been run at all', () => {
-    render(<SaveQuery {...splitSaveBtnProps} />, {
-      useRedux: true,
-      store: mockStore(mockState),
-    });
-
-    expect(
-      screen.getByRole('button', { name: /save dataset/i }),
-    ).toBeDisabled();
-  });
-
-  test('blocks "Save dataset" when the SQL changed after a successful run', () => {
-    // The run succeeded, but not for what is in the editor now -- and it is
-    // the editor's SQL that gets saved.
-    render(<SaveQuery {...splitSaveBtnProps} />, {
-      useRedux: true,
-      store: mockStore(
-        stateWithLatestQuery({
-          id: 'qid-1',
-          state: 'success',
-          sql: 'SELECT 1 AS ran_earlier',
-        }),
-      ),
-    });
-
-    expect(
-      screen.getByRole('button', { name: /save dataset/i }),
-    ).toBeDisabled();
-  });
-
-  test('blocks "Save dataset" when the successful query returned no columns', () => {
-    // e.g. a DDL/DML statement -- there is nothing to introspect into a dataset.
-    render(<SaveQuery {...splitSaveBtnProps} columns={[]} />, {
-      useRedux: true,
-      store: mockStore(stateWithLatestQuery({ id: 'qid-1', state: 'success' })),
-    });
-
-    expect(
-      screen.getByRole('button', { name: /save dataset/i }),
-    ).toBeDisabled();
-  });
-
-  test('enables "Save dataset" once the query has succeeded', () => {
-    render(<SaveQuery {...splitSaveBtnProps} />, {
-      useRedux: true,
-      store: mockStore(stateWithLatestQuery({ id: 'qid-1', state: 'success' })),
-    });
-
-    expect(screen.getByRole('button', { name: /save dataset/i })).toBeEnabled();
-  });
-
   test('renders a save query modal when user clicks save button', () => {
     render(<SaveQuery {...mockedProps} />, {
       useRedux: true,
@@ -323,7 +234,7 @@ describe('SavedQuery', () => {
   test('renders a save dataset modal when user clicks "save dataset" menu item', async () => {
     render(<SaveQuery {...splitSaveBtnProps} />, {
       useRedux: true,
-      store: mockStore(stateWithLatestQuery({ id: 'qid-1', state: 'success' })),
+      store: mockStore(mockState),
     });
 
     const saveDatasetMenuItem = await screen.findByLabelText(/save dataset/i);
@@ -337,7 +248,7 @@ describe('SavedQuery', () => {
   test('renders the save dataset modal UI', async () => {
     render(<SaveQuery {...splitSaveBtnProps} />, {
       useRedux: true,
-      store: mockStore(stateWithLatestQuery({ id: 'qid-1', state: 'success' })),
+      store: mockStore(mockState),
     });
     const saveDatasetMenuItem = await screen.findByLabelText(/save dataset/i);
     userEvent.click(saveDatasetMenuItem);

superset-frontend/src/SqlLab/components/SaveQuery/index.tsx

@@ -17,8 +17,6 @@
  * under the License.
  */
 import { useState, useEffect, useMemo, ChangeEvent } from 'react';
-import { useSelector } from 'react-redux';
-import { Query, QueryState } from '@superset-ui/core';
 import type { DatabaseObject } from 'src/features/databases/types';
 import { t } from '@apache-superset/core/translation';
 import { styled } from '@apache-superset/core/theme';
@@ -39,7 +37,7 @@ import {
 } from 'src/SqlLab/components/SaveDatasetModal';
 import { getDatasourceAsSaveableDataset } from 'src/utils/datasourceUtils';
 import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
-import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types';
+import { QueryEditor } from 'src/SqlLab/types';
 import useLogAction from 'src/logger/useLogAction';
 import {
   LOG_ACTIONS_SQLLAB_CREATE_CHART,
@@ -54,6 +52,7 @@ interface SaveQueryProps {
   onUpdate: (arg0: QueryPayload, id: string) => void;
   saveQueryWarning: string | null;
   database: Partial<DatabaseObject> | undefined;
+  canSaveDataset: boolean;
 }
 
 export type QueryPayload = {
@@ -83,6 +82,7 @@ const SaveQuery = ({
   saveQueryWarning,
   database,
   columns,
+  canSaveDataset,
 }: SaveQueryProps) => {
   const queryEditor = useQueryEditor(queryEditorId, [
     'autorun',
@@ -113,17 +113,6 @@ const SaveQuery = ({
   const [label, setLabel] = useState<string>(defaultLabel);
   const [showSave, setShowSave] = useState<boolean>(false);
   const [showSaveDatasetModal, setShowSaveDatasetModal] = useState(false);
-  // Saving a dataset runs the SQL to introspect columns, so it needs a
-  // successful run of the SQL being saved that produced at least one column
-  // -- editing after a run invalidates it, and running a selection only
-  // validates that selection.
-  const latestQuery = useSelector<SqlLabRootState, Query | undefined>(
-    ({ sqlLab }) => sqlLab.queries[queryEditor.latestQueryId || ''],
-  );
-  const canSaveDataset =
-    latestQuery?.state === QueryState.Success &&
-    latestQuery.sql === queryEditor.sql &&
-    columns.length > 0;
   const isSaved = !!query.remoteId;
   const isLabelEmpty = label.trim().length === 0;
   const canExploreDatabase = !!database?.allows_virtual_table_explore;

superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx

@@ -356,10 +356,7 @@ describe('SqlEditor', () => {
     );
 
   test('enables the save dataset button when the latest query succeeded', async () => {
-    const { findByRole } = setupWithLatestQuery({
-      state: QueryState.Success,
-      sql: mockedProps.queryEditor.sql,
-    });
+    const { findByRole } = setupWithLatestQuery({ state: QueryState.Success });
     expect(await findByRole('button', { name: 'Save dataset' })).toBeEnabled();
   });
 

superset-frontend/src/SqlLab/components/SqlEditor/index.tsx

@@ -868,6 +868,7 @@ const SqlEditor: FC<Props> = ({
           }
           saveQueryWarning={saveQueryWarning}
           database={database}
+          canSaveDataset={successful && resultColumns.length > 0}
         />
         <ShareSqlLabQuery queryEditorId={queryEditor.id} />
       </>

superset/commands/dataset/create.py

@@ -33,12 +33,7 @@ from superset.commands.dataset.exceptions import (
 )
 from superset.commands.utils import populate_subjects
 from superset.daos.dataset import DatasetDAO
-from superset.exceptions import (
-    OAuth2RedirectError,
-    SupersetException,
-    SupersetParseError,
-    SupersetSecurityException,
-)
+from superset.exceptions import SupersetParseError, SupersetSecurityException
 from superset.extensions import security_manager
 from superset.sql.parse import Table
 from superset.utils.decorators import on_error, transaction
@@ -55,28 +50,7 @@ class CreateDatasetCommand(CreateMixin, BaseCommand):
         self.validate()
 
         dataset = DatasetDAO.create(attributes=self._properties)
-        try:
-            dataset.fetch_metadata()
-        except OAuth2RedirectError:
-            # Must reach the caller unchanged to start the OAuth2 dance.
-            raise
-        except SupersetException as ex:
-            # Not a SQLAlchemyError, so ``on_error`` re-raises it untouched and
-            # it escapes to FAB's ``@safe`` as an opaque 500 "Fatal error".
-            # Deliberately covers the 403 ``SupersetSecurityException`` raised
-            # for mutation/multi-statement SQL too: ``validate()`` already
-            # reports that class of rejection as a 422 on ``sql`` via
-            # ``DatasetDataAccessIsNotAllowed``.
-            raise DatasetInvalidError(
-                exceptions=[
-                    ValidationError(
-                        # ``lazy_gettext`` messages aren't ``str``, so
-                        # marshmallow won't wrap them into a list on its own.
-                        [str(ex.message)],
-                        field_name="sql" if self._properties.get("sql") else "table",
-                    )
-                ]
-            ) from ex
+        dataset.fetch_metadata()
         return dataset
 
     def validate(self) -> None:  # noqa: C901

superset/connectors/sqla/utils.py

@@ -125,14 +125,12 @@ def get_virtual_table_metadata(dataset: SqlaTable) -> list[ResultSetColumnType]:
         # rest (sandbox violations, malformed template syntax, encoding
         # errors) indicate a real problem with the template that must
         # surface. See #38012.
-        # str(ex) stringifies the raw SupersetError list (enum reprs and all).
-        error_message = "; ".join(err.message for err in ex.errors)
         if isinstance(ex.__cause__, UndefinedError):
             raise SupersetVirtualTableParseException(
-                message=_("Template processing error: %(error)s", error=error_message),
+                message=_("Template processing error: %(error)s", error=str(ex)),
             ) from ex
         raise SupersetGenericDBErrorException(
-            message=_("Template processing error: %(error)s", error=error_message),
+            message=_("Template processing error: %(error)s", error=str(ex)),
         ) from ex
     try:
         parsed_script = SQLScript(sql, engine=db_engine_spec.engine)

superset/mcp_service/dataset/tool/create_virtual_dataset.py

@@ -218,8 +218,6 @@ async def create_virtual_dataset(  # noqa: C901
             error=f"Failed to update dataset metadata (creation rolled back): {exc}",
         )
     except SupersetGenericDBErrorException as exc:
-        # Defensive backstop for direct raises (see
-        # test_create_virtual_dataset_sql_error_is_actionable).
         logger.warning("Virtual dataset SQL validation failed", exc_info=True)
         await ctx.warning(f"Virtual dataset SQL failed validation: {exc}")
         return CreateVirtualDatasetResponse(

superset/tasks/async_queries.py

@@ -106,6 +106,10 @@ def load_chart_data_into_cache(
 ) -> None:
     # pylint: disable=import-outside-toplevel
     from superset.commands.chart.data.get_data_command import ChartDataCommand
+    from superset.commands.chart.exceptions import (
+        ChartDataCacheLoadError,
+        ChartDataQueryFailedError,
+    )
 
     with override_user(_load_user_from_job_metadata(job_metadata), force=False):
         try:
@@ -123,6 +127,20 @@ def load_chart_data_into_cache(
         except SoftTimeLimitExceeded as ex:
             _handle_soft_time_limit(job_metadata, ex, "loading chart data")
             raise
+        except (ChartDataCacheLoadError, ChartDataQueryFailedError) as ex:
+            # These map to 422/400 in the synchronous chart/data endpoint (see
+            # ChartDataRestApi._get_data_response) - expected, client-facing
+            # validation failures (e.g. a chart still referencing columns a
+            # customer has since dropped from the dataset), not application
+            # bugs. The failure is already delivered to the client via
+            # update_job below; re-raising would only surface it a second
+            # time as an unhandled Celery task exception.
+            logger.info("Chart data query failed while loading into cache: %s", ex)
+            async_query_manager.update_job(
+                job_metadata,
+                async_query_manager.STATUS_ERROR,
+                errors=sanitize_error_dicts([{"message": str(ex.message)}]),
+            )
         except Exception as ex:
             # Extract SIP-40 style errors when available
             if isinstance(ex, SupersetErrorException):

tests/integration_tests/tasks/async_queries_tests.py

@@ -110,8 +110,11 @@ class TestAsyncQueries(SupersetTestCase):
             "status": "pending",
             "errors": [],
         }
-        with pytest.raises(ChartDataQueryFailedError):
-            load_chart_data_into_cache(job_metadata, query_context)
+        # ChartDataQueryFailedError mirrors the synchronous chart/data endpoint's
+        # 400 (see ChartDataRestApi._get_data_response) - an expected validation
+        # failure, not a bug, so the task reports it via update_job and does not
+        # re-raise (see superset/tasks/async_queries.py).
+        load_chart_data_into_cache(job_metadata, query_context)
 
         mock_run_command.assert_called_once_with(cache=True)
         errors = [{"message": "Error: foo"}]

tests/unit_tests/commands/dataset/test_create.py

@@ -18,16 +18,11 @@ from unittest.mock import Mock, patch
 
 import pytest
 from marshmallow import ValidationError
-from pytest_mock import MockerFixture
 
 from superset.commands.dataset.create import CreateDatasetCommand
 from superset.commands.dataset.exceptions import DatasetInvalidError
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-from superset.exceptions import (
-    OAuth2RedirectError,
-    SupersetGenericDBErrorException,
-    SupersetParseError,
-)
+from superset.exceptions import SupersetParseError
 from superset.models.core import Database
 
 
@@ -255,112 +250,3 @@ def test_create_dataset_generic_exists_error_when_no_twin() -> None:
                             )
                             with pytest.raises(DatasetInvalidError):
                                 command.validate()
-
-
-def test_create_dataset_metadata_fetch_error_is_structured(
-    mocker: MockerFixture,
-) -> None:
-    """A metadata-fetch failure must surface the engine's own message.
-
-    ``run()`` executes the SQL to introspect columns; the resulting
-    ``SupersetGenericDBErrorException`` used to escape as a 500 "Fatal error".
-    """
-    mocker.patch.object(CreateDatasetCommand, "validate")
-    dataset = Mock()
-    dataset.fetch_metadata.side_effect = SupersetGenericDBErrorException(
-        message="Invalid SQL: Unable to parse: SELECT ...",
-    )
-    mocker.patch(
-        "superset.commands.dataset.create.DatasetDAO.create",
-        return_value=dataset,
-    )
-
-    command = CreateDatasetCommand(
-        {
-            "database": 1,
-            "table_name": "dataset wrong",
-            "sql": "SELECT ...",
-        }
-    )
-
-    with pytest.raises(DatasetInvalidError) as exc_info:
-        command.run()
-
-    validation_errors = exc_info.value._exceptions
-    assert len(validation_errors) == 1
-    assert validation_errors[0].field_name == "sql"
-    assert "Invalid SQL: Unable to parse: SELECT ..." in str(
-        validation_errors[0].messages[0]
-    )
-
-
-def test_create_dataset_metadata_fetch_error_physical_table(
-    mocker: MockerFixture,
-) -> None:
-    """The same conversion applies to physical datasets, keyed on ``table``."""
-    mocker.patch.object(CreateDatasetCommand, "validate")
-    dataset = Mock()
-    dataset.fetch_metadata.side_effect = SupersetGenericDBErrorException(
-        message="(psycopg2.OperationalError) could not connect to server",
-    )
-    mocker.patch(
-        "superset.commands.dataset.create.DatasetDAO.create",
-        return_value=dataset,
-    )
-
-    command = CreateDatasetCommand({"database": 1, "table_name": "physical_table"})
-
-    with pytest.raises(DatasetInvalidError) as exc_info:
-        command.run()
-
-    validation_errors = exc_info.value._exceptions
-    assert validation_errors[0].field_name == "table"
-    assert "could not connect to server" in str(validation_errors[0].messages[0])
-
-
-def test_create_dataset_oauth2_redirect_propagates_unchanged(
-    mocker: MockerFixture,
-) -> None:
-    """OAuth2 redirects must not be flattened into a DatasetInvalidError."""
-    mocker.patch.object(CreateDatasetCommand, "validate")
-    dataset = Mock()
-    oauth2_error = OAuth2RedirectError(
-        url="https://example.org/oauth2/authorize",
-        tab_id="tab-123",
-        redirect_uri="https://superset.example.org/oauth2/redirect",
-    )
-    dataset.fetch_metadata.side_effect = oauth2_error
-    mocker.patch(
-        "superset.commands.dataset.create.DatasetDAO.create",
-        return_value=dataset,
-    )
-
-    command = CreateDatasetCommand(
-        {"database": 1, "table_name": "good_dataset", "sql": "SELECT 1 AS a"}
-    )
-
-    with pytest.raises(OAuth2RedirectError) as exc_info:
-        command.run()
-
-    assert exc_info.value is oauth2_error
-    assert exc_info.value.error.extra["url"] == "https://example.org/oauth2/authorize"
-    assert exc_info.value.error.extra["tab_id"] == "tab-123"
-
-
-def test_create_dataset_run_succeeds_when_metadata_fetch_works(
-    mocker: MockerFixture,
-) -> None:
-    """Control: the happy path still returns the created dataset."""
-    mocker.patch.object(CreateDatasetCommand, "validate")
-    dataset = Mock()
-    mocker.patch(
-        "superset.commands.dataset.create.DatasetDAO.create",
-        return_value=dataset,
-    )
-
-    command = CreateDatasetCommand(
-        {"database": 1, "table_name": "good_dataset", "sql": "SELECT 1 AS a"}
-    )
-
-    assert command.run() is dataset
-    dataset.fetch_metadata.assert_called_once()

tests/unit_tests/connectors/sqla/test_utils.py

@@ -191,39 +191,6 @@ def test_get_virtual_table_metadata_template_security_error_is_not_softened():
     assert "Template processing error" in str(exc_info.value.message)
 
 
-def test_get_virtual_table_metadata_template_error_message_is_clean():
-    """The message must be the SupersetError's own text, not str(ex)."""
-    mock_dataset = Mock(spec=SqlaTable)
-    mock_database = Mock(spec=Database)
-    mock_dataset.database = mock_database
-    mock_dataset.sql = "SELECT 1 {% if %}"
-
-    ex = SupersetSyntaxErrorException(
-        [
-            SupersetError(
-                message="Malformed template, expected 'endif'",
-                error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
-                level=ErrorLevel.ERROR,
-            )
-        ]
-    )
-    ex.__cause__ = SecurityError("unrelated cause, not UndefinedError")
-    mock_template_processor = Mock()
-    mock_template_processor.process_template.side_effect = ex
-    mock_dataset.get_template_processor.return_value = mock_template_processor
-    mock_dataset.template_params_dict = {}
-
-    with pytest.raises(SupersetGenericDBErrorException) as exc_info:
-        get_virtual_table_metadata(mock_dataset)
-
-    message = str(exc_info.value.message)
-    assert message == (
-        "Template processing error: Malformed template, expected 'endif'"
-    )
-    assert "SupersetError(" not in message
-    assert "error_type=<" not in message
-
-
 def test_get_virtual_table_metadata_multiple_statements_not_allowed():
     """Test that multiple SQL statements raise security error."""
     mock_dataset = Mock(spec=SqlaTable)

tests/unit_tests/datasets/api_tests.py

@@ -214,46 +214,3 @@ def test_handle_filters_args_returns_request_scoped_filters(
     fresh_filters = api.datamodel.get_filters.return_value
     assert fresh_filters.rest_add_filters.call_count == 2
     assert fresh_filters.get_joined_filters.call_count == 2
-
-
-def test_post_dataset_with_invalid_sql_returns_actionable_422(
-    session: Session,
-    client: Any,
-    full_api_access: None,
-) -> None:
-    """Saving a dataset over unrunnable SQL must explain what is wrong.
-
-    With blanket database access ``validate()`` never parses the SQL, so
-    ``run()``'s column introspection is the first thing to reject it. That
-    used to surface as a bare 500 ``{"message": "Fatal error"}``.
-    """
-    from superset.connectors.sqla.models import SqlaTable
-    from superset.models.core import Database
-
-    SqlaTable.metadata.create_all(db.session.get_bind())
-
-    database = Database(database_name="invalid_sql_db", sqlalchemy_uri="sqlite://")
-    db.session.add(database)
-    db.session.flush()
-
-    response = client.post(
-        "/api/v1/dataset/",
-        json={
-            "database": database.id,
-            "schema": "main",
-            "table_name": "dataset wrong",
-            "sql": "SELECT ...",
-        },
-    )
-
-    assert response.status_code == 422
-    message = response.json["message"]
-    assert "Fatal error" not in str(message)
-    # Not the parser's exact wording -- that would break on a sqlglot bump.
-    assert message["sql"][0].startswith("Invalid SQL")
-
-    # The failed create must not leave a half-built dataset behind.
-    assert (
-        db.session.query(SqlaTable).filter_by(table_name="dataset wrong").one_or_none()
-        is None
-    )

tests/unit_tests/tasks/test_async_queries.py

@@ -21,7 +21,10 @@ import pytest
 from celery.exceptions import SoftTimeLimitExceeded
 from flask_babel import lazy_gettext as _
 
-from superset.commands.chart.exceptions import ChartDataQueryFailedError
+from superset.commands.chart.exceptions import (
+    ChartDataCacheLoadError,
+    ChartDataQueryFailedError,
+)
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import (
     OAuth2RedirectError,
@@ -43,7 +46,7 @@ def test_load_chart_data_into_cache_with_error(
     job_metadata = {"user_id": 1}
     form_data = {}
     err_message = "Something went wrong"
-    err = ChartDataQueryFailedError(_(err_message))
+    err = RuntimeError(err_message)
 
     mock_user = mock.MagicMock()
     mock_query_context_schema = mock.MagicMock()
@@ -54,7 +57,7 @@ def test_load_chart_data_into_cache_with_error(
 
     mock_query_context_schema.load.side_effect = err
 
-    with pytest.raises(ChartDataQueryFailedError):
+    with pytest.raises(RuntimeError):
         load_chart_data_into_cache(job_metadata, form_data)
 
     expected_errors = [{"message": err_message}]
@@ -64,6 +67,76 @@ def test_load_chart_data_into_cache_with_error(
     )
 
 
+@mock.patch("superset.tasks.async_queries.security_manager")
+@mock.patch("superset.tasks.async_queries.async_query_manager")
+@mock.patch("superset.commands.chart.data.get_data_command.ChartDataCommand")
+@mock.patch("superset.tasks.async_queries.ChartDataQueryContextSchema")
+def test_load_chart_data_into_cache_with_query_failed_error_does_not_reraise(
+    mock_query_context_schema_cls: mock.MagicMock,
+    mock_command_cls: mock.MagicMock,
+    mock_async_query_manager: mock.MagicMock,
+    mock_security_manager: mock.MagicMock,
+) -> None:
+    """
+    ChartDataQueryFailedError maps to a 400 in the synchronous chart/data
+    endpoint (see ChartDataRestApi._get_data_response) - an expected,
+    client-facing validation failure (e.g. a chart still referencing columns
+    a customer has since dropped from the dataset), not an application bug.
+    The task must still report it to the client via update_job, but must not
+    re-raise it - that would surface it a second time as an unhandled Celery
+    task exception.
+    """
+    from superset.tasks.async_queries import load_chart_data_into_cache
+
+    job_metadata = {"user_id": 1}
+    form_data: dict[str, Any] = {}
+    err_message = "Columns missing in dataset: ['foo']"
+
+    mock_security_manager.get_user_by_id.return_value = mock.MagicMock()
+    mock_async_query_manager.STATUS_ERROR = "error"
+    mock_query_context_schema_cls.return_value.load.return_value = mock.MagicMock()
+    mock_command_cls.return_value.run.side_effect = ChartDataQueryFailedError(
+        _(err_message)
+    )
+
+    # Should not raise.
+    load_chart_data_into_cache(job_metadata, form_data)
+
+    mock_async_query_manager.update_job.assert_called_once_with(
+        job_metadata, "error", errors=[{"message": err_message}]
+    )
+
+
+@mock.patch("superset.tasks.async_queries.security_manager")
+@mock.patch("superset.tasks.async_queries.async_query_manager")
+@mock.patch("superset.commands.chart.data.get_data_command.ChartDataCommand")
+@mock.patch("superset.tasks.async_queries.ChartDataQueryContextSchema")
+def test_load_chart_data_into_cache_with_cache_load_error_does_not_reraise(
+    mock_query_context_schema_cls: mock.MagicMock,
+    mock_command_cls: mock.MagicMock,
+    mock_async_query_manager: mock.MagicMock,
+    mock_security_manager: mock.MagicMock,
+) -> None:
+    """Same as above, for the sibling 422-mapped ChartDataCacheLoadError."""
+    from superset.tasks.async_queries import load_chart_data_into_cache
+
+    job_metadata = {"user_id": 1}
+    form_data: dict[str, Any] = {}
+    err_message = "Cache load failed"
+
+    mock_security_manager.get_user_by_id.return_value = mock.MagicMock()
+    mock_async_query_manager.STATUS_ERROR = "error"
+    mock_query_context_schema_cls.return_value.load.return_value = mock.MagicMock()
+    mock_command_cls.return_value.run.side_effect = ChartDataCacheLoadError(err_message)
+
+    # Should not raise.
+    load_chart_data_into_cache(job_metadata, form_data)
+
+    mock_async_query_manager.update_job.assert_called_once_with(
+        job_metadata, "error", errors=[{"message": err_message}]
+    )
+
+
 @mock.patch("superset.tasks.async_queries.security_manager")
 @mock.patch("superset.tasks.async_queries.async_query_manager")
 @mock.patch("superset.tasks.async_queries.ChartDataQueryContextSchema")