Merge pull request #170 from alanconway/useeffect-race

NO-JIRA: fix: Reduce useEffect dependencies to avoid cancelled requests.

Author: openshift-merge-bot[bot]

web/locales/en/plugin__troubleshooting-panel-console-plugin.json

@@ -2,13 +2,14 @@
   "Advanced": "Advanced",
   "Ago": "Ago",
   "Correlation result was empty.": "Correlation result was empty.",
-  "Create a graph of correlated items from resources in the current view.": "Create a graph of correlated items from resources in the curret view.",
+  "Create a graph of items correlated from resources in the current page.": "Create a graph of items correlated from resources in the current page.",
   "Distance": "Distance",
   "Find all paths to items of the specified goal class.": "Find all paths to items of the specified goal class.",
   "Focus": "Focus",
   "Follows correlation rules from the starting point to find related data, then continues the search from that data, up to the number of steps you specify.": "Follows correlation rules from the starting point to find related data, then continues the search from that data, up to the number of steps you specify.",
   "Goal Class": "Goal Class",
   "Korrel8r Error": "Korrel8r Error",
+  "Loading": "Loading",
   "Logging Plugin Disabled": "Logging Plugin Disabled",
   "Netflow Plugin Disabled": "Netflow Plugin Disabled",
   "No Correlated Signals Found": "No Correlated Signals Found",
@@ -24,7 +25,7 @@
   "Search Type": "Search Type",
   "Selects the starting point for correlation search. This query is set automatically by the <1>Focus</1> button. You can edit it manually to specify a custom query.": "Selects the starting point for correlation search. This query is set automatically by the <1>Focus</1> button. You can edit it manually to specify a custom query.",
   "Since": "Since",
-  "The current view does not support correlation.": "The current view does not support correlation.",
+  "The current page does not support correlation.": "The current page does not support correlation.",
   "Time": "Time",
   "to": "to",
   "Troubleshooting": "Troubleshooting",

web/src/components/Korrel8rPanel.tsx

@@ -51,7 +51,6 @@ export default function Korrel8rPanel() {
   });
   const dispatch = useDispatch();
 
-  // State
   const alertRules = useSelector((state: State) => state.observe?.get('rules'));
   const alertIDs = React.useMemo(() => {
     if (!alertRules) return new Map<string, string>();
@@ -62,79 +61,67 @@ export default function Korrel8rPanel() {
     [alertIDs],
   );
   const locationQuery = useLocationQuery(domains);
+
+  // Search parameters.
   const [search, setSearch] = React.useState<Search>({
     ...defaultSearch, // Default search parameters.
-    queryStr: locationQuery?.toString(), // Default query string.
+    queryStr: locationQuery?.toString(), // Default query string from location.
     ...searchResult?.search, // Use persisted search if available.
   });
+  // Search result
   const [result, setResult] = React.useState<Result | null>(searchResult?.result ?? null);
+  // Showing advanced query
   const [showQuery, setShowQuery] = React.useState(false);
 
-  const focusTip = t('Create a graph of correlated items from resources in the current view.');
-  const cannotFocus = t('The current view does not support correlation.');
-
   React.useEffect(() => {
-    // Set result = null to trigger a new client request, don't run the query till then.
-    if (result !== null) {
-      return;
-    }
-    if (!search?.queryStr && !locationQuery) {
-      setResult({ message: cannotFocus });
-      return;
-    }
     // eslint-disable-next-line no-console
     console.debug('korrel8r search', search);
     const queryStr = search?.queryStr?.trim();
     const start: api.Start = {
       queries: queryStr ? [queryStr] : undefined,
       constraint: search?.constraint?.toAPI() ?? undefined,
     };
-    const cancellableFetch =
+    let cancelled = false; // Detect if returned cleanup function was called.
+    const onResult = (newResult: Result) => {
+      if (!cancelled) {
+        setResult(newResult);
+        dispatch(setPersistedSearch({ search, result: newResult }));
+      }
+      // eslint-disable-next-line no-console
+      console.debug('korrel8r result', newResult, 'cancelled', cancelled);
+    };
+    const fetch =
       search.type === SearchType.Goal
         ? getGoalsGraph({ start, goals: [search.goal] })
         : getNeighborsGraph({ start, depth: search.depth });
-
-    cancellableFetch
-      .then((response: api.Graph) => {
-        const result: Result = { graph: new korrel8r.Graph(response) };
-        // eslint-disable-next-line no-console
-        console.debug('korrel8r result', result);
-        setResult(result);
-        dispatch(setPersistedSearch({ search, result }));
-      })
+    fetch
+      .then((response: api.Graph) => onResult({ graph: new korrel8r.Graph(response) }))
       .catch((e: api.ApiError) => {
-        const result = {
-          message: e.body?.error || e.message || 'Unknown Error',
+        onResult({
           title: e?.body?.error ? t('Korrel8r Error') : t('Request Failed'),
-        };
-        // eslint-disable-next-line no-console
-        console.debug('korrel8r result', result);
-        setResult(result);
-        dispatch(setPersistedSearch({ search, result }));
+          message: e?.body?.error || e.message || 'Unknown Error',
+        });
       });
-    return () => cancellableFetch.cancel();
-  }, [result, t, dispatch, search, cannotFocus, locationQuery]);
+    return () => {
+      cancelled = true;
+      fetch.cancel();
+    };
+  }, [search, t, dispatch]);
 
   const queryToggleID = 'query-toggle';
   const queryContentID = 'query-content';
   const queryInputID = 'query-input';
 
-  const depthBounds = applyBounds(1, 10);
-
-  const runSearch = React.useCallback(
-    (newSearch: Search) => {
-      // Update constraint from time period
-      if (newSearch.period) {
-        const [start, end] = newSearch.period.startEnd();
-        newSearch.constraint = new korrel8r.Constraint({ ...newSearch.constraint, start, end });
-      }
-      newSearch.depth = depthBounds(newSearch.depth);
-      newSearch.type = !newSearch.goal ? SearchType.Distance : newSearch.type;
-      setSearch(newSearch);
-      setResult(null);
-    },
-    [setResult, depthBounds],
-  );
+  const runSearch = React.useCallback((newSearch: Search) => {
+    // Update constraint from time period
+    if (newSearch.period) {
+      const [start, end] = newSearch.period.startEnd();
+      newSearch.constraint = new korrel8r.Constraint({ ...newSearch.constraint, start, end });
+    }
+    newSearch.depth = Math.max(1, Math.min(newSearch.depth, 10));
+    setSearch({ ...newSearch }); // Create a new search object to trigger useEffect
+    setResult(null);
+  }, []);
 
   const queryHelp = (
     <>
@@ -153,7 +140,13 @@ export default function Korrel8rPanel() {
   );
 
   const focusButton = (
-    <Tooltip content={locationQuery ? focusTip : cannotFocus}>
+    <Tooltip
+      content={
+        locationQuery
+          ? t('Create a graph of items correlated from resources in the current page.')
+          : t('The current page does not support correlation.')
+      }
+    >
       <Button
         isAriaDisabled={!locationQuery}
         onClick={() => {
@@ -346,7 +339,3 @@ const TopologyInfoState: React.FC<TopologyInfoStateProps> = ({ titleText, text,
     </div>
   );
 };
-
-const applyBounds = (min: number, max: number) => {
-  return (val: number) => Math.max(min, Math.min(val, max));
-};