mirror of
https://github.com/apache/superset.git
synced 2026-05-13 03:45:12 +00:00
fix(explore): Prevent error toast when navigating away from Explore page (#39065)
(cherry picked from commit 66a9e2e16e)
This commit is contained in:
committed by
Michael S. Molina
parent
2b4e9909db
commit
9499ccf52c
@@ -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(<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 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(
|
||||
<>
|
||||
<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.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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,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);
|
||||
|
||||
Reference in New Issue
Block a user