Capture Sentry errors prior to initialization (#20265)

* Capture Sentry errors prior to initialization

Sentry errors captured before/during the wallet initialization are
currently not captured because we don't have the controller state yet
to determine whether the user has consented.

The Sentry setup has been updated to check the persisted state for
whether the user has consented, as a fallback in case the controller
state hasn't been initialized yet. This ensures that we capture errors
during initialization if the user has opted in.

* Always await async check for whether the user has opted in

* Remove unused import

* Update JSDoc return type

* Remove unused driver method

* Fix metametrics controller unit tests

* Fix e2e tests

* Fix e2e test on Firefox

* Start session upon install rather than toggle

app/scripts/background.js

@@ -2,6 +2,10 @@
  * @file The entry point for the web extension singleton process.
  */
 
+// This import sets up a global function required for Sentry to function.
+// It must be run first in case an error is thrown later during initialization.
+import './lib/setup-persisted-state-hook';
+
 import EventEmitter from 'events';
 import endOfStream from 'end-of-stream';
 import pump from 'pump';

app/scripts/controllers/metametrics.js

@@ -419,18 +419,18 @@ export default class MetaMetricsController {
    *
    * @param {boolean} participateInMetaMetrics - Whether or not the user wants
    *  to participate in MetaMetrics
-   * @returns {string|null} the string of the new metametrics id, or null
+   * @returns {Promise<string|null>} the string of the new metametrics id, or null
    *  if not set
    */
-  setParticipateInMetaMetrics(participateInMetaMetrics) {
+  async setParticipateInMetaMetrics(participateInMetaMetrics) {
     let { metaMetricsId } = this.state;
     if (participateInMetaMetrics && !metaMetricsId) {
       // We also need to start sentry automatic session tracking at this point
-      globalThis.sentry?.startSession();
+      await globalThis.sentry?.startSession();
       metaMetricsId = this.generateMetaMetricsId();
     } else if (participateInMetaMetrics === false) {
       // We also need to stop sentry automatic session tracking at this point
-      globalThis.sentry?.endSession();
+      await globalThis.sentry?.endSession();
       metaMetricsId = null;
     }
     this.store.updateState({ participateInMetaMetrics, metaMetricsId });

app/scripts/controllers/metametrics.test.js

@@ -313,30 +313,30 @@ describe('MetaMetricsController', function () {
   });
 
   describe('setParticipateInMetaMetrics', function () {
-    it('should update the value of participateInMetaMetrics', function () {
+    it('should update the value of participateInMetaMetrics', async function () {
       const metaMetricsController = getMetaMetricsController({
         participateInMetaMetrics: null,
         metaMetricsId: null,
       });
       assert.equal(metaMetricsController.state.participateInMetaMetrics, null);
-      metaMetricsController.setParticipateInMetaMetrics(true);
+      await metaMetricsController.setParticipateInMetaMetrics(true);
       assert.ok(globalThis.sentry.startSession.calledOnce);
       assert.equal(metaMetricsController.state.participateInMetaMetrics, true);
-      metaMetricsController.setParticipateInMetaMetrics(false);
+      await metaMetricsController.setParticipateInMetaMetrics(false);
       assert.equal(metaMetricsController.state.participateInMetaMetrics, false);
     });
-    it('should generate and update the metaMetricsId when set to true', function () {
+    it('should generate and update the metaMetricsId when set to true', async function () {
       const metaMetricsController = getMetaMetricsController({
         participateInMetaMetrics: null,
         metaMetricsId: null,
       });
       assert.equal(metaMetricsController.state.metaMetricsId, null);
-      metaMetricsController.setParticipateInMetaMetrics(true);
+      await metaMetricsController.setParticipateInMetaMetrics(true);
       assert.equal(typeof metaMetricsController.state.metaMetricsId, 'string');
     });
-    it('should nullify the metaMetricsId when set to false', function () {
+    it('should nullify the metaMetricsId when set to false', async function () {
       const metaMetricsController = getMetaMetricsController();
-      metaMetricsController.setParticipateInMetaMetrics(false);
+      await metaMetricsController.setParticipateInMetaMetrics(false);
       assert.ok(globalThis.sentry.endSession.calledOnce);
       assert.equal(metaMetricsController.state.metaMetricsId, null);
     });

app/scripts/lib/sentry-filter-events.ts

@@ -29,7 +29,7 @@ export class FilterEvents implements Integration {
    * @returns `true` if MetaMask's state has been initialized, and MetaMetrics
    * is enabled, `false` otherwise.
    */
-  private getMetaMetricsEnabled: () => boolean;
+  private getMetaMetricsEnabled: () => Promise<boolean>;
 
   /**
    * @param options - Constructor options.
@@ -40,7 +40,7 @@ export class FilterEvents implements Integration {
   constructor({
     getMetaMetricsEnabled,
   }: {
-    getMetaMetricsEnabled: () => boolean;
+    getMetaMetricsEnabled: () => Promise<boolean>;
   }) {
     this.getMetaMetricsEnabled = getMetaMetricsEnabled;
   }
@@ -56,13 +56,13 @@ export class FilterEvents implements Integration {
     addGlobalEventProcessor: (callback: EventProcessor) => void,
     getCurrentHub: () => Hub,
   ): void {
-    addGlobalEventProcessor((currentEvent: SentryEvent) => {
+    addGlobalEventProcessor(async (currentEvent: SentryEvent) => {
       // Sentry integrations use the Sentry hub to get "this" references, for
       // reasons I don't fully understand.
       // eslint-disable-next-line consistent-this
       const self = getCurrentHub().getIntegration(FilterEvents);
       if (self) {
-        if (!self.getMetaMetricsEnabled()) {
+        if (!(await self.getMetaMetricsEnabled())) {
           logger.warn(`Event dropped due to MetaMetrics setting.`);
           return null;
         }

app/scripts/lib/setup-persisted-state-hook.js

@@ -0,0 +1,10 @@
+import LocalStore from './local-store';
+import ReadOnlyNetworkStore from './network-store';
+
+const localStore = process.env.IN_TEST
+  ? new ReadOnlyNetworkStore()
+  : new LocalStore();
+
+globalThis.stateHooks.getPersistedState = async function () {
+  return await localStore.get();
+};

app/scripts/lib/setupSentry.js

@@ -118,16 +118,20 @@ export default function setupSentry({ release, getState }) {
    * @returns `true` if MetaMask's state has been initialized, and MetaMetrics
    * is enabled, `false` otherwise.
    */
-  function getMetaMetricsEnabled() {
+  async function getMetaMetricsEnabled() {
-    if (getState) {
+    const appState = getState();
-      const appState = getState();
+    if (Object.keys(appState) > 0) {
-      if (!appState?.store?.metamask?.participateInMetaMetrics) {
+      return Boolean(appState?.store?.metamask?.participateInMetaMetrics);
-        return false;
+    }
-      }
+    try {
-    } else {
+      const persistedState = await globalThis.stateHooks.getPersistedState();
+      return Boolean(
+        persistedState?.data?.MetaMetricsController?.participateInMetaMetrics,
+      );
+    } catch (error) {
+      console.error(error);
       return false;
     }
-    return true;
   }
 
   Sentry.init({
@@ -186,10 +190,10 @@ export default function setupSentry({ release, getState }) {
    * opted into MetaMetrics, change the autoSessionTracking option and start
    * a new sentry session.
    */
-  const startSession = () => {
+  const startSession = async () => {
     const hub = Sentry.getCurrentHub?.();
     const options = hub.getClient?.().getOptions?.() ?? {};
-    if (hub && getMetaMetricsEnabled() === true) {
+    if (hub && (await getMetaMetricsEnabled()) === true) {
       options.autoSessionTracking = true;
       hub.startSession();
     }
@@ -200,10 +204,10 @@ export default function setupSentry({ release, getState }) {
    * opted out of MetaMetrics, change the autoSessionTracking option and end
    * the current sentry session.
    */
-  const endSession = () => {
+  const endSession = async () => {
     const hub = Sentry.getCurrentHub?.();
     const options = hub.getClient?.().getOptions?.() ?? {};
-    if (hub && getMetaMetricsEnabled() === false) {
+    if (hub && (await getMetaMetricsEnabled()) === false) {
       options.autoSessionTracking = false;
       hub.endSession();
     }
@@ -214,22 +218,22 @@ export default function setupSentry({ release, getState }) {
    * on the state of metaMetrics optin and the state of autoSessionTracking on
    * the Sentry client.
    */
-  const toggleSession = () => {
+  const toggleSession = async () => {
     const hub = Sentry.getCurrentHub?.();
     const options = hub.getClient?.().getOptions?.() ?? {
       autoSessionTracking: false,
     };
-    const isMetaMetricsEnabled = getMetaMetricsEnabled();
+    const isMetaMetricsEnabled = await getMetaMetricsEnabled();
     if (
       isMetaMetricsEnabled === true &&
       options.autoSessionTracking === false
     ) {
-      startSession();
+      await startSession();
     } else if (
       isMetaMetricsEnabled === false &&
       options.autoSessionTracking === true
     ) {
-      endSession();
+      await endSession();
     }
   };
 

app/scripts/sentry-install.js

@@ -10,17 +10,16 @@ global.sentry = setupSentry({
 });
 
 /**
- * As soon as state is available via getSentryState we can call the
+ * As soon as state is available via getSentryState we can start automatic
- * toggleSession method added to sentry in setupSentry to start automatic
  * session tracking.
  */
-function waitForStateHooks() {
+async function waitForStateHooks() {
   if (global.stateHooks.getSentryState) {
     // sentry is not defined in dev mode, so if sentry doesn't exist at this
     // point it means that we are in dev mode and do not need to toggle the
     // session. Using optional chaining is sufficient to prevent the error in
     // development.
-    global.sentry?.toggleSession();
+    await global.sentry?.startSession();
   } else {
     setTimeout(waitForStateHooks, 100);
   }

app/scripts/ui.js

@@ -1,6 +1,10 @@
 // dev only, "react-devtools" import is skipped in prod builds
 import 'react-devtools';
 
+// This import sets up a global function required for Sentry to function.
+// It must be run first in case an error is thrown later during initialization.
+import './lib/setup-persisted-state-hook';
+
 import PortStream from 'extension-port-stream';
 import browser from 'webextension-polyfill';
 

test/e2e/tests/errors.spec.js

@@ -1,9 +1,26 @@
 const { strict: assert } = require('assert');
+const { Browser } = require('selenium-webdriver');
 const { convertToHexValue, withFixtures } = require('../helpers');
 const FixtureBuilder = require('../fixture-builder');
 
 describe('Sentry errors', function () {
-  async function mockSentry(mockServer) {
+  const migrationError =
+    process.env.SELENIUM_BROWSER === Browser.CHROME
+      ? `Cannot read properties of undefined (reading 'version')`
+      : 'meta is undefined';
+  async function mockSentryMigratorError(mockServer) {
+    return await mockServer
+      .forPost('https://sentry.io/api/0000000/envelope/')
+      .withBodyIncluding(migrationError)
+      .thenCallback(() => {
+        return {
+          statusCode: 200,
+          json: {},
+        };
+      });
+  }
+
+  async function mockSentryTestError(mockServer) {
     return await mockServer
       .forPost('https://sentry.io/api/0000000/envelope/')
       .withBodyIncluding('Test Error')
@@ -23,73 +40,149 @@ describe('Sentry errors', function () {
       },
     ],
   };
-  it('should NOT send error events when participateInMetaMetrics is false', async function () {
+
-    await withFixtures(
+  describe('before initialization', function () {
-      {
+    it('should NOT send error events when participateInMetaMetrics is false', async function () {
-        fixtures: new FixtureBuilder()
+      await withFixtures(
-          .withMetaMetricsController({
+        {
-            metaMetricsId: null,
+          fixtures: {
-            participateInMetaMetrics: false,
+            ...new FixtureBuilder()
-          })
+              .withMetaMetricsController({
-          .build(),
+                metaMetricsId: null,
-        ganacheOptions,
+                participateInMetaMetrics: false,
-        title: this.test.title,
+              })
-        failOnConsoleError: false,
+              .build(),
-        testSpecificMock: mockSentry,
+            // Intentionally corrupt state to trigger migration error during initialization
-      },
+            meta: undefined,
-      async ({ driver, mockedEndpoint }) => {
+          },
-        await driver.navigate();
+          ganacheOptions,
-        await driver.fill('#password', 'correct horse battery staple');
+          title: this.test.title,
-        await driver.press('#password', driver.Key.ENTER);
+          failOnConsoleError: false,
-        // Trigger error
+          testSpecificMock: mockSentryMigratorError,
-        driver.executeScript('window.stateHooks.throwTestError()');
+        },
-        driver.delay(3000);
+        async ({ driver, mockedEndpoint }) => {
-        // Wait for Sentry request
+          await driver.navigate();
-        const isPending = await mockedEndpoint.isPending();
+
-        assert.ok(
+          // Wait for Sentry request
-          isPending,
+          await driver.delay(3000);
-          'A request to sentry was sent when it should not have been',
-        );
-      },
-    );
-  });
-  it('should send error events', async function () {
-    await withFixtures(
-      {
-        fixtures: new FixtureBuilder()
-          .withMetaMetricsController({
-            metaMetricsId: 'fake-metrics-id',
-            participateInMetaMetrics: true,
-          })
-          .build(),
-        ganacheOptions,
-        title: this.test.title,
-        failOnConsoleError: false,
-        testSpecificMock: mockSentry,
-      },
-      async ({ driver, mockedEndpoint }) => {
-        await driver.navigate();
-        await driver.fill('#password', 'correct horse battery staple');
-        await driver.press('#password', driver.Key.ENTER);
-        // Trigger error
-        driver.executeScript('window.stateHooks.throwTestError()');
-        // Wait for Sentry request
-        await driver.wait(async () => {
           const isPending = await mockedEndpoint.isPending();
-          return isPending === false;
+          assert.ok(
-        }, 10000);
+            isPending,
-        const [mockedRequest] = await mockedEndpoint.getSeenRequests();
+            'A request to sentry was sent when it should not have been',
-        const mockTextBody = mockedRequest.body.text.split('\n');
+          );
-        const mockJsonBody = JSON.parse(mockTextBody[2]);
+        },
-        const { level, extra } = mockJsonBody;
+      );
-        const [{ type, value }] = mockJsonBody.exception.values;
+    });
-        const { participateInMetaMetrics } = extra.appState.store.metamask;
+    it('should send error events', async function () {
-        // Verify request
+      await withFixtures(
-        assert.equal(type, 'TestError');
+        {
-        assert.equal(value, 'Test Error');
+          fixtures: {
-        assert.equal(level, 'error');
+            ...new FixtureBuilder()
-        assert.equal(participateInMetaMetrics, true);
+              .withMetaMetricsController({
-      },
+                metaMetricsId: 'fake-metrics-id',
-    );
+                participateInMetaMetrics: true,
+              })
+              .build(),
+            // Intentionally corrupt state to trigger migration error during initialization
+            meta: undefined,
+          },
+          ganacheOptions,
+          title: this.test.title,
+          failOnConsoleError: false,
+          testSpecificMock: mockSentryMigratorError,
+        },
+        async ({ driver, mockedEndpoint }) => {
+          await driver.navigate();
+
+          // Wait for Sentry request
+          await driver.wait(async () => {
+            const isPending = await mockedEndpoint.isPending();
+            return isPending === false;
+          }, 3000);
+
+          const [mockedRequest] = await mockedEndpoint.getSeenRequests();
+          const mockTextBody = mockedRequest.body.text.split('\n');
+          const mockJsonBody = JSON.parse(mockTextBody[2]);
+          const { level } = mockJsonBody;
+          const [{ type, value }] = mockJsonBody.exception.values;
+          // Verify request
+          assert.equal(type, 'TypeError');
+          assert(value.includes(migrationError));
+          assert.equal(level, 'error');
+        },
+      );
+    });
+  });
+
+  describe('after initialization', function () {
+    it('should NOT send error events when participateInMetaMetrics is false', async function () {
+      await withFixtures(
+        {
+          fixtures: new FixtureBuilder()
+            .withMetaMetricsController({
+              metaMetricsId: null,
+              participateInMetaMetrics: false,
+            })
+            .build(),
+          ganacheOptions,
+          title: this.test.title,
+          failOnConsoleError: false,
+          testSpecificMock: mockSentryTestError,
+        },
+        async ({ driver, mockedEndpoint }) => {
+          await driver.navigate();
+          await driver.fill('#password', 'correct horse battery staple');
+          await driver.press('#password', driver.Key.ENTER);
+          // Trigger error
+          driver.executeScript('window.stateHooks.throwTestError()');
+          driver.delay(3000);
+          // Wait for Sentry request
+          const isPending = await mockedEndpoint.isPending();
+          assert.ok(
+            isPending,
+            'A request to sentry was sent when it should not have been',
+          );
+        },
+      );
+    });
+    it('should send error events', async function () {
+      await withFixtures(
+        {
+          fixtures: new FixtureBuilder()
+            .withMetaMetricsController({
+              metaMetricsId: 'fake-metrics-id',
+              participateInMetaMetrics: true,
+            })
+            .build(),
+          ganacheOptions,
+          title: this.test.title,
+          failOnConsoleError: false,
+          testSpecificMock: mockSentryTestError,
+        },
+        async ({ driver, mockedEndpoint }) => {
+          await driver.navigate();
+          await driver.fill('#password', 'correct horse battery staple');
+          await driver.press('#password', driver.Key.ENTER);
+          // Trigger error
+          driver.executeScript('window.stateHooks.throwTestError()');
+          // Wait for Sentry request
+          await driver.wait(async () => {
+            const isPending = await mockedEndpoint.isPending();
+            return isPending === false;
+          }, 10000);
+          const [mockedRequest] = await mockedEndpoint.getSeenRequests();
+          const mockTextBody = mockedRequest.body.text.split('\n');
+          const mockJsonBody = JSON.parse(mockTextBody[2]);
+          const { level, extra } = mockJsonBody;
+          const [{ type, value }] = mockJsonBody.exception.values;
+          const { participateInMetaMetrics } = extra.appState.store.metamask;
+          // Verify request
+          assert.equal(type, 'TestError');
+          assert.equal(value, 'Test Error');
+          assert.equal(level, 'error');
+          assert.equal(participateInMetaMetrics, true);
+        },
+      );
+    });
   });
 });