You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by as...@apache.org on 2026/04/02 12:52:17 UTC
(superset) branch alexandrusoare/fix/abort-control-for-chart created (now c6676287caa)
This is an automated email from the ASF dual-hosted git repository.
asoare pushed a change to branch alexandrusoare/fix/abort-control-for-chart
in repository https://gitbox.apache.org/repos/asf/superset.git
at c6676287caa fix(chart): abort network calls when unmounting
This branch includes the following new commits:
new c6676287caa fix(chart): abort network calls when unmounting
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(chart): abort network calls when unmounting
Posted by as...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asoare pushed a commit to branch alexandrusoare/fix/abort-control-for-chart
in repository https://gitbox.apache.org/repos/asf/superset.git
commit c6676287caa6b982a5597b167ad811e69dd3e44d
Author: alexandrusoare <al...@gmail.com>
AuthorDate: Thu Apr 2 15:51:57 2026 +0300
fix(chart): abort network calls when unmounting
---
.../components/Chart/DrillBy/DrillByModal.test.tsx | 74 ++++++++++++++-
.../src/components/Chart/DrillBy/DrillByModal.tsx | 13 ++-
superset-frontend/src/pages/Chart/Chart.test.tsx | 100 +++++++++++++++++++++
superset-frontend/src/pages/Chart/index.tsx | 30 ++++++-
4 files changed, 212 insertions(+), 5 deletions(-)
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
index d49697af7e8..973e6850d95 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
@@ -37,6 +37,18 @@ jest.mock('src/dashboard/util/isEmbedded', () => ({
isEmbedded: jest.fn(() => false),
}));
+const mockAddDangerToast = jest.fn();
+jest.mock('src/components/MessageToasts/withToasts', () => ({
+ __esModule: true,
+ default: (Component: React.ComponentType) => Component,
+ useToasts: () => ({
+ addDangerToast: mockAddDangerToast,
+ addInfoToast: jest.fn(),
+ addSuccessToast: jest.fn(),
+ addWarningToast: jest.fn(),
+ }),
+}));
+
const CHART_DATA_ENDPOINT = 'glob:*/api/v1/chart/data*';
const FORM_DATA_KEY_ENDPOINT = 'glob:*/api/v1/explore/form_data';
@@ -131,7 +143,10 @@ beforeEach(() => {
);
});
-afterEach(() => fetchMock.removeRoutes().clearHistory());
+afterEach(() => {
+ fetchMock.removeRoutes().clearHistory();
+ mockAddDangerToast.mockClear();
+});
test('should render the title', async () => {
await renderModal();
@@ -576,3 +591,60 @@ describe('Table view with pagination', () => {
});
});
});
+
+test('does not show error toast when chart data request is aborted on modal close', async () => {
+ let rejectRequest: (error: Error) => void;
+ const pendingPromise = new Promise((_, reject) => {
+ rejectRequest = reject;
+ });
+
+ fetchMock.removeRoute(CHART_DATA_ENDPOINT);
+ fetchMock.post(CHART_DATA_ENDPOINT, () => pendingPromise, {
+ name: CHART_DATA_ENDPOINT,
+ });
+
+ const DrillByModalWrapper = () => {
+ const [showModal, setShowModal] = useState(false);
+
+ return (
+ <DashboardPageIdContext.Provider value="1">
+ <button type="button" onClick={() => setShowModal(true)}>
+ Show modal
+ </button>
+ {showModal && (
+ <DrillByModal
+ formData={formData}
+ onHideModal={() => setShowModal(false)}
+ dataset={dataset}
+ drillByConfig={{ groupbyFieldName: 'groupby', filters: [] }}
+ canDownload
+ />
+ )}
+ </DashboardPageIdContext.Provider>
+ );
+ };
+
+ render(<DrillByModalWrapper />, {
+ useDnd: true,
+ useRedux: true,
+ useRouter: true,
+ initialState: drillByModalState,
+ });
+
+ // Open the modal
+ userEvent.click(screen.getByRole('button', { name: 'Show modal' }));
+ await screen.findByRole('dialog', { name: `Drill by: ${chartName}` });
+
+ // Close the modal before the request completes
+ userEvent.click(screen.getAllByRole('button', { name: 'Close' })[1]);
+
+ // Simulate the aborted request rejection
+ const abortError = new Error('The operation was aborted.');
+ abortError.name = 'AbortError';
+ rejectRequest!(abortError);
+
+ // Wait a bit to ensure any async error handling would have occurred
+ await waitFor(() => {
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+ });
+});
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
index 1bb63d0d81e..f7491c93aa3 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
@@ -440,11 +440,13 @@ export default function DrillByModal({
useEffect(() => {
if (drilledFormData) {
+ const controller = new AbortController();
const [useLegacyApi] = getQuerySettings(drilledFormData);
setIsChartDataLoading(true);
setChartDataResult(undefined);
getChartDataRequest({
formData: drilledFormData,
+ requestParams: { signal: controller.signal },
})
.then(({ response, json }) =>
handleChartDataResponse(response, json, useLegacyApi),
@@ -452,13 +454,22 @@ export default function DrillByModal({
.then(queriesResponse => {
setChartDataResult(queriesResponse);
})
- .catch(() => {
+ .catch(err => {
+ // Silently ignore aborted requests
+ if (err.name === 'AbortError') {
+ return;
+ }
addDangerToast(t('Failed to load chart data.'));
})
.finally(() => {
setIsChartDataLoading(false);
});
+
+ return () => {
+ controller.abort();
+ };
}
+ return undefined;
}, [addDangerToast, drilledFormData]);
const { metadataBar } = useDatasetMetadataBar({ dataset });
diff --git a/superset-frontend/src/pages/Chart/Chart.test.tsx b/superset-frontend/src/pages/Chart/Chart.test.tsx
index 58106375f0e..6f6543b7f05 100644
--- a/superset-frontend/src/pages/Chart/Chart.test.tsx
+++ b/superset-frontend/src/pages/Chart/Chart.test.tsx
@@ -54,6 +54,15 @@ jest.mock('src/explore/exploreUtils/getParsedExploreURLParams', () => ({
getParsedExploreURLParams: jest.fn(),
}));
+const mockAddDangerToast = jest.fn();
+jest.mock('src/components/MessageToasts/actions', () => ({
+ ...jest.requireActual('src/components/MessageToasts/actions'),
+ addDangerToast: (...args: unknown[]) => {
+ mockAddDangerToast(...args);
+ return { type: 'ADD_TOAST' };
+ },
+}));
+
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('ChartPage', () => {
beforeEach(() => {
@@ -69,6 +78,7 @@ describe('ChartPage', () => {
afterEach(() => {
fetchMock.clearHistory().removeRoutes();
+ mockAddDangerToast.mockClear();
});
test('fetches metadata on mount', async () => {
@@ -358,4 +368,94 @@ describe('ChartPage', () => {
);
});
});
+
+ test('does not show error toast when request is aborted on unmount', async () => {
+ const exploreApiRoute = 'glob:*/api/v1/explore/*';
+ let rejectRequest: (error: Error) => void;
+ const pendingPromise = new Promise((_, reject) => {
+ rejectRequest = reject;
+ });
+
+ fetchMock.get(exploreApiRoute, () => pendingPromise);
+
+ const { unmount } = render(<ChartPage />, {
+ useRouter: true,
+ useRedux: true,
+ useDnd: true,
+ });
+
+ // Unmount before the request completes
+ unmount();
+
+ // Simulate the aborted request rejection
+ const abortError = new Error('The operation was aborted.');
+ abortError.name = 'AbortError';
+ rejectRequest!(abortError);
+
+ // Wait a bit to ensure any async error handling would have occurred
+ await waitFor(() => {
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+ });
+ });
+
+ test('aborts in-flight request when a new request is made', async () => {
+ const exploreApiRoute = 'glob:*/api/v1/explore/*';
+ const exploreFormData = getExploreFormData({
+ viz_type: VizType.Table,
+ show_cell_bars: true,
+ });
+
+ // First request will be slow
+ let firstRequestResolve: () => void;
+ const firstRequestPromise = new Promise<void>(resolve => {
+ firstRequestResolve = resolve;
+ });
+
+ fetchMock.get(
+ exploreApiRoute,
+ () =>
+ firstRequestPromise.then(() => ({
+ result: { dataset: { id: 1 }, form_data: exploreFormData },
+ })),
+ { overwriteRoutes: true },
+ );
+
+ render(
+ <>
+ <Link to="/?slice_id=99">Navigate</Link>
+ <ChartPage />
+ </>,
+ {
+ useRouter: true,
+ useRedux: true,
+ useDnd: true,
+ },
+ );
+
+ // Wait for the first request to be initiated
+ await waitFor(() =>
+ expect(fetchMock.callHistory.calls(exploreApiRoute).length).toBe(1),
+ );
+
+ // Set up second request to return immediately
+ fetchMock.get(
+ exploreApiRoute,
+ { result: { dataset: { id: 1 }, form_data: exploreFormData } },
+ { overwriteRoutes: true },
+ );
+
+ // Navigate to trigger a new request (which should abort the first)
+ fireEvent.click(screen.getByText('Navigate'));
+
+ // Complete the first request after it should have been aborted
+ firstRequestResolve!();
+
+ // Wait for the second request to complete
+ await waitFor(() =>
+ expect(fetchMock.callHistory.calls(exploreApiRoute).length).toBe(2),
+ );
+
+ // No error toast should be shown from the aborted first request
+ expect(mockAddDangerToast).not.toHaveBeenCalled();
+ });
});
diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx
index 46acccad36a..c452cdd1270 100644
--- a/superset-frontend/src/pages/Chart/index.tsx
+++ b/superset-frontend/src/pages/Chart/index.tsx
@@ -50,10 +50,14 @@ const isValidResult = (rv: JsonObject): boolean =>
const hasDatasetId = (rv: JsonObject): boolean =>
isDefined(rv?.result?.dataset?.id);
-const fetchExploreData = async (exploreUrlParams: URLSearchParams) => {
+const fetchExploreData = async (
+ exploreUrlParams: URLSearchParams,
+ signal?: AbortSignal,
+) => {
const rv = await makeApi<{}, ExploreResponsePayload>({
method: 'GET',
endpoint: 'api/v1/explore/',
+ signal,
})(exploreUrlParams);
if (isValidResult(rv)) {
if (hasDatasetId(rv)) {
@@ -130,6 +134,7 @@ const getDashboardContextFormData = (search: string) => {
export default function ExplorePage() {
const [isLoaded, setIsLoaded] = useState(false);
const fetchGeneration = useRef(0);
+ const abortControllerRef = useRef<AbortController | null>(null);
const dispatch = useDispatch();
const history = useHistory();
@@ -138,6 +143,11 @@ export default function ExplorePage() {
loc: { search: string; pathname: string },
saveAction?: SaveActionType | null,
) => {
+ // Abort any in-flight request before starting a new one
+ abortControllerRef.current?.abort();
+ const controller = new AbortController();
+ abortControllerRef.current = controller;
+
fetchGeneration.current += 1;
const generation = fetchGeneration.current;
const exploreUrlParams = getParsedExploreURLParams(loc);
@@ -145,7 +155,7 @@ export default function ExplorePage() {
const isStale = () => generation !== fetchGeneration.current;
- fetchExploreData(exploreUrlParams)
+ fetchExploreData(exploreUrlParams, controller.signal)
.then(({ result }) => {
if (isStale()) {
return;
@@ -183,7 +193,13 @@ export default function ExplorePage() {
}),
);
})
- .catch(err => Promise.all([getClientErrorObject(err), err]))
+ .catch(err => {
+ // Silently ignore aborted requests
+ if (err.name === 'AbortError') {
+ return undefined;
+ }
+ return Promise.all([getClientErrorObject(err), err]);
+ })
.then(resolved => {
if (isStale()) {
return;
@@ -259,6 +275,14 @@ export default function ExplorePage() {
[dispatch],
);
+ // Cleanup: abort in-flight requests on unmount
+ useEffect(
+ () => () => {
+ abortControllerRef.current?.abort();
+ },
+ [],
+ );
+
// Initial fetch on mount
useEffect(() => {
loadExploreData(history.location);