metametrics - ensure segment submission failures do not bubble up (#12573)

* metametrics - ensure segment submission failures do not bubble up

* metametrics - differentiate between trackEvent and submitEvent

* metametrics - validate event in trackEvent

* metametrics - re-throw error on clean stack

* lint fix

* controllers/metametrics - take a captureException option

* controllers/metametrics - capture and report any errors in trackPage

* Update app/scripts/controllers/metametrics.js

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

app/scripts/controllers/metametrics.js

@@ -7,6 +7,14 @@ import {
   METAMETRICS_BACKGROUND_PAGE_OBJECT,
 } from '../../../shared/constants/metametrics';
 
+const defaultCaptureException = (err) => {
+  // throw error on clean stack so its captured by platform integrations (eg sentry)
+  // but does not interupt the call stack
+  setTimeout(() => {
+    throw err;
+  });
+};
+
 /**
  * @typedef {import('../../../shared/constants/metametrics').MetaMetricsContext} MetaMetricsContext
  * @typedef {import('../../../shared/constants/metametrics').MetaMetricsEventPayload} MetaMetricsEventPayload
@@ -51,7 +59,9 @@ export default class MetaMetricsController {
     version,
     environment,
     initState,
+    captureException = defaultCaptureException,
   }) {
+    this._captureException = captureException;
     const prefState = preferencesStore.getState();
     this.chainId = getCurrentChainId();
     this.network = getNetworkIdentifier();
@@ -258,32 +268,52 @@ export default class MetaMetricsController {
    *  view
    */
   trackPage({ name, params, environmentType, page, referrer }, options) {
-    if (this.state.participateInMetaMetrics === false) {
+    try {
-      return;
+      if (this.state.participateInMetaMetrics === false) {
-    }
+        return;
+      }
 
-    if (this.state.participateInMetaMetrics === null && !options?.isOptInPath) {
+      if (
-      return;
+        this.state.participateInMetaMetrics === null &&
+        !options?.isOptInPath
+      ) {
+        return;
+      }
+      const { metaMetricsId } = this.state;
+      const idTrait = metaMetricsId ? 'userId' : 'anonymousId';
+      const idValue = metaMetricsId ?? METAMETRICS_ANONYMOUS_ID;
+      this.segment.page({
+        [idTrait]: idValue,
+        name,
+        properties: {
+          params,
+          locale: this.locale,
+          network: this.network,
+          chain_id: this.chainId,
+          environment_type: environmentType,
+        },
+        context: this._buildContext(referrer, page),
+      });
+    } catch (err) {
+      this._captureException(err);
     }
-    const { metaMetricsId } = this.state;
-    const idTrait = metaMetricsId ? 'userId' : 'anonymousId';
-    const idValue = metaMetricsId ?? METAMETRICS_ANONYMOUS_ID;
-    this.segment.page({
-      [idTrait]: idValue,
-      name,
-      properties: {
-        params,
-        locale: this.locale,
-        network: this.network,
-        chain_id: this.chainId,
-        environment_type: environmentType,
-      },
-      context: this._buildContext(referrer, page),
-    });
   }
 
   /**
-   * track a metametrics event, performing necessary payload manipulation and
+   * submits a metametrics event, not waiting for it to complete or allowing its error to bubble up
+   * @param {MetaMetricsEventPayload} payload - details of the event
+   * @param {MetaMetricsEventOptions} [options] - options for handling/routing the event
+   */
+  trackEvent(payload, options) {
+    // validation is not caught and handled
+    this.validatePayload(payload);
+    this.submitEvent(payload, options).catch((err) =>
+      this._captureException(err),
+    );
+  }
+
+  /**
+   * submits (or queues for submission) a metametrics event, performing necessary payload manipulation and
    * routing the event to the appropriate segment source. Will split events
    * with sensitiveProperties into two events, tracking the sensitiveProperties
    * with the anonymousId only.
@@ -291,21 +321,8 @@ export default class MetaMetricsController {
    * @param {MetaMetricsEventOptions} [options] - options for handling/routing the event
    * @returns {Promise<void>}
    */
-  async trackEvent(payload, options) {
+  async submitEvent(payload, options) {
-    // event and category are required fields for all payloads
+    this.validatePayload(payload);
-    if (!payload.event || !payload.category) {
-      throw new Error(
-        `Must specify event and category. Event was: ${
-          payload.event
-        }. Category was: ${payload.category}. Payload keys were: ${Object.keys(
-          payload,
-        )}. ${
-          typeof payload.properties === 'object'
-            ? `Payload property keys were: ${Object.keys(payload.properties)}`
-            : ''
-        }`,
-      );
-    }
 
     if (!this.state.participateInMetaMetrics && !options?.isOptIn) {
       return;
@@ -345,4 +362,25 @@ export default class MetaMetricsController {
 
     await Promise.all(events);
   }
+
+  /**
+   * validates a metametrics event
+   * @param {MetaMetricsEventPayload} payload - details of the event
+   */
+  validatePayload(payload) {
+    // event and category are required fields for all payloads
+    if (!payload.event || !payload.category) {
+      throw new Error(
+        `Must specify event and category. Event was: ${
+          payload.event
+        }. Category was: ${payload.category}. Payload keys were: ${Object.keys(
+          payload,
+        )}. ${
+          typeof payload.properties === 'object'
+            ? `Payload property keys were: ${Object.keys(payload.properties)}`
+            : ''
+        }`,
+      );
+    }
+  }
 }

app/scripts/controllers/metametrics.test.js

@@ -196,14 +196,14 @@ describe('MetaMetricsController', function () {
     });
   });
 
-  describe('trackEvent', function () {
+  describe('submitEvent', function () {
     it('should not track an event if user is not participating in metametrics', function () {
       const mock = sinon.mock(segment);
       const metaMetricsController = getMetaMetricsController({
         participateInMetaMetrics: false,
       });
       mock.expects('track').never();
-      metaMetricsController.trackEvent({
+      metaMetricsController.submitEvent({
         event: 'Fake Event',
         category: 'Unit Test',
         properties: {
@@ -230,7 +230,7 @@ describe('MetaMetricsController', function () {
             ...DEFAULT_EVENT_PROPERTIES,
           },
         });
-      metaMetricsController.trackEvent(
+      metaMetricsController.submitEvent(
         {
           event: 'Fake Event',
           category: 'Unit Test',
@@ -260,7 +260,7 @@ describe('MetaMetricsController', function () {
             ...DEFAULT_EVENT_PROPERTIES,
           },
         });
-      metaMetricsController.trackEvent(
+      metaMetricsController.submitEvent(
         {
           event: 'Fake Event',
           category: 'Unit Test',
@@ -289,7 +289,7 @@ describe('MetaMetricsController', function () {
             ...DEFAULT_EVENT_PROPERTIES,
           },
         });
-      metaMetricsController.trackEvent(
+      metaMetricsController.submitEvent(
         {
           event: 'Fake Event',
           category: 'Unit Test',
@@ -317,7 +317,7 @@ describe('MetaMetricsController', function () {
             ...DEFAULT_EVENT_PROPERTIES,
           },
         });
-      metaMetricsController.trackEvent({
+      metaMetricsController.submitEvent({
         event: 'Fake Event',
         category: 'Unit Test',
         properties: {
@@ -331,7 +331,7 @@ describe('MetaMetricsController', function () {
       const metaMetricsController = getMetaMetricsController();
       const flushStub = sinon.stub(segment, 'flush');
       const flushCalled = waitUntilCalled(flushStub, segment);
-      metaMetricsController.trackEvent(
+      metaMetricsController.submitEvent(
         {
           event: 'Fake Event',
           category: 'Unit Test',
@@ -344,13 +344,13 @@ describe('MetaMetricsController', function () {
     it('should throw if event or category not provided', function () {
       const metaMetricsController = getMetaMetricsController();
       assert.rejects(
-        () => metaMetricsController.trackEvent({ event: 'test' }),
+        () => metaMetricsController.submitEvent({ event: 'test' }),
         /Must specify event and category\./u,
         'must specify category',
       );
 
       assert.rejects(
-        () => metaMetricsController.trackEvent({ category: 'test' }),
+        () => metaMetricsController.submitEvent({ category: 'test' }),
         /Must specify event and category\./u,
         'must specify event',
       );
@@ -360,7 +360,7 @@ describe('MetaMetricsController', function () {
       const metaMetricsController = getMetaMetricsController();
       assert.rejects(
         () =>
-          metaMetricsController.trackEvent(
+          metaMetricsController.submitEvent(
             {
               event: 'Fake Event',
               category: 'Unit Test',
@@ -375,7 +375,7 @@ describe('MetaMetricsController', function () {
     it('should track sensitiveProperties in a separate, anonymous event', function () {
       const metaMetricsController = getMetaMetricsController();
       const spy = sinon.spy(segment, 'track');
-      metaMetricsController.trackEvent({
+      metaMetricsController.submitEvent({
         event: 'Fake Event',
         category: 'Unit Test',
         sensitiveProperties: { foo: 'bar' },

app/scripts/metamask-controller.js

@@ -17,6 +17,7 @@ import LedgerBridgeKeyring from '@metamask/eth-ledger-bridge-keyring';
 import LatticeKeyring from 'eth-lattice-keyring';
 import EthQuery from 'eth-query';
 import nanoid from 'nanoid';
+import { captureException } from '@sentry/browser';
 import {
   AddressBookController,
   ApprovalController,
@@ -190,6 +191,7 @@ export default class MetamaskController extends EventEmitter {
       version: this.platform.getVersion(),
       environment: process.env.METAMASK_ENVIRONMENT,
       initState: initState.MetaMetricsController,
+      captureException,
     });
 
     const gasFeeMessenger = this.controllerMessenger.getRestricted({