fix(app): Fix React Router navigation in React Query callbacks

app/src/App/hooks.ts

@@ -126,14 +126,20 @@ export function useProtocolReceiptToast(): void {
 
 export function useCurrentRunRoute(): string | null {
   const currentRunId = useCurrentRunId({ refetchInterval: CURRENT_RUN_POLL })
-  const { data: runRecord } = useNotifyRunQuery(currentRunId, {
-    staleTime: Infinity,
-    enabled: currentRunId != null,
+  const { data: runRecord, isFetching } = useNotifyRunQuery(currentRunId, {
+    refetchInterval: CURRENT_RUN_POLL,
   })
 
   const runStatus = runRecord?.data.status
   const runActions = runRecord?.data.actions
-  if (runRecord == null || runStatus == null || runActions == null) return null
+  if (
+    runRecord == null ||
+    runStatus == null ||
+    runActions == null ||
+    isFetching
+  ) {
+    return null
+  }
   // grabbing run id off of the run query to have all routing info come from one source of truth
   const runId = runRecord.data.id
   const hasRunStarted = runActions?.some(

app/src/organisms/ProtocolUpload/hooks/useCloseCurrentRun.ts

@@ -23,10 +23,10 @@ export function useCloseCurrentRun(): {
   ): void => {
     if (currentRunId != null) {
       dismissCurrentRun(currentRunId, {
-        ...options,
         onError: () => {
           console.warn('failed to dismiss current')
         },
+        ...options,
       })
     }
   }

app/src/pages/RunSummary/index.tsx

@@ -84,7 +84,7 @@ export function RunSummary(): JSX.Element {
     staleTime: Infinity,
     onError: () => {
       // in case the run is remotely deleted by a desktop app, navigate to the dash
-      navigate('/')
+      navigate('/dashboard')
     },
   })
   const isRunCurrent = Boolean(
@@ -144,6 +144,19 @@ export function RunSummary(): JSX.Element {
   const { reset, isResetRunLoading } = useRunControls(runId, onCloneRunSuccess)
   const trackEvent = useTrackEvent()
   const { closeCurrentRun, isClosingCurrentRun } = useCloseCurrentRun()
+  // Close the current run only if it's active and then execute the onSuccess callback. Prefer this wrapper over
+  // closeCurrentRun directly, since the callback is swallowed if currentRun is null.
+  const closeCurrentRunIfValid = (onSuccess?: () => void): void => {
+    if (isRunCurrent) {
+      closeCurrentRun({
+        onSuccess: () => {
+          onSuccess?.()
+        },
+      })
+    } else {
+      onSuccess?.()
+    }
+  }
   const [showRunFailedModal, setShowRunFailedModal] = React.useState<boolean>(
     false
   )
@@ -239,11 +252,9 @@ export function RunSummary(): JSX.Element {
       deleteRun(runId)
       navigate('/quick-transfer')
     } else {
-      closeCurrentRun({
-        onSuccess: () => {
-          deleteRun(runId)
-          navigate('/quick-transfer')
-        },
+      closeCurrentRunIfValid(() => {
+        deleteRun(runId)
+        navigate('/quick-transfer')
       })
     }
   }
@@ -281,20 +292,16 @@ export function RunSummary(): JSX.Element {
         robotType: FLEX_ROBOT_TYPE,
         isRunCurrent,
         onSkipAndHome: () => {
-          closeCurrentRun({
-            onSettled: () => {
-              navigate('/')
-            },
+          closeCurrentRunIfValid(() => {
+            navigate('/dashboard')
           })
         },
       })
     } else if (isQuickTransfer) {
       returnToQuickTransfer()
     } else {
-      closeCurrentRun({
-        onSettled: () => {
-          navigate('/')
-        },
+      closeCurrentRunIfValid(() => {
+        navigate('/dashboard')
       })
     }
   }

app/src/resources/runs/useNotifyAllRunsQuery.ts

@@ -8,26 +8,36 @@ import type { HostConfig, GetRunsParams, Runs } from '@opentrons/api-client'
 import type { UseAllRunsQueryOptions } from '@opentrons/react-api-client/src/runs/useAllRunsQuery'
 import type { QueryOptionsWithPolling } from '../useNotifyDataReady'
 
+// TODO(jh, 08-21-24): Abstract harder.
 export function useNotifyAllRunsQuery(
   params: GetRunsParams = {},
   options: QueryOptionsWithPolling<UseAllRunsQueryOptions, AxiosError> = {},
   hostOverride?: HostConfig | null
 ): UseQueryResult<Runs, AxiosError> {
-  const { notifyOnSettled, shouldRefetch } = useNotifyDataReady({
+  const {
+    notifyOnSettled,
+    shouldRefetch,
+    isNotifyEnabled,
+  } = useNotifyDataReady({
     topic: 'robot-server/runs',
     options,
     hostOverride,
   })
 
+  const queryOptions = {
+    ...options,
+    onSettled: isNotifyEnabled ? notifyOnSettled : options.onSettled,
+    refetchInterval: isNotifyEnabled ? false : options.refetchInterval,
+  }
   const httpResponse = useAllRunsQuery(
     params,
-    {
-      ...(options as UseAllRunsQueryOptions),
-      enabled: options?.enabled !== false && shouldRefetch,
-      onSettled: notifyOnSettled,
-    },
+    queryOptions as UseAllRunsQueryOptions,
     hostOverride
   )
 
+  if (isNotifyEnabled && shouldRefetch) {
+    httpResponse.refetch()
+  }
+
   return httpResponse
 }

app/src/resources/runs/useNotifyRunQuery.ts

@@ -7,28 +7,32 @@ import type { Run, HostConfig } from '@opentrons/api-client'
 import type { QueryOptionsWithPolling } from '../useNotifyDataReady'
 import type { NotifyTopic } from '../../redux/shell/types'
 
+// TODO(jh, 08-21-24): Abstract harder.
 export function useNotifyRunQuery<TError = Error>(
   runId: string | null,
   options: QueryOptionsWithPolling<Run, TError> = {},
   hostOverride?: HostConfig | null
 ): UseQueryResult<Run, TError> {
-  const isEnabled = options.enabled !== false && runId != null
-
-  const { notifyOnSettled, shouldRefetch } = useNotifyDataReady({
+  const {
+    notifyOnSettled,
+    shouldRefetch,
+    isNotifyEnabled,
+  } = useNotifyDataReady({
     topic: `robot-server/runs/${runId}` as NotifyTopic,
-    options: { ...options, enabled: options.enabled != null && runId != null },
+    options,
     hostOverride,
   })
 
-  const httpResponse = useRunQuery(
-    runId,
-    {
-      ...options,
-      enabled: isEnabled && shouldRefetch,
-      onSettled: notifyOnSettled,
-    },
-    hostOverride
-  )
+  const queryOptions = {
+    ...options,
+    onSettled: isNotifyEnabled ? notifyOnSettled : options.onSettled,
+    refetchInterval: isNotifyEnabled ? false : options.refetchInterval,
+  }
+  const httpResponse = useRunQuery(runId, queryOptions, hostOverride)
+
+  if (isNotifyEnabled && shouldRefetch) {
+    httpResponse.refetch()
+  }
 
   return httpResponse
 }

app/src/resources/useNotifyDataReady.ts

@@ -16,7 +16,7 @@ import type { UseQueryOptions } from 'react-query'
 import type { HostConfig } from '@opentrons/api-client'
 import type { NotifyTopic, NotifyResponseData } from '../redux/shell/types'
 
-export type HTTPRefetchFrequency = 'once' | 'always' | null
+export type HTTPRefetchFrequency = 'once' | null
 
 export interface QueryOptionsWithPolling<TData, TError = Error>
   extends UseQueryOptions<TData, TError> {
@@ -30,10 +30,20 @@ interface useNotifyDataReadyProps<TData, TError = Error> {
 }
 
 interface useNotifyDataReadyResults {
+  /* Reset notification refetch state. */
   notifyOnSettled: () => void
+  /* Whether notifications indicate the server has new data ready. */
   shouldRefetch: boolean
+  /* Whether notifications are enabled as determined by client options and notification health. */
+  isNotifyEnabled: boolean
 }
 
+// React query hooks perform refetches when instructed by the shell via a refetch mechanism, which useNotifyDataReady manages.
+// The notification refetch states may be:
+// 'once' - The shell has received an MQTT update. Execute the HTTP refetch once.
+// null - The shell has not received an MQTT update. Don't execute an HTTP refetch.
+//
+// Eagerly assume notifications are enabled unless specified by the client via React Query options or by the shell via errors.
 export function useNotifyDataReady<TData, TError = Error>({
   topic,
   options,
@@ -47,6 +57,7 @@ export function useNotifyDataReady<TData, TError = Error>({
   const forcePollingFF = useFeatureFlag('forceHttpPolling')
   const seenHostname = React.useRef<string | null>(null)
   const [refetch, setRefetch] = React.useState<HTTPRefetchFrequency>(null)
+  const [isNotifyEnabled, setIsNotifyEnabled] = React.useState(true)
 
   const { enabled, staleTime, forceHttpPolling } = options
 
@@ -69,7 +80,7 @@ export function useNotifyDataReady<TData, TError = Error>({
       dispatch(notifySubscribeAction(hostname, topic))
       seenHostname.current = hostname
     } else {
-      setRefetch('always')
+      setIsNotifyEnabled(false)
     }
 
     return () => {
@@ -86,7 +97,7 @@ export function useNotifyDataReady<TData, TError = Error>({
 
   const onDataEvent = React.useCallback((data: NotifyResponseData): void => {
     if (data === 'ECONNFAILED' || data === 'ECONNREFUSED') {
-      setRefetch('always')
+      setIsNotifyEnabled(false)
       if (data === 'ECONNREFUSED') {
         doTrackEvent({
           name: ANALYTICS_NOTIFICATION_PORT_BLOCK_ERROR,
@@ -104,5 +115,9 @@ export function useNotifyDataReady<TData, TError = Error>({
     }
   }, [refetch])
 
-  return { notifyOnSettled, shouldRefetch: refetch != null }
+  return {
+    notifyOnSettled,
+    shouldRefetch: refetch != null,
+    isNotifyEnabled,
+  }
 }