diff --git a/superset-frontend/src/pages/Chart/Chart.test.tsx b/superset-frontend/src/pages/Chart/Chart.test.tsx index 58106375f0e..b80ccc32b29 100644 --- a/superset-frontend/src/pages/Chart/Chart.test.tsx +++ b/superset-frontend/src/pages/Chart/Chart.test.tsx @@ -32,6 +32,7 @@ import { URL_PARAMS } from 'src/constants'; import { JsonObject, VizType } from '@superset-ui/core'; import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt'; import { getParsedExploreURLParams } from 'src/explore/exploreUtils/getParsedExploreURLParams'; +import * as messageToastActions from 'src/components/MessageToasts/actions'; import ChartPage from '.'; jest.mock('src/hooks/useUnsavedChangesPrompt', () => ({ @@ -358,4 +359,96 @@ describe('ChartPage', () => { ); }); }); + + test('does not show error toast when request is aborted on unmount', async () => { + const addDangerToastSpy = jest.spyOn(messageToastActions, 'addDangerToast'); + 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(, { + 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 for the rejected request to settle before asserting no toast was shown + await pendingPromise.catch(() => undefined); + expect(addDangerToastSpy).not.toHaveBeenCalled(); + + addDangerToastSpy.mockRestore(); + }); + + test('aborts in-flight request when a new request is made', async () => { + const addDangerToastSpy = jest.spyOn(messageToastActions, 'addDangerToast'); + const exploreApiRoute = 'glob:*/api/v1/explore/*'; + const exploreFormData = getExploreFormData({ + viz_type: VizType.Table, + show_cell_bars: true, + }); + + // First request will reject with AbortError when aborted + let rejectFirstRequest: (error: Error) => void; + const firstRequestPromise = new Promise((_, reject) => { + rejectFirstRequest = reject; + }); + + fetchMock.get(exploreApiRoute, () => firstRequestPromise); + + render( + <> + Navigate + + , + { + 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.clearHistory().removeRoutes(); + fetchMock.get(exploreApiRoute, { + result: { dataset: { id: 1 }, form_data: exploreFormData }, + }); + + // Navigate to trigger a new request (which should abort the first) + fireEvent.click(screen.getByText('Navigate')); + + // Simulate the first request being aborted + const abortError = new Error('The operation was aborted.'); + abortError.name = 'AbortError'; + rejectFirstRequest!(abortError); + + // Wait for the first request to settle before asserting + await firstRequestPromise.catch(() => undefined); + + // Wait for the second request to complete + await waitFor(() => + expect(fetchMock.callHistory.calls(exploreApiRoute).length).toBe(1), + ); + + // No error toast should be shown from the aborted first request + expect(addDangerToastSpy).not.toHaveBeenCalled(); + + addDangerToastSpy.mockRestore(); + }); }); diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx index 46acccad36a..d1e3f7b8f75 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(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,19 @@ export default function ExplorePage() { }), ); }) - .catch(err => Promise.all([getClientErrorObject(err), err])) + .catch(err => { + // Silently ignore aborted requests - AbortError may be wrapped in SupersetApiError by makeApi + // or come through with statusText === 'abort' from SupersetClient + if ( + err.name === 'AbortError' || + err.statusText === 'abort' || + err.originalError?.name === 'AbortError' || + err.originalError?.statusText === 'abort' + ) { + return; + } + return Promise.all([getClientErrorObject(err), err]); + }) .then(resolved => { if (isStale()) { return; @@ -251,7 +273,7 @@ export default function ExplorePage() { return Promise.resolve(); }) .finally(() => { - if (!isStale()) { + if (!isStale() && !controller.signal.aborted) { setIsLoaded(true); } }); @@ -259,6 +281,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);