diff --git a/cypress/integration/basic.ts b/cypress/integration/basic.ts index 91a1211725..bfafcbf916 100644 --- a/cypress/integration/basic.ts +++ b/cypress/integration/basic.ts @@ -8,34 +8,13 @@ describe('basic test', () => { it('internal sidebar links work', () => { cy.visit('/') - waitInDevMode(100); - cy.findByTestId('sidebar-comparison').click(); - waitInDevMode(100); cy.location('pathname').should('eq', '/comparison'); cy.findByTestId('sidebar-comparison-diff').click(); - waitInDevMode(100); cy.location('pathname').should('eq', '/comparison-diff'); cy.findByTestId('sidebar-root').click(); - waitInDevMode(100); cy.location('pathname').should('eq', '/'); }); }) - -// very nasty, just to avoid dealing with the following error -// which requires aborting fetch call and whatnot -// react-dom.development.js:21 Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method. -// in FlameGraphRenderer (created by Context.Consumer) -// in e (created by ConnectFunction) -// in ConnectFunction (created by PyroscopeApp) -// in div (created by PyroscopeApp) -// in div (created by PyroscopeApp) -// in PyroscopeApp (created by ConnectFunction) -// in ConnectFunction -function waitInDevMode(t: number) { - if (!process.env.CI) { - cy.wait(t); - } -} diff --git a/webapp/javascript/components/ComparisonApp.jsx b/webapp/javascript/components/ComparisonApp.jsx index eca39cfb94..40ea5bb865 100644 --- a/webapp/javascript/components/ComparisonApp.jsx +++ b/webapp/javascript/components/ComparisonApp.jsx @@ -20,6 +20,8 @@ function ComparisonApp(props) { if (prevPropsRef.renderURL !== renderURL) { actions.fetchTimeline(renderURL); } + + return actions.abortTimelineRequest; }, [renderURL]); return ( diff --git a/webapp/javascript/components/ComparisonDiffApp.jsx b/webapp/javascript/components/ComparisonDiffApp.jsx index 2edbb3028b..1745cdc8a3 100644 --- a/webapp/javascript/components/ComparisonDiffApp.jsx +++ b/webapp/javascript/components/ComparisonDiffApp.jsx @@ -16,6 +16,7 @@ function ComparisonDiffApp(props) { if (prevPropsRef.diffRenderURL !== diffRenderURL) { actions.fetchTimeline(diffRenderURL); } + return actions.abortTimelineRequest; }, [diffRenderURL]); return ( diff --git a/webapp/javascript/components/FlameGraphRenderer.jsx b/webapp/javascript/components/FlameGraphRenderer.jsx index c1637e4d67..4b2fae556f 100644 --- a/webapp/javascript/components/FlameGraphRenderer.jsx +++ b/webapp/javascript/components/FlameGraphRenderer.jsx @@ -42,6 +42,7 @@ import ProfilerHeader from "./ProfilerHeader"; import { deltaDiffWrapper, parseFlamebearerFormat } from "../util/flamebearer"; import ExportData from "./ExportData"; +import {isAbortError} from "../util/abort"; const PX_PER_LEVEL = 18; @@ -143,10 +144,18 @@ class FlameGraphRenderer extends React.Component { } } - fetchFlameBearerData(url) { + componentWillUnmount() { + this.abortCurrentJSONController(); + } + + abortCurrentJSONController() { if (this.currentJSONController) { this.currentJSONController.abort(); } + } + + fetchFlameBearerData(url) { + this.abortCurrentJSONController(); this.currentJSONController = new AbortController(); fetch(`${url}&format=json`, { signal: this.currentJSONController.signal }) @@ -161,6 +170,12 @@ class FlameGraphRenderer extends React.Component { this.updateData(); }) }) + .catch(e => { + // AbortErrors are fine + if (!isAbortError(e)) { + throw e; + } + }) .finally(); } diff --git a/webapp/javascript/components/PyroscopeApp.jsx b/webapp/javascript/components/PyroscopeApp.jsx index 91ad6f9158..db19423337 100644 --- a/webapp/javascript/components/PyroscopeApp.jsx +++ b/webapp/javascript/components/PyroscopeApp.jsx @@ -8,7 +8,7 @@ import TimelineChartWrapper from "./TimelineChartWrapper"; import Header from "./Header"; import Footer from "./Footer"; import { buildRenderURL } from "../util/updateRequests"; -import { fetchNames, fetchTimeline } from "../redux/actions"; +import { fetchNames, fetchTimeline, abortTimelineRequest } from "../redux/actions"; function PyroscopeApp(props) { const { actions, renderURL } = props; @@ -18,6 +18,8 @@ function PyroscopeApp(props) { if (prevPropsRef.renderURL !== renderURL) { actions.fetchTimeline(renderURL); } + + return actions.abortTimelineRequest; }, [renderURL]); return ( @@ -42,6 +44,7 @@ const mapDispatchToProps = (dispatch) => ({ { fetchTimeline, fetchNames, + abortTimelineRequest, }, dispatch ), diff --git a/webapp/javascript/components/TagsBar.jsx b/webapp/javascript/components/TagsBar.jsx index 854042d4cf..20f2333e85 100644 --- a/webapp/javascript/components/TagsBar.jsx +++ b/webapp/javascript/components/TagsBar.jsx @@ -4,7 +4,7 @@ import { connect } from "react-redux"; import "react-dom"; import { Menu, SubMenu, MenuItem, MenuButton } from "@szhsin/react-menu"; -import { fetchTags, fetchTagValues, setQuery } from "../redux/actions"; +import { fetchTags, fetchTagValues, setQuery, abortFetchTags, abortFetchTagValues } from "../redux/actions"; import "../util/prism"; function TagsBar({ query, actions, tags, tagValuesLoading }) { @@ -16,6 +16,8 @@ function TagsBar({ query, actions, tags, tagValuesLoading }) { useEffect(() => { actions.fetchTags(query); + + return actions.abortFetchTags; }, [query]); const submitTagsValue = (newValue) => { @@ -40,6 +42,10 @@ function TagsBar({ query, actions, tags, tagValuesLoading }) { const loadTagValues = (tag) => { actions.fetchTagValues(query, tag); }; + useEffect(() => { + // since fetchTagValues may be running + return actions.abortFetchTagValues; + }, []) const onTagsValueChange = (tagKey, tagValue) => { let newQuery; @@ -135,6 +141,7 @@ const mapDispatchToProps = (dispatch) => ({ { fetchTags, fetchTagValues, + abortFetchTags, setQuery, }, dispatch diff --git a/webapp/javascript/redux/actions.js b/webapp/javascript/redux/actions.js index ab453e5f7b..cca883a8a3 100644 --- a/webapp/javascript/redux/actions.js +++ b/webapp/javascript/redux/actions.js @@ -20,6 +20,7 @@ import { SET_RIGHT_FROM, SET_RIGHT_UNTIL, } from "./actionTypes"; +import { isAbortError } from "../util/abort"; export const setDateRange = (from, until) => ({ type: SET_DATE_RANGE, @@ -103,7 +104,15 @@ export const setQuery = (query) => ({ payload: { query }, }); +/** + * ATTENTION! There may be race conditions: + * Since a new controller is created every time a 'fetch' action is called + * A badly timed 'abort' action may cancel the brand new 'fetch' action! + */ let currentTimelineController; +let fetchTagController; +let fetchTagValuesController; + export function fetchTimeline(url) { return (dispatch) => { if (currentTimelineController) { @@ -118,24 +127,62 @@ export function fetchTimeline(url) { .then((data) => { dispatch(receiveTimeline(data)); }) + .catch((e) => { + // AbortErrors are fine + if (!isAbortError(e)) { + throw e; + } + }) .finally(); }; } +export function abortTimelineRequest() { + return () => { + if (currentTimelineController) { + currentTimelineController.abort(); + } + }; +} + export function fetchTags(query) { return (dispatch) => { + if (fetchTagController) { + fetchTagController.abort(); + } + fetchTagController = new AbortController(); + dispatch(requestTags()); return fetch(`/labels?query=${encodeURIComponent(query)}`) .then((response) => response.json()) .then((data) => { dispatch(receiveTags(data)); }) + .catch((e) => { + // AbortErrors are fine + if (!isAbortError(e)) { + throw e; + } + }) .finally(); }; } +export function abortFetchTags() { + return () => { + if (fetchTagController) { + fetchTagController.abort(); + } + }; +} + export function fetchTagValues(query, tag) { return (dispatch) => { + if (fetchTagValuesController) { + fetchTagValuesController.abort(); + } + fetchTagValuesController = new AbortController(); + dispatch(requestTagValues(tag)); return fetch( `/label-values?label=${encodeURIComponent( @@ -146,9 +193,22 @@ export function fetchTagValues(query, tag) { .then((data) => { dispatch(receiveTagValues(data, tag)); }) + .catch((e) => { + // AbortErrors are fine + if (!fetchTagValuesController.signal.aborted) { + throw e; + } + }) .finally(); }; } +export function abortFetchTagValues() { + return () => { + if (fetchTagValuesController) { + fetchTagValuesController.abort(); + } + }; +} let currentNamesController; export function fetchNames() { @@ -166,6 +226,19 @@ export function fetchNames() { .then((data) => { dispatch(receiveNames(data)); }) + .catch((e) => { + // AbortErrors are fine + if (!isAbortError(e)) { + throw e; + } + }) .finally(); }; } +export function abortFetchNames() { + return () => { + if (abortFetchNames) { + abortFetchNames.abort(); + } + }; +} diff --git a/webapp/javascript/util/abort.js b/webapp/javascript/util/abort.js new file mode 100644 index 0000000000..6ed58b5129 --- /dev/null +++ b/webapp/javascript/util/abort.js @@ -0,0 +1,9 @@ +export function isAbortError(err) { + if (!err) { + return false; + } + + // https://developer.mozilla.org/en-US/docs/Web/API/DOMException + return err.name === 'AbortError' + || error.code === 20; +}