From ba5ba2230e4184660d276630784b49ef9c50d236 Mon Sep 17 00:00:00 2001
From: Jamey Huffnagle <jamey.huffnagle@opentrons.com>
Date: Mon, 26 Feb 2024 14:08:00 -0500
Subject: [PATCH] fix(app-shell, app-shell-odd): reduce port block analytics
 reporting frequency

Instead of reporting every port block event, let's just report one event per session of the desktop
app/odd.
---
 app-shell-odd/src/notify.ts | 24 ++++++++++++++++++------
 app-shell/src/notify.ts     | 24 ++++++++++++++++++------
 2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/app-shell-odd/src/notify.ts b/app-shell-odd/src/notify.ts
index 77ad4e170fd..7ff2f73b22f 100644
--- a/app-shell-odd/src/notify.ts
+++ b/app-shell-odd/src/notify.ts
@@ -15,6 +15,11 @@ import type { Action, Dispatch } from './types'
 // However, redundant subscriptions are permitted and result in the broker sending the retained message for that topic.
 // To mitigate redundant connections, the connection manager eagerly adds the host, removing the host if the connection fails.
 
+const FAILURE_STATUSES = {
+  ECONNREFUSED: 'ECONNREFUSED',
+  ECONNFAILED: 'ECONNFAILED',
+} as const
+
 interface ConnectionStore {
   [hostname: string]: {
     client: mqtt.MqttClient | null
@@ -72,6 +77,7 @@ export function registerNotify(
 }
 
 const CHECK_CONNECTION_INTERVAL = 500
+let hasReportedAPortBlockEvent = false
 
 interface NotifyParams {
   browserWindow: BrowserWindow
@@ -101,9 +107,15 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
         log.warn(
           `Failed to connect to ${hostname} - ${error.name}: ${error.message} `
         )
-        const failureMessage = error.message.includes('ECONNREFUSED')
-          ? 'ECONNREFUSED'
-          : 'ECONNFAILED'
+
+        let failureMessage: string = FAILURE_STATUSES.ECONNFAILED
+        if (
+          error.message.includes(FAILURE_STATUSES.ECONNREFUSED) &&
+          !hasReportedAPortBlockEvent
+        ) {
+          failureMessage = FAILURE_STATUSES.ECONNREFUSED
+          hasReportedAPortBlockEvent = true
+        }
 
         sendToBrowserDeserialized({
           browserWindow,
@@ -128,7 +140,7 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
         browserWindow,
         hostname,
         topic,
-        message: 'ECONNFAILED',
+        message: FAILURE_STATUSES.ECONNFAILED,
       })
       handleDecrementSubscriptionCount(hostname, topic)
     })
@@ -141,7 +153,7 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
         browserWindow,
         hostname,
         topic,
-        message: 'ECONNFAILED',
+        message: FAILURE_STATUSES.ECONNFAILED,
       })
       handleDecrementSubscriptionCount(hostname, topic)
     } else {
@@ -312,7 +324,7 @@ function establishListeners({
       browserWindow,
       hostname,
       topic,
-      message: 'ECONNFAILED',
+      message: FAILURE_STATUSES.ECONNFAILED,
     })
     client.end()
   })
diff --git a/app-shell/src/notify.ts b/app-shell/src/notify.ts
index 33f1fcd8ff4..9ec4c04e62b 100644
--- a/app-shell/src/notify.ts
+++ b/app-shell/src/notify.ts
@@ -13,6 +13,11 @@ import type { Action, Dispatch } from './types'
 // However, redundant subscriptions are permitted and result in the broker sending the retained message for that topic.
 // To mitigate redundant connections, the connection manager eagerly adds the host, removing the host if the connection fails.
 
+const FAILURE_STATUSES = {
+  ECONNREFUSED: 'ECONNREFUSED',
+  ECONNFAILED: 'ECONNFAILED',
+} as const
+
 interface ConnectionStore {
   [hostname: string]: {
     client: mqtt.MqttClient | null
@@ -68,6 +73,7 @@ export function registerNotify(
 }
 
 const CHECK_CONNECTION_INTERVAL = 500
+let hasReportedAPortBlockEvent = false
 
 interface NotifyParams {
   browserWindow: BrowserWindow
@@ -97,9 +103,15 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
         log.warn(
           `Failed to connect to ${hostname} - ${error.name}: ${error.message} `
         )
-        const failureMessage = error.message.includes('ECONNREFUSED')
-          ? 'ECONNREFUSED'
-          : 'ECONNFAILED'
+
+        let failureMessage: string = FAILURE_STATUSES.ECONNFAILED
+        if (
+          error.message.includes(FAILURE_STATUSES.ECONNREFUSED) &&
+          !hasReportedAPortBlockEvent
+        ) {
+          failureMessage = FAILURE_STATUSES.ECONNREFUSED
+          hasReportedAPortBlockEvent = true
+        }
 
         sendToBrowserDeserialized({
           browserWindow,
@@ -124,7 +136,7 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
         browserWindow,
         hostname,
         topic,
-        message: 'ECONNFAILED',
+        message: FAILURE_STATUSES.ECONNFAILED,
       })
       handleDecrementSubscriptionCount(hostname, topic)
     })
@@ -137,7 +149,7 @@ function subscribe(notifyParams: NotifyParams): Promise<void> {
         browserWindow,
         hostname,
         topic,
-        message: 'ECONNFAILED',
+        message: FAILURE_STATUSES.ECONNFAILED,
       })
       handleDecrementSubscriptionCount(hostname, topic)
     } else {
@@ -308,7 +320,7 @@ function establishListeners({
       browserWindow,
       hostname,
       topic,
-      message: 'ECONNFAILED',
+      message: FAILURE_STATUSES.ECONNFAILED,
     })
     client.end()
   })