fix errors in log entries polling logic

PagerDuty · Jun 29, 2023 · b2bfede · b2bfede
1 parent e7f458f
commit b2bfede
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 24 deletions.
diff --git a/src/App.js b/src/App.js
@@ -108,12 +108,8 @@ const App = () => {
 
   const darkMode = useSelector((state) => state.settings.darkMode);
   const abilities = useSelector((state) => state.connection.abilities);
-  const queryError = useSelector((state) => state.querySettings.error);
   const {
     fetchingIncidents,
-    fetchingIncidentNotes,
-    fetchingIncidentAlerts,
-    refreshingIncidents,
     lastFetchDate,
   } = useSelector((state) => state.incidents);
   const {
@@ -155,22 +151,35 @@ const App = () => {
     moment.locale(currentUserLocale);
   }, [currentUserLocale]);
 
+  // use these refs in the polling interval to avoid stale values
+  // without having to add them to the dependency array
+  const latestLogEntryDateRef = useRef(latestLogEntryDate);
+  useEffect(() => {
+    latestLogEntryDateRef.current = latestLogEntryDate;
+  }, [latestLogEntryDate]);
+  const fetchingIncidentsRef = useRef(fetchingIncidents);
+  useEffect(() => {
+    fetchingIncidentsRef.current = fetchingIncidents;
+  }, [fetchingIncidents]);
+  const fetchingLogEntriesRef = useRef(fetchingLogEntries);
+  useEffect(() => {
+    fetchingLogEntriesRef.current = fetchingLogEntries;
+  }, [fetchingLogEntries]);
+
+  // Set up log entry polling
   useEffect(
     () => {
       const pollingInterval = setInterval(() => {
         checkConnectionStatus();
-        if (userAuthorized && abilities.includes(PD_REQUIRED_ABILITY) && !queryError) {
-          if (fetchingLogEntries) {
+        if (userAuthorized && abilities.includes(PD_REQUIRED_ABILITY)) {
+          if (fetchingLogEntriesRef.current) {
             // eslint-disable-next-line no-console
-            console.log('skipping log entries fetch because already fetching');
+            console.error('skipping log entries fetch because already fetching log entries');
             return;
           }
 
           if (
-            !fetchingIncidents
-            && !fetchingIncidentNotes
-            && !fetchingIncidentAlerts
-            && !refreshingIncidents
+            !fetchingIncidentsRef.current
             && !DEBUG_DISABLE_POLLING
           ) {
             // Determine lookback based on last fetch/refresh of incidents
@@ -181,24 +190,21 @@ const App = () => {
               since = new Date(lastFetchDate - 1000);
             }
             // If we have a latest log entry date and it's newer than last fetch date, use that
-            if (latestLogEntryDate && latestLogEntryDate > since) {
-              since = new Date(latestLogEntryDate - 1000);
+            if (latestLogEntryDateRef.current && latestLogEntryDateRef.current > since) {
+              since = new Date(latestLogEntryDateRef.current - 1000);
             }
             getLogEntriesAsync(since);
+          } else if (fetchingIncidentsRef.current) {
+            // eslint-disable-next-line no-console
+            console.error('skipping log entries fetch because already fetching incidents');
           }
         }
       }, LOG_ENTRIES_POLLING_INTERVAL_SECONDS * 1000);
       return () => clearInterval(pollingInterval);
     },
-    // Changes to any of these in the store resets log entries timer
     [
       userAuthorized,
-      queryError,
-      fetchingIncidents,
-      fetchingIncidentNotes,
-      fetchingIncidentAlerts,
-      refreshingIncidents,
-      latestLogEntryDate,
+      lastFetchDate,
     ],
   );
 

diff --git a/src/redux/incidents/reducers.js b/src/redux/incidents/reducers.js
@@ -63,6 +63,14 @@ import {
   FILTER_INCIDENTS_LIST_BY_QUERY_ERROR,
 } from './actions';
 
+const uniqOnId = (arr) => {
+  const map = new Map();
+  arr.forEach((item) => {
+    map.set(item.id, item);
+  });
+  return [...map.values()];
+};
+
 const incidents = produce(
   (draft, action) => {
     switch (action.type) {
@@ -219,9 +227,9 @@ const incidents = produce(
           if (!draft.incidentAlerts[incidentId]) {
             draft.incidentAlerts[incidentId] = [];
           }
-          draft.incidentAlerts[incidentId] = draft.incidentAlerts[incidentId].concat(
+          draft.incidentAlerts[incidentId] = uniqOnId(draft.incidentAlerts[incidentId].concat(
             action.incidentAlertsMap[incidentId],
-          );
+          ));
         }
         for (let i = 0; i < Object.keys(action.incidentAlertsUnlinkMap).length; i++) {
           const incidentId = Object.keys(action.incidentAlertsUnlinkMap)[i];

diff --git a/src/util/pd-api-wrapper.js b/src/util/pd-api-wrapper.js
@@ -58,9 +58,9 @@ export const pdAxiosRequest = async (method, endpoint, params = {}, data = {}) =
       return `Token token=${tokenObj.token}`;
     })(),
     Accept: 'application/vnd.pagerduty+json;version=2',
-    'content-type': 'application/json; charset=utf-8',
+    'Content-Type': 'application/json; charset=utf-8',
   },
-  params,
+  params: { ...params, rand: Math.random().toString(36).substring(2, 7) },
   data,
 });