fix(app): Fix improper transitioning run status in protocol runs (#14459

)

Closes RQA-2291, RQA-2307, RQA-2306, RQA-2304

* fix(app): fix non polling notify hooks not always refetching data appropriately

Instead of checking the refetchInterval property to see if a notification refetch should occur, we
should check if staleTime is infinity. This accurately captures the refetchHTTP behavior that we
actually want.

* fix(app): fix infinite cancelling run state when run status is idle->stop-requested

app/src/organisms/RunTimeControl/hooks.ts

@@ -77,9 +77,11 @@ export function useRunStatus(
     refetchInterval: DEFAULT_STATUS_REFETCH_INTERVAL,
     enabled:
       lastRunStatus.current == null ||
-      !([RUN_STATUS_FAILED, RUN_STATUS_SUCCEEDED] as RunStatus[]).includes(
-        lastRunStatus.current
-      ),
+      !([
+        RUN_STATUS_FAILED,
+        RUN_STATUS_SUCCEEDED,
+        RUN_STATUS_STOP_REQUESTED,
+      ] as RunStatus[]).includes(lastRunStatus.current),
     onSuccess: data => (lastRunStatus.current = data?.data?.status ?? null),
     ...options,
   })

app/src/resources/runs/useNotifyRunQuery.ts

@@ -19,7 +19,7 @@ export function useNotifyRunQuery<TError = Error>(
   const { isNotifyError } = useNotifyService({
     topic: `robot-server/runs/${runId}` as NotifyTopic,
     refetchUsingHTTP: () => setRefetchUsingHTTP(true),
-    options,
+    options: { ...options, enabled: options.enabled && runId != null },
   })
 
   const isNotifyEnabled = !isNotifyError && !options?.forceHttpPolling

app/src/resources/useNotifyService.ts

@@ -43,33 +43,37 @@ export function useNotifyService<TData, TError = Error>({
   const host = useHost()
   const isNotifyError = React.useRef(false)
   const doTrackEvent = useTrackEvent()
-  const { enabled, refetchInterval, forceHttpPolling } = options
-  const isRefetchEnabled =
-    refetchInterval !== undefined && refetchInterval !== false
+  const { enabled, staleTime, forceHttpPolling } = options
+  const hostname = host?.hostname ?? null
 
   React.useEffect(() => {
     // Always fetch on initial mount.
     refetchUsingHTTP()
-    if (!forceHttpPolling && isRefetchEnabled && enabled !== false) {
-      const hostname = host?.hostname ?? null
+    if (
+      !forceHttpPolling &&
+      enabled !== false &&
+      hostname != null &&
+      staleTime !== Infinity
+    ) {
       const eventEmitter = appShellListener(hostname, topic)
-
       eventEmitter.on('data', onDataListener)
-
-      if (hostname != null) {
-        dispatch(notifySubscribeAction(hostname, topic))
-      } else {
-        console.error('NotifyService expected hostname, received null.')
-      }
+      dispatch(notifySubscribeAction(hostname, topic))
 
       return () => {
         eventEmitter.off('data', onDataListener)
         if (hostname != null) {
           dispatch(notifyUnsubscribeAction(hostname, topic))
         }
       }
+    } else {
+      if (hostname == null) {
+        console.error(
+          'NotifyService expected hostname, received null for topic:',
+          topic
+        )
+      }
     }
-  }, [topic])
+  }, [topic, host])
 
   return { isNotifyError: isNotifyError.current }
 

react-api-client/src/runs/useAllRunsQuery.ts

@@ -13,6 +13,10 @@ export type UseAllRunsQueryOptions = UseQueryOptions<
   Array<string | HostConfig>
 >
 
+/**
+ * @property {HostConfig | null | undefined} hostOverride:
+ * When using all runs query outside of the host context provider, we must specify the host manually.
+ */
 export function useAllRunsQuery(
   params: GetRunsParams = {},
   options: UseAllRunsQueryOptions = {},

robot-server/robot_server/runs/run_data_manager.py

@@ -112,6 +112,11 @@ async def create(
             EngineConflictError: There is a currently active run that cannot
                 be superceded by this new run.
         """
+        await self._runs_publisher.begin_polling_engine_store(
+            get_current_command=self.get_current_command,
+            get_state_summary=self._get_state_summary,
+            run_id=run_id,
+        )
         prev_run_id = self._engine_store.current_run_id
         if prev_run_id is not None:
             prev_run_result = await self._engine_store.clear()
@@ -120,7 +125,6 @@ async def create(
                 summary=prev_run_result.state_summary,
                 commands=prev_run_result.commands,
             )
-
         state_summary = await self._engine_store.create(
             run_id=run_id,
             labware_offsets=labware_offsets,
@@ -132,11 +136,6 @@ async def create(
             created_at=created_at,
             protocol_id=protocol.protocol_id if protocol is not None else None,
         )
-        await self._runs_publisher.begin_polling_engine_store(
-            get_current_command=self.get_current_command,
-            get_state_summary=self._get_state_summary,
-            run_id=run_id,
-        )
 
         return _build_run(
             run_resource=run_resource,

robot-server/robot_server/runs/run_store.py

@@ -411,6 +411,7 @@ def remove(self, run_id: str) -> None:
             raise RunNotFoundError(run_id)
 
         self._clear_caches()
+        self._runs_publisher.publish_runs(run_id=run_id)
 
     def _run_exists(
         self, run_id: str, connection: sqlalchemy.engine.Connection

robot-server/robot_server/service/notifications/publishers/runs_publisher.py

@@ -22,6 +22,7 @@ def __init__(self, client: NotificationClient) -> None:
         self._run_data_manager_polling = asyncio.Event()
         self._previous_current_command: Union[CurrentCommand, None] = None
         self._previous_state_summary_status: Union[EngineStatus, None] = None
+        self._poller: Optional[asyncio.Task[None]] = None
 
     # TODO(jh, 2023-02-02): Instead of polling, emit current_commands directly from PE.
     async def begin_polling_engine_store(
@@ -36,18 +37,22 @@ async def begin_polling_engine_store(
             current_command: The currently executing command, if any.
             run_id: ID of the current run.
         """
-        asyncio.create_task(
-            self._poll_engine_store(
-                get_current_command=get_current_command,
-                run_id=run_id,
-                get_state_summary=get_state_summary,
+        if self._poller is None:
+            self._poller = asyncio.create_task(
+                self._poll_engine_store(
+                    get_current_command=get_current_command,
+                    run_id=run_id,
+                    get_state_summary=get_state_summary,
+                )
             )
-        )
 
     async def stop_polling_engine_store(self) -> None:
         """Stops polling the engine store."""
-        self._run_data_manager_polling.set()
-        await self._client.publish_async(topic=Topics.RUNS_CURRENT_COMMAND.value)
+        if self._poller is not None:
+            self._run_data_manager_polling.set()
+            await self._client.publish_async(topic=Topics.RUNS_CURRENT_COMMAND.value)
+            self._poller.cancel()
+            self._poller = None
 
     def publish_runs(self, run_id: str) -> None:
         """Publishes the equivalent of GET /runs and GET /runs/:runId.