From 7be8cb6775222832ed2052512947d896d90f0613 Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Wed, 5 Nov 2025 14:02:32 +0300 Subject: [PATCH 1/8] fix: fix crossfilter persisting after removal --- superset-frontend/src/components/Chart/chartReducer.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts index 081bd4154724..d8e9d1a063db 100644 --- a/superset-frontend/src/components/Chart/chartReducer.ts +++ b/superset-frontend/src/components/Chart/chartReducer.ts @@ -67,6 +67,9 @@ export default function chartReducer( }; }, [actions.CHART_UPDATE_STARTED](state) { + if (state.queryController) { + state.queryController.abort(); + } return { ...state, chartStatus: 'loading', From 9dd7e4b11b350cd3ac014e44e10c08a0a9c8dcd2 Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Wed, 5 Nov 2025 23:46:53 +0300 Subject: [PATCH 2/8] fix: fix crossfilter persisting after removal --- superset-frontend/src/components/Chart/chartReducer.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts index d8e9d1a063db..8620b8e28d72 100644 --- a/superset-frontend/src/components/Chart/chartReducer.ts +++ b/superset-frontend/src/components/Chart/chartReducer.ts @@ -67,8 +67,9 @@ export default function chartReducer( }; }, [actions.CHART_UPDATE_STARTED](state) { - if (state.queryController) { - state.queryController.abort(); + const controller = state.queryController; + if (controller) { + controller.abort(); } return { ...state, From 0d33e947c196a64132985ec8ee9e4ae6b2bbd3a7 Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Thu, 6 Nov 2025 11:15:50 +0300 Subject: [PATCH 3/8] fix: fix crossfilter persisting after removal --- superset-frontend/src/components/Chart/chartReducer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts index 8620b8e28d72..e43e954e6658 100644 --- a/superset-frontend/src/components/Chart/chartReducer.ts +++ b/superset-frontend/src/components/Chart/chartReducer.ts @@ -69,7 +69,7 @@ export default function chartReducer( [actions.CHART_UPDATE_STARTED](state) { const controller = state.queryController; if (controller) { - controller.abort(); + setTimeout(() => controller.abort(), 0); } return { ...state, From 7b08174d08a7e39e0f39ff035f3ea3fb2358f442 Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Thu, 6 Nov 2025 11:48:57 +0300 Subject: [PATCH 4/8] add tests --- .../components/Chart/chartReducers.test.js | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/superset-frontend/src/components/Chart/chartReducers.test.js b/superset-frontend/src/components/Chart/chartReducers.test.js index a4081aa851bc..6b8f599775de 100644 --- a/superset-frontend/src/components/Chart/chartReducers.test.js +++ b/superset-frontend/src/components/Chart/chartReducers.test.js @@ -60,4 +60,35 @@ describe('chart reducers', () => { expect(newState[chartKey].chartUpdateEndTime).toBeGreaterThan(0); expect(newState[chartKey].chartStatus).toEqual('failed'); }); + + test('should defer abort of previous controller to avoid Redux state mutation', () => { + jest.useFakeTimers(); + + const oldController = new AbortController(); + const abortSpy = jest.spyOn(oldController, 'abort'); + + const chartWithController = { + ...testChart, + queryController: oldController, + chartStatus: 'loading', + }; + + const newController = new AbortController(); + const newState = chartReducer( + { [chartKey]: chartWithController }, + actions.chartUpdateStarted(newController, {}, chartKey), + ); + + expect(abortSpy).not.toHaveBeenCalled(); + expect(oldController.signal.aborted).toBe(false); + + jest.runAllTimers(); + expect(abortSpy).toHaveBeenCalledTimes(1); + expect(oldController.signal.aborted).toBe(true); + + expect(newState[chartKey].queryController).toBe(newController); + expect(newState[chartKey].chartStatus).toBe('loading'); + + jest.useRealTimers(); + }); }); From 373b3772ac887035325ad91c1e18a432b45b1501 Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Mon, 10 Nov 2025 22:43:21 +0300 Subject: [PATCH 5/8] add comments for future proofing --- superset-frontend/src/components/Chart/chartReducer.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts index e43e954e6658..49f74b7b18e2 100644 --- a/superset-frontend/src/components/Chart/chartReducer.ts +++ b/superset-frontend/src/components/Chart/chartReducer.ts @@ -68,6 +68,11 @@ export default function chartReducer( }, [actions.CHART_UPDATE_STARTED](state) { const controller = state.queryController; + /** + * Race condition fix for cross-filters: + * When a filter is removed while charts are loading, abort the + * in-flight request to prevent stale filtered data from rendering. + */ if (controller) { setTimeout(() => controller.abort(), 0); } From 3513313362ab577ac83522c1fceb6161ed2da3cd Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Wed, 12 Nov 2025 20:17:11 +0300 Subject: [PATCH 6/8] fis state update --- superset-frontend/src/components/Chart/chartAction.js | 11 ++++++++++- .../src/components/Chart/chartReducer.ts | 9 --------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js index ff1d6744bf17..91f9b3894a42 100644 --- a/superset-frontend/src/components/Chart/chartAction.js +++ b/superset-frontend/src/components/Chart/chartAction.js @@ -407,10 +407,19 @@ export function exploreJSON( ownState, ) { return async (dispatch, getState) => { + const state = getState(); const logStart = Logger.getTimestamp(); const controller = new AbortController(); + const prevController = state.charts?.[key]?.queryController; + /** + * Abort in-flight requests before starting a new query to avoid race + * conditions where stale data renders after filters change. + */ + if (prevController) { + prevController.abort(); + } const queryTimeout = - timeout || getState().common.conf.SUPERSET_WEBSERVER_TIMEOUT; + timeout || state.common.conf.SUPERSET_WEBSERVER_TIMEOUT; const requestParams = { signal: controller.signal, diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts index 49f74b7b18e2..081bd4154724 100644 --- a/superset-frontend/src/components/Chart/chartReducer.ts +++ b/superset-frontend/src/components/Chart/chartReducer.ts @@ -67,15 +67,6 @@ export default function chartReducer( }; }, [actions.CHART_UPDATE_STARTED](state) { - const controller = state.queryController; - /** - * Race condition fix for cross-filters: - * When a filter is removed while charts are loading, abort the - * in-flight request to prevent stale filtered data from rendering. - */ - if (controller) { - setTimeout(() => controller.abort(), 0); - } return { ...state, chartStatus: 'loading', From 28e5336ebcefac1dead4da49d9633ddf0d157b24 Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Wed, 12 Nov 2025 20:34:00 +0300 Subject: [PATCH 7/8] fis state update --- .../src/components/Chart/chartAction.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js index 91f9b3894a42..bd9bd94a56ba 100644 --- a/superset-frontend/src/components/Chart/chartAction.js +++ b/superset-frontend/src/components/Chart/chartAction.js @@ -411,13 +411,6 @@ export function exploreJSON( const logStart = Logger.getTimestamp(); const controller = new AbortController(); const prevController = state.charts?.[key]?.queryController; - /** - * Abort in-flight requests before starting a new query to avoid race - * conditions where stale data renders after filters change. - */ - if (prevController) { - prevController.abort(); - } const queryTimeout = timeout || state.common.conf.SUPERSET_WEBSERVER_TIMEOUT; @@ -431,6 +424,14 @@ export function exploreJSON( dispatch(updateDataMask(formData.slice_id, dataMask)); }; dispatch(chartUpdateStarted(controller, formData, key)); + /** + * Abort in-flight requests after the new controller has been stored in + * state. Delaying ensures we do not mutate the Redux state between + * dispatches while still cancelling the previous request promptly. + */ + if (prevController) { + setTimeout(() => prevController.abort(), 0); + } const chartDataRequest = getChartDataRequest({ setDataMask, From e8b16557b6510a515f5cc9c21bc3aad6cf125603 Mon Sep 17 00:00:00 2001 From: LevisNgigi Date: Wed, 12 Nov 2025 21:04:48 +0300 Subject: [PATCH 8/8] add tests --- .../src/components/Chart/chartActions.test.js | 64 +++++++++++++++++++ .../components/Chart/chartReducers.test.js | 31 --------- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/superset-frontend/src/components/Chart/chartActions.test.js b/superset-frontend/src/components/Chart/chartActions.test.js index 55b86eb90082..4d57f53c2cb8 100644 --- a/superset-frontend/src/components/Chart/chartActions.test.js +++ b/superset-frontend/src/components/Chart/chartActions.test.js @@ -31,6 +31,7 @@ import * as exploreUtils from 'src/explore/exploreUtils'; import * as actions from 'src/components/Chart/chartAction'; import * as asyncEvent from 'src/middleware/asyncEvent'; import { handleChartDataResponse } from 'src/components/Chart/chartAction'; +import * as dataMaskActions from 'src/dataMask/actions'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; @@ -110,6 +111,69 @@ describe('chart actions', () => { .callsFake(data => Promise.resolve(data)); }); + test('should defer abort of previous controller to avoid Redux state mutation', async () => { + jest.useFakeTimers(); + const chartKey = 'defer_abort_test'; + const formData = { + slice_id: 123, + datasource: 'table__1', + viz_type: 'table', + }; + const oldController = new AbortController(); + const abortSpy = jest.spyOn(oldController, 'abort'); + const state = { + charts: { + [chartKey]: { + queryController: oldController, + }, + }, + common: { + conf: { + SUPERSET_WEBSERVER_TIMEOUT: 60, + }, + }, + }; + const getState = jest.fn(() => state); + const dispatchMock = jest.fn(); + const getChartDataRequestSpy = jest + .spyOn(actions, 'getChartDataRequest') + .mockResolvedValue({ + response: { status: 200 }, + json: { result: [] }, + }); + const handleChartDataResponseSpy = jest + .spyOn(actions, 'handleChartDataResponse') + .mockResolvedValue([]); + const updateDataMaskSpy = jest + .spyOn(dataMaskActions, 'updateDataMask') + .mockReturnValue({ type: 'UPDATE_DATA_MASK' }); + const getQuerySettingsStub = sinon + .stub(exploreUtils, 'getQuerySettings') + .returns([false, () => {}]); + + try { + const thunk = actions.exploreJSON(formData, false, undefined, chartKey); + const promise = thunk(dispatchMock, getState); + + expect(abortSpy).not.toHaveBeenCalled(); + expect(oldController.signal.aborted).toBe(false); + + jest.runOnlyPendingTimers(); + + expect(abortSpy).toHaveBeenCalledTimes(1); + expect(oldController.signal.aborted).toBe(true); + + await promise; + } finally { + getChartDataRequestSpy.mockRestore(); + handleChartDataResponseSpy.mockRestore(); + updateDataMaskSpy.mockRestore(); + getQuerySettingsStub.restore(); + abortSpy.mockRestore(); + jest.useRealTimers(); + } + }); + afterEach(() => { getExploreUrlStub.restore(); getChartDataUriStub.restore(); diff --git a/superset-frontend/src/components/Chart/chartReducers.test.js b/superset-frontend/src/components/Chart/chartReducers.test.js index 6b8f599775de..a4081aa851bc 100644 --- a/superset-frontend/src/components/Chart/chartReducers.test.js +++ b/superset-frontend/src/components/Chart/chartReducers.test.js @@ -60,35 +60,4 @@ describe('chart reducers', () => { expect(newState[chartKey].chartUpdateEndTime).toBeGreaterThan(0); expect(newState[chartKey].chartStatus).toEqual('failed'); }); - - test('should defer abort of previous controller to avoid Redux state mutation', () => { - jest.useFakeTimers(); - - const oldController = new AbortController(); - const abortSpy = jest.spyOn(oldController, 'abort'); - - const chartWithController = { - ...testChart, - queryController: oldController, - chartStatus: 'loading', - }; - - const newController = new AbortController(); - const newState = chartReducer( - { [chartKey]: chartWithController }, - actions.chartUpdateStarted(newController, {}, chartKey), - ); - - expect(abortSpy).not.toHaveBeenCalled(); - expect(oldController.signal.aborted).toBe(false); - - jest.runAllTimers(); - expect(abortSpy).toHaveBeenCalledTimes(1); - expect(oldController.signal.aborted).toBe(true); - - expect(newState[chartKey].queryController).toBe(newController); - expect(newState[chartKey].chartStatus).toBe('loading'); - - jest.useRealTimers(); - }); });