diff --git a/app/scripts/background.js b/app/scripts/background.js index d949aefc8..9dc28d238 100644 --- a/app/scripts/background.js +++ b/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'; diff --git a/app/scripts/controllers/metametrics.js b/app/scripts/controllers/metametrics.js index b5ef191fc..20a7cdcb5 100644 --- a/app/scripts/controllers/metametrics.js +++ b/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} 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 }); diff --git a/app/scripts/controllers/metametrics.test.js b/app/scripts/controllers/metametrics.test.js index ceab26a5a..09170d4ee 100644 --- a/app/scripts/controllers/metametrics.test.js +++ b/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); }); diff --git a/app/scripts/lib/sentry-filter-events.ts b/app/scripts/lib/sentry-filter-events.ts index 050f0bcd2..50d2f4c77 100644 --- a/app/scripts/lib/sentry-filter-events.ts +++ b/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; /** * @param options - Constructor options. @@ -40,7 +40,7 @@ export class FilterEvents implements Integration { constructor({ getMetaMetricsEnabled, }: { - getMetaMetricsEnabled: () => boolean; + getMetaMetricsEnabled: () => Promise; }) { 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; } diff --git a/app/scripts/lib/setup-persisted-state-hook.js b/app/scripts/lib/setup-persisted-state-hook.js new file mode 100644 index 000000000..9b29fad26 --- /dev/null +++ b/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(); +}; diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 7df0cea78..8cdc65223 100644 --- a/app/scripts/lib/setupSentry.js +++ b/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() { - if (getState) { - const appState = getState(); - if (!appState?.store?.metamask?.participateInMetaMetrics) { - return false; - } - } else { + async function getMetaMetricsEnabled() { + const appState = getState(); + if (Object.keys(appState) > 0) { + return Boolean(appState?.store?.metamask?.participateInMetaMetrics); + } + try { + 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(); } }; diff --git a/app/scripts/sentry-install.js b/app/scripts/sentry-install.js index aa1264c04..99d8ddd39 100644 --- a/app/scripts/sentry-install.js +++ b/app/scripts/sentry-install.js @@ -10,17 +10,16 @@ global.sentry = setupSentry({ }); /** - * As soon as state is available via getSentryState we can call the - * toggleSession method added to sentry in setupSentry to start automatic + * As soon as state is available via getSentryState we can 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); } diff --git a/app/scripts/ui.js b/app/scripts/ui.js index b3aa15c6c..50f37c430 100644 --- a/app/scripts/ui.js +++ b/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'; diff --git a/test/e2e/tests/errors.spec.js b/test/e2e/tests/errors.spec.js index 6285ac3b6..960135215 100644 --- a/test/e2e/tests/errors.spec.js +++ b/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( - { - fixtures: new FixtureBuilder() - .withMetaMetricsController({ - metaMetricsId: null, - participateInMetaMetrics: false, - }) - .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()'); - 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: 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 () => { + + describe('before 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(), + // 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.delay(3000); 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); - }, - ); + 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(), + // 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); + }, + ); + }); }); });