cypress: cleanup nasty waitInDevMode (#380)

aborted requests when component is unmounting
which usually happens in cypress tests
since we click around faster than the server responds

Author: eh-am

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);
-  }
-}

webapp/javascript/components/ComparisonApp.jsx

@@ -20,6 +20,8 @@ function ComparisonApp(props) {
     if (prevPropsRef.renderURL !== renderURL) {
       actions.fetchTimeline(renderURL);
     }
+
+    return actions.abortTimelineRequest;
   }, [renderURL]);
 
   return (

webapp/javascript/components/ComparisonDiffApp.jsx

@@ -16,6 +16,7 @@ function ComparisonDiffApp(props) {
     if (prevPropsRef.diffRenderURL !== diffRenderURL) {
       actions.fetchTimeline(diffRenderURL);
     }
+    return actions.abortTimelineRequest;
   }, [diffRenderURL]);
 
   return (

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();
   }
 

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
   ),

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

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();
+    }
+  };
+}

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;
+}