diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js index ff1d6744bf17..bd9bd94a56ba 100644 --- a/superset-frontend/src/components/Chart/chartAction.js +++ b/superset-frontend/src/components/Chart/chartAction.js @@ -407,10 +407,12 @@ 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; const queryTimeout = - timeout || getState().common.conf.SUPERSET_WEBSERVER_TIMEOUT; + timeout || state.common.conf.SUPERSET_WEBSERVER_TIMEOUT; const requestParams = { signal: controller.signal, @@ -422,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, 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();