chore(extension): make sure to log errors in crucial places (#656)

* make sure to log errors in crucial places * make sure new callbacks are called in specs * update types for test helpers
gnosisguild · Jan 27, 2025 · fa0b395 · fa0b395
1 parent 359ddac
commit fa0b395
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 22 deletions.
diff --git a/deployables/extension/src/background/cspHeaderRule.ts b/deployables/extension/src/background/cspHeaderRule.ts
@@ -2,6 +2,8 @@
 // (Unfortunately, redirect targets are subject to the page's CSPs.)
 // So we keep declarativeNetRequest rule in sync wth activeExtensionTabs.
 
+import { captureLastError } from '@/sentry'
+
 // This rule removes CSP headers for extension tabs and must be kept in sync with activeExtensionTabs.
 export const REMOVE_CSP_RULE_ID = 1
 
@@ -11,6 +13,8 @@ export const removeCSPHeaderRule = () => {
       removeRuleIds: [REMOVE_CSP_RULE_ID],
     },
     () => {
+      captureLastError()
+
       if (chrome.runtime.lastError) {
         console.error('CSP rule could not be removed', chrome.runtime.lastError)
       } else {
@@ -61,6 +65,8 @@ export const updateCSPHeaderRule = (tabIds: Set<number>) => {
       removeRuleIds: [REMOVE_CSP_RULE_ID],
     },
     () => {
+      captureLastError()
+
       if (chrome.runtime.lastError) {
         console.error('Headers rule update failed', chrome.runtime.lastError)
       } else {

diff --git a/deployables/extension/src/background/rpcRedirect.ts b/deployables/extension/src/background/rpcRedirect.ts
@@ -1,14 +1,26 @@
+import { captureLastError } from '@/sentry'
 import { REMOVE_CSP_RULE_ID } from './cspHeaderRule'
 import type { Fork } from './types'
 
 export const removeAllRpcRedirectRules = async (tabIds: number[]) => {
-  await chrome.declarativeNetRequest.updateSessionRules({
-    removeRuleIds: tabIds,
-  })
+  const { promise, resolve } = Promise.withResolvers<void>()
 
-  chrome.declarativeNetRequest.getSessionRules((rules) => {
-    console.debug('RPC redirect rules updated', rules)
-  })
+  chrome.declarativeNetRequest.updateSessionRules(
+    {
+      removeRuleIds: tabIds,
+    },
+    () => {
+      captureLastError()
+
+      chrome.declarativeNetRequest.getSessionRules((rules) => {
+        console.debug('RPC redirect rules updated', rules)
+
+        resolve()
+      })
+    },
+  )
+
+  return promise
 }
 
 /**
@@ -45,15 +57,24 @@ export const addRpcRedirectRules = async (
       },
     }))
 
-  await chrome.declarativeNetRequest.updateSessionRules({
-    addRules,
-  })
+  const { promise, resolve } = Promise.withResolvers()
 
-  chrome.declarativeNetRequest.getSessionRules((rules) => {
-    console.debug('RPC redirect rules updated', rules)
-  })
+  chrome.declarativeNetRequest.updateSessionRules(
+    {
+      addRules,
+    },
+    () => {
+      captureLastError()
+
+      chrome.declarativeNetRequest.getSessionRules((rules) => {
+        console.debug('RPC redirect rules updated', rules)
+      })
+
+      resolve(new Set(addRules.map(({ id }) => id)))
+    },
+  )
 
-  return new Set(addRules.map(({ id }) => id))
+  return promise
 }
 
 /**

diff --git a/deployables/extension/src/background/simulationTracking.spec.ts b/deployables/extension/src/background/simulationTracking.spec.ts
@@ -51,6 +51,7 @@ describe('Simulation tracking', () => {
             },
           ],
         }),
+        expect.anything(),
       )
     })
 
@@ -64,9 +65,12 @@ describe('Simulation tracking', () => {
 
       expect(
         chromeMock.declarativeNetRequest.updateSessionRules,
-      ).toHaveBeenLastCalledWith({
-        removeRuleIds: [2],
-      })
+      ).toHaveBeenLastCalledWith(
+        {
+          removeRuleIds: [2],
+        },
+        expect.anything(),
+      )
     })
 
     it('removes redirect rules when the pilot session ends', async () => {
@@ -82,9 +86,12 @@ describe('Simulation tracking', () => {
 
       expect(
         chromeMock.declarativeNetRequest.updateSessionRules,
-      ).toHaveBeenLastCalledWith({
-        removeRuleIds: [2],
-      })
+      ).toHaveBeenLastCalledWith(
+        {
+          removeRuleIds: [2],
+        },
+        expect.anything(),
+      )
     })
 
     it('updates the redirect rules when a new RPC endpoint is detected during simulation', async () => {
@@ -115,6 +122,7 @@ describe('Simulation tracking', () => {
             },
           ],
         }),
+        expect.anything(),
       )
     })
 
@@ -148,6 +156,7 @@ describe('Simulation tracking', () => {
             },
           ],
         }),
+        expect.anything(),
       )
     })
 

diff --git a/deployables/extension/src/companion/contentScripts/main.ts b/deployables/extension/src/companion/contentScripts/main.ts
@@ -1,3 +1,4 @@
+import { captureLastError } from '@/sentry'
 import { injectScript } from '@/utils'
 import {
   CompanionAppMessageType,
@@ -14,7 +15,9 @@ window.addEventListener(
       return
     }
 
-    chrome.runtime.sendMessage(event.data)
+    chrome.runtime.sendMessage(event.data, () => {
+      captureLastError()
+    })
   },
 )
 

diff --git a/deployables/extension/src/panel/port-handling/createPort.ts b/deployables/extension/src/panel/port-handling/createPort.ts
@@ -1,3 +1,4 @@
+import { captureLastError } from '@/sentry'
 import { isValidTab } from '@/utils'
 import {
   type ConnectedWalletMessage,
@@ -47,6 +48,8 @@ const connectToDApp = (tabId: number): Promise<chrome.runtime.Port> => {
 
   const port = chrome.tabs.connect(tabId)
 
+  captureLastError()
+
   const timeout = setTimeout(() => {
     port.disconnect()
 

diff --git a/deployables/extension/src/panel/port-handling/usePilotPort.ts b/deployables/extension/src/panel/port-handling/usePilotPort.ts
@@ -1,3 +1,4 @@
+import { captureLastError } from '@/sentry'
 import { isValidTab, useActiveTab } from '@/utils'
 import { type Message, PilotMessageType } from '@zodiac/messages'
 import { useEffect, useState } from 'react'
@@ -74,11 +75,15 @@ type ConnectPortOptions = {
 const connectPort = ({ tabId, windowId }: ConnectPortOptions) => {
   const port = chrome.runtime.connect({ name: PILOT_PANEL_PORT })
 
+  captureLastError()
+
   port.postMessage({
     type: PilotMessageType.PILOT_PANEL_OPENED,
     windowId,
     tabId,
   } satisfies Message)
 
+  captureLastError()
+
   return port
 }
diff --git a/deployables/extension/src/sentry/captureLastError.ts b/deployables/extension/src/sentry/captureLastError.ts
@@ -0,0 +1,9 @@
+import { client } from './client'
+
+export const captureLastError = () => {
+  if (chrome.runtime.lastError == null) {
+    return
+  }
+
+  client.captureException(chrome.runtime.lastError)
+}
diff --git a/deployables/extension/src/sentry/index.ts b/deployables/extension/src/sentry/index.ts
@@ -1,2 +1,3 @@
+export { captureLastError } from './captureLastError'
 export { client as sentry } from './client'
 export { SentryErrorBoundary } from './SentryErrorBoundary'
diff --git a/packages/test-utils/src/chrome/chromeMock.ts b/packages/test-utils/src/chrome/chromeMock.ts
@@ -77,8 +77,26 @@ const vitestChromeWithMissingMethods = Object.assign(vitestChrome, {
     HeaderOperation,
     ResourceType,
 
-    updateSessionRules: vi.fn(),
-    getSessionRules: vi.fn(),
+    updateSessionRules: vi.fn((_, callback: unknown) => {
+      if (typeof callback === 'function') {
+        callback()
+
+        return
+      }
+
+      return Promise.resolve()
+    }),
+    getSessionRules: vi.fn(
+      (callback): void | Promise<chrome.declarativeNetRequest.Rule[]> => {
+        if (typeof callback === 'function') {
+          callback()
+
+          return
+        }
+
+        return Promise.resolve([])
+      },
+    ),
   },
 
   scripting: {