From 7073a1c479a0707ec1b0b42030387d888968c050 Mon Sep 17 00:00:00 2001
From: Jamey Huffnagle <jamey.huffnagle@opentrons.com>
Date: Mon, 12 Feb 2024 14:38:12 -0500
Subject: [PATCH] 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.
---
 app-shell-odd/src/notify.ts | 48 +++++++++++++++++++++----------------
 app-shell/src/notify.ts     | 48 +++++++++++++++++++++----------------
 2 files changed, 56 insertions(+), 40 deletions(-)

diff --git a/app-shell-odd/src/notify.ts b/app-shell-odd/src/notify.ts
index 69aea75e39b..1c7976d5cbb 100644
--- 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 = 250
+
 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
index 822dcebd084..436f5ed2935 100644
--- 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 = 250
+
 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}`