fix(app-shell, app-shell-odd): Fix intermittently dropped notificatio…

…ns (#14477) Closes RQA-2339, RQA-2321, RQA-2319, RAUT-962, RQA-2346 * fix(app-shell, app-shell-odd): fix intermittently dropped notifications Because subscription logic is directly tied to the component lifecycle, it is possible for a component to trigger an unsubscribe event on dismount while a new component mounts and triggers a subscribe event. For the connection store and MQTT to reflect correct topic subscriptions, do not unsubscribe and close connections before newly mounted components have had time to update the connection store.
Opentrons · Feb 16, 2024 · 0f76d33 · 0f76d33
1 parent 9c00eb0
commit 0f76d33
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 40 deletions.
diff --git a/app-shell-odd/src/notify.ts b/app-shell-odd/src/notify.ts
@@ -184,30 +184,38 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
   }
 }
 
+// Because subscription logic is directly tied to the component lifecycle, it is possible
+// for a component to trigger an unsubscribe event on dismount while a new component mounts and
+// triggers a subscribe event. For the connection store and MQTT to reflect correct topic subscriptions,
+// do not unsubscribe and close connections before newly mounted components have had time to update the connection store.
+const RENDER_TIMEOUT = 10000 // 10 seconds
+
 function unsubscribe(notifyParams: NotifyParams): Promise<void> {
   const { hostname, topic } = notifyParams
   return new Promise<void>(() => {
     if (hostname in connectionStore) {
-      const { client } = connectionStore[hostname]
-      const subscriptions = connectionStore[hostname]?.subscriptions
-      const isLastSubscription = subscriptions[topic] <= 1
-
-      if (isLastSubscription) {
-        client?.unsubscribe(topic, {}, (error, result) => {
-          if (error != null) {
-            log.warn(
-              `Failed to unsubscribe on ${hostname} from topic: ${topic}`
-            )
-          } else {
-            log.info(
-              `Successfully unsubscribed on ${hostname} from topic: ${topic}`
-            )
-            handleDecrementSubscriptionCount(hostname, topic)
-          }
-        })
-      } else {
-        subscriptions[topic] -= 1
-      }
+      setTimeout(() => {
+        const { client } = connectionStore[hostname]
+        const subscriptions = connectionStore[hostname]?.subscriptions
+        const isLastSubscription = subscriptions[topic] <= 1
+
+        if (isLastSubscription) {
+          client?.unsubscribe(topic, {}, (error, result) => {
+            if (error != null) {
+              log.warn(
+                `Failed to unsubscribe on ${hostname} from topic: ${topic}`
+              )
+            } else {
+              log.info(
+                `Successfully unsubscribed on ${hostname} from topic: ${topic}`
+              )
+              handleDecrementSubscriptionCount(hostname, topic)
+            }
+          })
+        } else {
+          subscriptions[topic] -= 1
+        }
+      }, RENDER_TIMEOUT)
     } else {
       log.info(
         `Attempted to unsubscribe from unconnected hostname: ${hostname}`

diff --git a/app-shell/src/notify.ts b/app-shell/src/notify.ts
@@ -180,30 +180,38 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
   }
 }
 
+// Because subscription logic is directly tied to the component lifecycle, it is possible
+// for a component to trigger an unsubscribe event on dismount while a new component mounts and
+// triggers a subscribe event. For the connection store and MQTT to reflect correct topic subscriptions,
+// do not unsubscribe and close connections before newly mounted components have had time to update the connection store.
+const RENDER_TIMEOUT = 10000 // 10 seconds
+
 function unsubscribe(notifyParams: NotifyParams): Promise<void> {
   const { hostname, topic } = notifyParams
   return new Promise<void>(() => {
     if (hostname in connectionStore) {
-      const { client } = connectionStore[hostname]
-      const subscriptions = connectionStore[hostname]?.subscriptions
-      const isLastSubscription = subscriptions[topic] <= 1
-
-      if (isLastSubscription) {
-        client?.unsubscribe(topic, {}, (error, result) => {
-          if (error != null) {
-            log.warn(
-              `Failed to unsubscribe on ${hostname} from topic: ${topic}`
-            )
-          } else {
-            log.info(
-              `Successfully unsubscribed on ${hostname} from topic: ${topic}`
-            )
-            handleDecrementSubscriptionCount(hostname, topic)
-          }
-        })
-      } else {
-        subscriptions[topic] -= 1
-      }
+      setTimeout(() => {
+        const { client } = connectionStore[hostname]
+        const subscriptions = connectionStore[hostname]?.subscriptions
+        const isLastSubscription = subscriptions[topic] <= 1
+
+        if (isLastSubscription) {
+          client?.unsubscribe(topic, {}, (error, result) => {
+            if (error != null) {
+              log.warn(
+                `Failed to unsubscribe on ${hostname} from topic: ${topic}`
+              )
+            } else {
+              log.info(
+                `Successfully unsubscribed on ${hostname} from topic: ${topic}`
+              )
+              handleDecrementSubscriptionCount(hostname, topic)
+            }
+          })
+        } else {
+          subscriptions[topic] -= 1
+        }
+      }, RENDER_TIMEOUT)
     } else {
       log.info(
         `Attempted to unsubscribe from unconnected hostname: ${hostname}`