fix(app): fix browser layer notification error handling (#16207)

Closes RQA-3108 (for the third time) Fixes an issue in which browser layer error messages were dropped. The app shell sends MQTT error event messages on the ALL_TOPICS topic, but browser layer callbacks do not ever subscribe to the ALL_TOPICS topic, just their individual topic. This means when the app receives a disconnect event that is applicable to all topics, those topics do not default back to polling.
Opentrons · Sep 6, 2024 · 6948d93 · 6948d93
1 parent 2878ab3
commit 6948d93
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 19 deletions.
diff --git a/app/src/redux/shell/remote.ts b/app/src/redux/shell/remote.ts
@@ -73,37 +73,42 @@ const callbackStore: CallbackStore = {}
 
 interface AppShellListener {
   hostname: string
-  topic: NotifyTopic
+  notifyTopic: NotifyTopic
   callback: (data: NotifyResponseData) => void
   isDismounting?: boolean
 }
 export function appShellListener({
   hostname,
-  topic,
+  notifyTopic,
   callback,
   isDismounting = false,
 }: AppShellListener): CallbackStore {
-  if (isDismounting) {
-    const callbacks = callbackStore[hostname]?.[topic]
-    if (callbacks != null) {
-      callbackStore[hostname][topic] = callbacks.filter(cb => cb !== callback)
-      if (!callbackStore[hostname][topic].length) {
-        // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
-        delete callbackStore[hostname][topic]
-        if (!Object.keys(callbackStore[hostname]).length) {
+  // The shell emits general messages to ALL_TOPICS, typically errors, and all listeners must handle those messages.
+  const topics: NotifyTopic[] = [notifyTopic, 'ALL_TOPICS'] as const
+
+  topics.forEach(topic => {
+    if (isDismounting) {
+      const callbacks = callbackStore[hostname]?.[topic]
+      if (callbacks != null) {
+        callbackStore[hostname][topic] = callbacks.filter(cb => cb !== callback)
+        if (!callbackStore[hostname][topic].length) {
           // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
-          delete callbackStore[hostname]
+          delete callbackStore[hostname][topic]
+          if (!Object.keys(callbackStore[hostname]).length) {
+            // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
+            delete callbackStore[hostname]
+          }
         }
       }
+    } else {
+      callbackStore[hostname] = callbackStore[hostname] ?? {}
+      callbackStore[hostname][topic] ??= []
+      callbackStore[hostname][topic].push(callback)
     }
-  } else {
-    callbackStore[hostname] = callbackStore[hostname] ?? {}
-    callbackStore[hostname][topic] ??= []
-    callbackStore[hostname][topic].push(callback)
-  }
+  })
+
   return callbackStore
 }
-
 // Instantiate the notify listener at runtime.
 remote.ipcRenderer.on(
   'notify',

diff --git a/app/src/resources/useNotifyDataReady.ts b/app/src/resources/useNotifyDataReady.ts
@@ -74,7 +74,7 @@ export function useNotifyDataReady<TData, TError = Error>({
       setRefetch('once')
       appShellListener({
         hostname,
-        topic,
+        notifyTopic: topic,
         callback: onDataEvent,
       })
       dispatch(notifySubscribeAction(hostname, topic))
@@ -87,7 +87,7 @@ export function useNotifyDataReady<TData, TError = Error>({
       if (seenHostname.current != null) {
         appShellListener({
           hostname: seenHostname.current,
-          topic,
+          notifyTopic: topic,
           callback: onDataEvent,
           isDismounting: true,
         })