diff --git a/.iyarc b/.iyarc index 2b0adcf36..c494a2016 100644 --- a/.iyarc +++ b/.iyarc @@ -5,11 +5,6 @@ GHSA-257v-vj4p-3w2h # addressed by temporary patch in .yarn/patches/request-npm-2.88.2-f4a57c72c4.patch GHSA-p8p7-x288-28g6 -# Prototype pollution -# Not easily patched -# Minimal risk to us because we're using lockdown which also prevents this case of prototype pollution -GHSA-h755-8qp9-cq85 - # tough-cookie # this will go away soon when we get rid of web3-provider-engine GHSA-72xf-g2v4-qvf3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 18c9eaab9..2977751b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [10.35.0] ### Uncategorized - temp +## [10.34.5] +### Changed +- Improve error diagnostic information + - Add additional logging for state migrations ([#20424](https://github.com/MetaMask/metamask-extension/pull/20424), [#20517](https://github.com/MetaMask/metamask-extension/pull/20517), [#20521](https://github.com/MetaMask/metamask-extension/pull/20521)) + - Improve diagnostic state snapshot ([#20457](https://github.com/MetaMask/metamask-extension/pull/20457), [#20491](https://github.com/MetaMask/metamask-extension/pull/20491), [#20428](https://github.com/MetaMask/metamask-extension/pull/20428), [#20458](https://github.com/MetaMask/metamask-extension/pull/20458)) + - Capture additional errors when state migrations fail ([#20427](https://github.com/MetaMask/metamask-extension/pull/20427)) + +### Fixed +- Fix bug where state was temporarily incomplete upon initial load ([#20468](https://github.com/MetaMask/metamask-extension/pull/20468)) + - In rare circumstances, this bug may have resulted in data loss (of preferences, permissions, or tracked NFTs/tokens) or UI crashes. + ## [10.34.4] ### Changed - Updated snaps execution environment ([#20420](https://github.com/MetaMask/metamask-extension/pull/20420)) @@ -3891,6 +3902,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.35.0...HEAD [10.35.0]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...v10.35.0 [Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.4...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.5...HEAD +[10.34.5]: https://github.com/MetaMask/metamask-extension/compare/v10.34.4...v10.34.5 [10.34.4]: https://github.com/MetaMask/metamask-extension/compare/v10.34.3...v10.34.4 [10.34.3]: https://github.com/MetaMask/metamask-extension/compare/v10.34.2...v10.34.3 [10.34.2]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...v10.34.2 diff --git a/app/scripts/background.js b/app/scripts/background.js index 44d3870ad..44f9d177d 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -2,9 +2,11 @@ * @file The entry point for the web extension singleton process. */ -// This import sets up a global function required for Sentry to function. +// Disabled to allow setting up initial state hooks first + +// This import sets up global functions 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 './lib/setup-initial-state-hooks'; import EventEmitter from 'events'; import endOfStream from 'end-of-stream'; @@ -13,6 +15,7 @@ import debounce from 'debounce-stream'; import log from 'loglevel'; import browser from 'webextension-polyfill'; import { storeAsStream } from '@metamask/obs-store'; +import { isObject } from '@metamask/utils'; ///: BEGIN:ONLY_INCLUDE_IN(snaps) import { ApprovalType } from '@metamask/controller-utils'; ///: END:ONLY_INCLUDE_IN @@ -41,7 +44,7 @@ import Migrator from './lib/migrator'; import ExtensionPlatform from './platforms/extension'; import LocalStore from './lib/local-store'; import ReadOnlyNetworkStore from './lib/network-store'; -import { SENTRY_STATE } from './lib/setupSentry'; +import { SENTRY_BACKGROUND_STATE } from './lib/setupSentry'; import createStreamSink from './lib/createStreamSink'; import NotificationManager, { @@ -68,6 +71,12 @@ import DesktopManager from '@metamask/desktop/dist/desktop-manager'; ///: END:ONLY_INCLUDE_IN /* eslint-enable import/order */ +// Setup global hook for improved Sentry state snapshots during initialization +const inTest = process.env.IN_TEST; +const localStore = inTest ? new ReadOnlyNetworkStore() : new LocalStore(); +global.stateHooks.getMostRecentPersistedState = () => + localStore.mostRecentRetrievedState; + const { sentry } = global; const firstTimeState = { ...rawFirstTimeState }; @@ -79,7 +88,7 @@ const metamaskInternalProcessHash = { const metamaskBlockedPorts = ['trezor-connect']; -log.setDefaultLevel(process.env.METAMASK_DEBUG ? 'debug' : 'info'); +log.setLevel(process.env.METAMASK_DEBUG ? 'debug' : 'info', false); const platform = new ExtensionPlatform(); const notificationManager = new NotificationManager(); @@ -90,10 +99,6 @@ let uiIsTriggering = false; const openMetamaskTabsIDs = {}; const requestAccountTabIds = {}; let controller; - -// state persistence -const inTest = process.env.IN_TEST; -const localStore = inTest ? new ReadOnlyNetworkStore() : new LocalStore(); let versionedData; if (inTest || process.env.METAMASK_DEBUG) { @@ -264,7 +269,8 @@ browser.runtime.onConnectExternal.addListener(async (...args) => { */ async function initialize() { try { - const initState = await loadStateFromPersistence(); + const initData = await loadStateFromPersistence(); + const initState = initData.data; const initLangCode = await getFirstPreferredLangCode(); ///: BEGIN:ONLY_INCLUDE_IN(desktop) @@ -287,6 +293,7 @@ async function initialize() { initLangCode, {}, isFirstMetaMaskControllerSetup, + initData.meta, ); if (!isManifestV3) { await loadPhishingWarningPage(); @@ -409,6 +416,19 @@ export async function loadStateFromPersistence() { versionedData = await migrator.migrateData(versionedData); if (!versionedData) { throw new Error('MetaMask - migrator returned undefined'); + } else if (!isObject(versionedData.meta)) { + throw new Error( + `MetaMask - migrator metadata has invalid type '${typeof versionedData.meta}'`, + ); + } else if (typeof versionedData.meta.version !== 'number') { + throw new Error( + `MetaMask - migrator metadata version has invalid type '${typeof versionedData + .meta.version}'`, + ); + } else if (!isObject(versionedData.data)) { + throw new Error( + `MetaMask - migrator data has invalid type '${typeof versionedData.data}'`, + ); } // this initializes the meta/version data as a class variable to be used for future writes localStore.setMetadata(versionedData.meta); @@ -417,7 +437,7 @@ export async function loadStateFromPersistence() { localStore.set(versionedData.data); // return just the data - return versionedData.data; + return versionedData; } /** @@ -430,12 +450,14 @@ export async function loadStateFromPersistence() { * @param {string} initLangCode - The region code for the language preferred by the current user. * @param {object} overrides - object with callbacks that are allowed to override the setup controller logic (usefull for desktop app) * @param isFirstMetaMaskControllerSetup + * @param {object} stateMetadata - Metadata about the initial state and migrations, including the most recent migration version */ export function setupController( initState, initLangCode, overrides, isFirstMetaMaskControllerSetup, + stateMetadata, ) { // // MetaMask Controller @@ -462,6 +484,7 @@ export function setupController( localStore, overrides, isFirstMetaMaskControllerSetup, + currentMigrationVersion: stateMetadata.version, }); setupEnsIpfsResolver({ @@ -870,14 +893,9 @@ browser.runtime.onInstalled.addListener(({ reason }) => { }); function setupSentryGetStateGlobal(store) { - global.stateHooks.getSentryState = function () { - const fullState = store.getState(); - const debugState = maskObject({ metamask: fullState }, SENTRY_STATE); - return { - browser: window.navigator.userAgent, - store: debugState, - version: platform.getVersion(), - }; + global.stateHooks.getSentryAppState = function () { + const backgroundState = store.memStore.getState(); + return maskObject(backgroundState, SENTRY_BACKGROUND_STATE); }; } diff --git a/app/scripts/controllers/app-metadata.test.ts b/app/scripts/controllers/app-metadata.test.ts new file mode 100644 index 000000000..890811ee2 --- /dev/null +++ b/app/scripts/controllers/app-metadata.test.ts @@ -0,0 +1,104 @@ +import assert from 'assert'; +import AppMetadataController from './app-metadata'; + +const EXPECTED_DEFAULT_STATE = { + currentAppVersion: '', + previousAppVersion: '', + previousMigrationVersion: 0, + currentMigrationVersion: 0, +}; + +describe('AppMetadataController', () => { + describe('constructor', () => { + it('accepts initial state and does not modify it if currentMigrationVersion and platform.getVersion() match respective values in state', async () => { + const initState = { + currentAppVersion: '1', + previousAppVersion: '1', + previousMigrationVersion: 1, + currentMigrationVersion: 1, + }; + const appMetadataController = new AppMetadataController({ + state: initState, + currentMigrationVersion: 1, + currentAppVersion: '1', + }); + assert.deepStrictEqual(appMetadataController.store.getState(), initState); + }); + + it('sets default state and does not modify it', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + }); + assert.deepStrictEqual( + appMetadataController.store.getState(), + EXPECTED_DEFAULT_STATE, + ); + }); + + it('sets default state and does not modify it if options version parameters match respective default values', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + currentMigrationVersion: 0, + currentAppVersion: '', + }); + assert.deepStrictEqual( + appMetadataController.store.getState(), + EXPECTED_DEFAULT_STATE, + ); + }); + + it('updates the currentAppVersion state property if options.currentAppVersion does not match the default value', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + currentMigrationVersion: 0, + currentAppVersion: '1', + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentAppVersion: '1', + }); + }); + + it('updates the currentAppVersion and previousAppVersion state properties if options.currentAppVersion, currentAppVersion and previousAppVersion are all different', async () => { + const appMetadataController = new AppMetadataController({ + state: { + currentAppVersion: '2', + previousAppVersion: '1', + }, + currentAppVersion: '3', + currentMigrationVersion: 0, + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentAppVersion: '3', + previousAppVersion: '2', + }); + }); + + it('updates the currentMigrationVersion state property if the currentMigrationVersion param does not match the default value', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + currentMigrationVersion: 1, + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentMigrationVersion: 1, + }); + }); + + it('updates the currentMigrationVersion and previousMigrationVersion state properties if the currentMigrationVersion param, the currentMigrationVersion state property and the previousMigrationVersion state property are all different', async () => { + const appMetadataController = new AppMetadataController({ + state: { + currentMigrationVersion: 2, + previousMigrationVersion: 1, + }, + currentMigrationVersion: 3, + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentMigrationVersion: 3, + previousMigrationVersion: 2, + }); + }); + }); +}); diff --git a/app/scripts/controllers/app-metadata.ts b/app/scripts/controllers/app-metadata.ts new file mode 100644 index 000000000..0d745730d --- /dev/null +++ b/app/scripts/controllers/app-metadata.ts @@ -0,0 +1,99 @@ +import EventEmitter from 'events'; +import { ObservableStore } from '@metamask/obs-store'; + +/** + * The state of the AppMetadataController + */ +export type AppMetadataControllerState = { + currentAppVersion: string; + previousAppVersion: string; + previousMigrationVersion: number; + currentMigrationVersion: number; +}; + +/** + * The options that NetworkController takes. + */ +export type AppMetadataControllerOptions = { + currentMigrationVersion?: number; + currentAppVersion?: string; + state?: Partial; +}; + +const defaultState: AppMetadataControllerState = { + currentAppVersion: '', + previousAppVersion: '', + previousMigrationVersion: 0, + currentMigrationVersion: 0, +}; + +/** + * The AppMetadata controller stores metadata about the current extension instance, + * including the currently and previously installed versions, and the most recently + * run migration. + * + */ +export default class AppMetadataController extends EventEmitter { + /** + * Observable store containing controller data. + */ + store: ObservableStore; + + /** + * Constructs a AppMetadata controller. + * + * @param options - the controller options + * @param options.state - Initial controller state. + * @param options.currentMigrationVersion + * @param options.currentAppVersion + */ + constructor({ + currentAppVersion = '', + currentMigrationVersion = 0, + state = {}, + }: AppMetadataControllerOptions) { + super(); + + this.store = new ObservableStore({ + ...defaultState, + ...state, + }); + + this.#maybeUpdateAppVersion(currentAppVersion); + + this.#maybeUpdateMigrationVersion(currentMigrationVersion); + } + + /** + * Updates the currentAppVersion in state, and sets the previousAppVersion to the old currentAppVersion. + * + * @param maybeNewAppVersion + */ + #maybeUpdateAppVersion(maybeNewAppVersion: string): void { + const oldCurrentAppVersion = this.store.getState().currentAppVersion; + + if (maybeNewAppVersion !== oldCurrentAppVersion) { + this.store.updateState({ + currentAppVersion: maybeNewAppVersion, + previousAppVersion: oldCurrentAppVersion, + }); + } + } + + /** + * Updates the migrationVersion in state. + * + * @param maybeNewMigrationVersion + */ + #maybeUpdateMigrationVersion(maybeNewMigrationVersion: number): void { + const oldCurrentMigrationVersion = + this.store.getState().currentMigrationVersion; + + if (maybeNewMigrationVersion !== oldCurrentMigrationVersion) { + this.store.updateState({ + previousMigrationVersion: oldCurrentMigrationVersion, + currentMigrationVersion: maybeNewMigrationVersion, + }); + } + } +} diff --git a/app/scripts/lib/ComposableObservableStore.js b/app/scripts/lib/ComposableObservableStore.js index ff722a82d..80d9c483d 100644 --- a/app/scripts/lib/ComposableObservableStore.js +++ b/app/scripts/lib/ComposableObservableStore.js @@ -51,6 +51,7 @@ export default class ComposableObservableStore extends ObservableStore { updateStructure(config) { this.config = config; this.removeAllListeners(); + const initialState = {}; for (const key of Object.keys(config)) { if (!config[key]) { throw new Error(`Undefined '${key}'`); @@ -72,7 +73,10 @@ export default class ComposableObservableStore extends ObservableStore { }, ); } + + initialState[key] = store.state ?? store.getState?.(); } + this.updateState(initialState); } /** diff --git a/app/scripts/lib/ComposableObservableStore.test.js b/app/scripts/lib/ComposableObservableStore.test.js index b6a58f1e1..bb3dd48fd 100644 --- a/app/scripts/lib/ComposableObservableStore.test.js +++ b/app/scripts/lib/ComposableObservableStore.test.js @@ -120,6 +120,46 @@ describe('ComposableObservableStore', () => { }); }); + it('should initialize state with all three types of stores', () => { + const controllerMessenger = new ControllerMessenger(); + const exampleStore = new ObservableStore(); + const exampleController = new ExampleController({ + messenger: controllerMessenger, + }); + const oldExampleController = new OldExampleController(); + exampleStore.putState('state'); + exampleController.updateBar('state'); + oldExampleController.updateBaz('state'); + const store = new ComposableObservableStore({ controllerMessenger }); + + store.updateStructure({ + Example: exampleController, + OldExample: oldExampleController, + Store: exampleStore, + }); + + expect(store.getState()).toStrictEqual({ + Example: { bar: 'state' }, + OldExample: { baz: 'state' }, + Store: 'state', + }); + }); + + it('should initialize falsy state', () => { + const controllerMessenger = new ControllerMessenger(); + const exampleStore = new ObservableStore(); + exampleStore.putState(false); + const store = new ComposableObservableStore({ controllerMessenger }); + + store.updateStructure({ + Example: exampleStore, + }); + + expect(store.getState()).toStrictEqual({ + Example: false, + }); + }); + it('should return flattened state', () => { const controllerMessenger = new ControllerMessenger(); const fooStore = new ObservableStore({ foo: 'foo' }); diff --git a/app/scripts/lib/local-store.js b/app/scripts/lib/local-store.js index b0a223561..40908af5e 100644 --- a/app/scripts/lib/local-store.js +++ b/app/scripts/lib/local-store.js @@ -16,6 +16,7 @@ export default class ExtensionStore { // once data persistence fails once and it flips true we don't send further // data persistence errors to sentry this.dataPersistenceFailing = false; + this.mostRecentRetrievedState = null; } setMetadata(initMetaData) { @@ -66,8 +67,10 @@ export default class ExtensionStore { // extension.storage.local always returns an obj // if the object is empty, treat it as undefined if (isEmpty(result)) { + this.mostRecentRetrievedState = null; return undefined; } + this.mostRecentRetrievedState = result; return result; } diff --git a/app/scripts/lib/local-store.test.js b/app/scripts/lib/local-store.test.js index 2c3cea405..8b786ca81 100644 --- a/app/scripts/lib/local-store.test.js +++ b/app/scripts/lib/local-store.test.js @@ -2,11 +2,12 @@ import browser from 'webextension-polyfill'; import LocalStore from './local-store'; jest.mock('webextension-polyfill', () => ({ + runtime: { lastError: null }, storage: { local: true }, })); -const setup = ({ isSupported }) => { - browser.storage.local = isSupported; +const setup = ({ localMock = jest.fn() } = {}) => { + browser.storage.local = localMock; return new LocalStore(); }; describe('LocalStore', () => { @@ -15,21 +16,27 @@ describe('LocalStore', () => { }); describe('contructor', () => { it('should set isSupported property to false when browser does not support local storage', () => { - const localStore = setup({ isSupported: false }); + const localStore = setup({ localMock: false }); expect(localStore.isSupported).toBe(false); }); it('should set isSupported property to true when browser supports local storage', () => { - const localStore = setup({ isSupported: true }); + const localStore = setup(); expect(localStore.isSupported).toBe(true); }); + + it('should initialize mostRecentRetrievedState to null', () => { + const localStore = setup({ localMock: false }); + + expect(localStore.mostRecentRetrievedState).toBeNull(); + }); }); describe('setMetadata', () => { it('should set the metadata property on LocalStore', () => { const metadata = { version: 74 }; - const localStore = setup({ isSupported: true }); + const localStore = setup(); localStore.setMetadata(metadata); expect(localStore.metadata).toStrictEqual(metadata); @@ -38,21 +45,21 @@ describe('LocalStore', () => { describe('set', () => { it('should throw an error if called in a browser that does not support local storage', async () => { - const localStore = setup({ isSupported: false }); + const localStore = setup({ localMock: false }); await expect(() => localStore.set()).rejects.toThrow( 'Metamask- cannot persist state to local store as this browser does not support this action', ); }); it('should throw an error if not passed a truthy value as an argument', async () => { - const localStore = setup({ isSupported: true }); + const localStore = setup(); await expect(() => localStore.set()).rejects.toThrow( 'MetaMask - updated state is missing', ); }); it('should throw an error if passed a valid argument but metadata has not yet been set', async () => { - const localStore = setup({ isSupported: true }); + const localStore = setup(); await expect(() => localStore.set({ appState: { test: true } }), ).rejects.toThrow( @@ -61,7 +68,7 @@ describe('LocalStore', () => { }); it('should not throw if passed a valid argument and metadata has been set', async () => { - const localStore = setup({ isSupported: true }); + const localStore = setup(); localStore.setMetadata({ version: 74 }); await expect(async function () { localStore.set({ appState: { test: true } }); @@ -71,9 +78,39 @@ describe('LocalStore', () => { describe('get', () => { it('should return undefined if called in a browser that does not support local storage', async () => { - const localStore = setup({ isSupported: false }); + const localStore = setup({ localMock: false }); const result = await localStore.get(); expect(result).toStrictEqual(undefined); }); + + it('should update mostRecentRetrievedState', async () => { + const localStore = setup({ + localMock: { + get: jest + .fn() + .mockImplementation(() => + Promise.resolve({ appState: { test: true } }), + ), + }, + }); + + await localStore.get(); + + expect(localStore.mostRecentRetrievedState).toStrictEqual({ + appState: { test: true }, + }); + }); + + it('should reset mostRecentRetrievedState to null if storage.local is empty', async () => { + const localStore = setup({ + localMock: { + get: jest.fn().mockImplementation(() => Promise.resolve({})), + }, + }); + + await localStore.get(); + + expect(localStore.mostRecentRetrievedState).toStrictEqual(null); + }); }); }); diff --git a/app/scripts/lib/migrator/index.js b/app/scripts/lib/migrator/index.js index ca2abb8ea..5925430f0 100644 --- a/app/scripts/lib/migrator/index.js +++ b/app/scripts/lib/migrator/index.js @@ -1,4 +1,5 @@ import EventEmitter from 'events'; +import log from 'loglevel'; /** * @typedef {object} Migration @@ -36,6 +37,8 @@ export default class Migrator extends EventEmitter { // perform each migration for (const migration of pendingMigrations) { try { + log.info(`Running migration ${migration.version}...`); + // attempt migration and validate const migratedData = await migration.migrate(versionedData); if (!migratedData.data) { @@ -52,6 +55,8 @@ export default class Migrator extends EventEmitter { // accept the migration as good // eslint-disable-next-line no-param-reassign versionedData = migratedData; + + log.info(`Migration ${migration.version} complete`); } catch (err) { // rewrite error message to add context without clobbering stack const originalErrorMessage = err.message; diff --git a/app/scripts/lib/network-store.js b/app/scripts/lib/network-store.js index ea6ba5876..3a0326a2b 100644 --- a/app/scripts/lib/network-store.js +++ b/app/scripts/lib/network-store.js @@ -15,6 +15,7 @@ export default class ReadOnlyNetworkStore { this._initialized = false; this._initializing = this._init(); this._state = undefined; + this.mostRecentRetrievedState = null; } /** @@ -47,6 +48,11 @@ export default class ReadOnlyNetworkStore { if (!this._initialized) { await this._initializing; } + // Delay setting this until after the first read, to match the + // behavior of the local store. + if (!this.mostRecentRetrievedState) { + this.mostRecentRetrievedState = this._state; + } return this._state; } diff --git a/app/scripts/lib/setup-initial-state-hooks.js b/app/scripts/lib/setup-initial-state-hooks.js new file mode 100644 index 000000000..9b3f18e09 --- /dev/null +++ b/app/scripts/lib/setup-initial-state-hooks.js @@ -0,0 +1,76 @@ +import { maskObject } from '../../../shared/modules/object.utils'; +import ExtensionPlatform from '../platforms/extension'; +import LocalStore from './local-store'; +import ReadOnlyNetworkStore from './network-store'; +import { SENTRY_BACKGROUND_STATE } from './setupSentry'; + +const platform = new ExtensionPlatform(); + +// This instance of `localStore` is used by Sentry to get the persisted state +const sentryLocalStore = process.env.IN_TEST + ? new ReadOnlyNetworkStore() + : new LocalStore(); + +/** + * Get the persisted wallet state. + * + * @returns The persisted wallet state. + */ +globalThis.stateHooks.getPersistedState = async function () { + return await sentryLocalStore.get(); +}; + +const persistedStateMask = { + data: SENTRY_BACKGROUND_STATE, + meta: { + version: true, + }, +}; + +/** + * Get a state snapshot for Sentry. This is used to add additional context to + * error reports, and it's used when processing errors and breadcrumbs to + * determine whether the user has opted into Metametrics. + * + * This uses the persisted state pre-initialization, and the in-memory state + * post-initialization. In both cases the state is anonymized. + * + * @returns A Sentry state snapshot. + */ +globalThis.stateHooks.getSentryState = function () { + const sentryState = { + browser: window.navigator.userAgent, + version: platform.getVersion(), + }; + // If `getSentryAppState` is set, it implies that initialization has completed + if (globalThis.stateHooks.getSentryAppState) { + return { + ...sentryState, + state: globalThis.stateHooks.getSentryAppState(), + }; + } else if ( + // This is truthy if Sentry has retrieved state at least once already. This + // should always be true when getting context for an error report, but can + // be unset when Sentry is performing the opt-in check. + sentryLocalStore.mostRecentRetrievedState || + // This is only set in the background process. + globalThis.stateHooks.getMostRecentPersistedState + ) { + const persistedState = + sentryLocalStore.mostRecentRetrievedState || + globalThis.stateHooks.getMostRecentPersistedState(); + // This can be unset when this method is called in the background for an + // opt-in check, but the state hasn't been loaded yet. + if (persistedState) { + return { + ...sentryState, + persistedState: maskObject(persistedState, persistedStateMask), + }; + } + } + // This branch means that local storage has not yet been read, so we have + // no choice but to omit the application state. + // This should be unreachable when getting context for an error report, but + // can be false when Sentry is performing the opt-in check. + return sentryState; +}; diff --git a/app/scripts/lib/setup-persisted-state-hook.js b/app/scripts/lib/setup-persisted-state-hook.js deleted file mode 100644 index 9b29fad26..000000000 --- a/app/scripts/lib/setup-persisted-state-hook.js +++ /dev/null @@ -1,10 +0,0 @@ -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 dd0eb7bd6..c6d29995e 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -23,58 +23,175 @@ export const ERROR_URL_ALLOWLIST = { SEGMENT: 'segment.io', }; -// This describes the subset of Redux state attached to errors sent to Sentry -// These properties have some potential to be useful for debugging, and they do -// not contain any identifiable information. -export const SENTRY_STATE = { - gas: true, - history: true, - metamask: { +// This describes the subset of background controller state attached to errors +// sent to Sentry These properties have some potential to be useful for +// debugging, and they do not contain any identifiable information. +export const SENTRY_BACKGROUND_STATE = { + AccountTracker: { + currentBlockGasLimit: true, + }, + AlertController: { alertEnabledness: true, - completedOnboarding: true, + }, + AppMetadataController: { + currentAppVersion: true, + previousAppVersion: true, + previousMigrationVersion: true, + currentMigrationVersion: true, + }, + AppStateController: { connectedStatusPopoverHasBeenShown: true, + defaultHomeActiveTabName: true, + }, + CurrencyController: { conversionDate: true, conversionRate: true, - currentBlockGasLimit: true, currentCurrency: true, - currentLocale: true, - customNonceValue: true, - defaultHomeActiveTabName: true, - desktopEnabled: true, - featureFlags: true, - firstTimeFlowType: true, - forgottenPassword: true, - incomingTxLastFetchedBlockByChainId: true, - ipfsGateway: true, - isAccountMenuOpen: true, - isInitialized: true, - isUnlocked: true, - metaMetricsId: true, nativeCurrency: true, + }, + DecryptMessageController: { + unapprovedDecryptMsgCount: true, + }, + DesktopController: { + desktopEnabled: true, + }, + EncryptionPublicKeyController: { + unapprovedEncryptionPublicKeyMsgCount: true, + }, + IncomingTransactionsController: { + incomingTxLastFetchedBlockByChainId: true, + }, + KeyringController: { + isUnlocked: true, + }, + MetaMetricsController: { + metaMetricsId: true, + participateInMetaMetrics: true, + }, + NetworkController: { networkId: true, networkStatus: true, - nextNonce: true, - participateInMetaMetrics: true, - preferences: true, providerConfig: { nickname: true, ticker: true, type: true, }, + }, + OnboardingController: { + completedOnboarding: true, + firstTimeFlowType: true, seedPhraseBackedUp: true, - unapprovedDecryptMsgCount: true, - unapprovedEncryptionPublicKeyMsgCount: true, - unapprovedMsgCount: true, - unapprovedPersonalMsgCount: true, - unapprovedTypedMessagesCount: true, + }, + PreferencesController: { + currentLocale: true, + featureFlags: true, + forgottenPassword: true, + ipfsGateway: true, + preferences: true, useBlockie: true, useNonceField: true, usePhishDetect: true, + }, + SignatureController: { + unapprovedMsgCount: true, + unapprovedPersonalMsgCount: true, + unapprovedTypedMessagesCount: true, + }, +}; + +const flattenedBackgroundStateMask = Object.values( + SENTRY_BACKGROUND_STATE, +).reduce((partialBackgroundState, controllerState) => { + return { + ...partialBackgroundState, + ...controllerState, + }; +}, {}); + +// This describes the subset of Redux state attached to errors sent to Sentry +// These properties have some potential to be useful for debugging, and they do +// not contain any identifiable information. +export const SENTRY_UI_STATE = { + gas: true, + history: true, + metamask: { + ...flattenedBackgroundStateMask, + // This property comes from the background but isn't in controller state + isInitialized: true, + // These properties are in the `metamask` slice but not in the background state + customNonceValue: true, + isAccountMenuOpen: true, + nextNonce: true, welcomeScreenSeen: true, }, unconnectedAccount: true, }; +/** + * Returns whether MetaMetrics is enabled, given the application state. + * + * @param {{ state: unknown} | { persistedState: unknown }} appState - Application state + * @returns `true` if MetaMask's state has been initialized, and MetaMetrics + * is enabled, `false` otherwise. + */ +function getMetaMetricsEnabledFromAppState(appState) { + // during initialization after loading persisted state + if (appState.persistedState) { + return getMetaMetricsEnabledFromPersistedState(appState.persistedState); + // After initialization + } else if (appState.state) { + // UI + if (appState.state.metamask) { + return Boolean(appState.state.metamask.participateInMetaMetrics); + } + // background + return Boolean( + appState.state.MetaMetricsController?.participateInMetaMetrics, + ); + } + // during initialization, before first persisted state is read + return false; +} + +/** + * Returns whether MetaMetrics is enabled, given the persisted state. + * + * @param {unknown} persistedState - Application state + * @returns `true` if MetaMask's state has been initialized, and MetaMetrics + * is enabled, `false` otherwise. + */ +function getMetaMetricsEnabledFromPersistedState(persistedState) { + return Boolean( + persistedState?.data?.MetaMetricsController?.participateInMetaMetrics, + ); +} + +/** + * Returns whether onboarding has completed, given the application state. + * + * @param {Record} appState - Application state + * @returns `true` if MetaMask's state has been initialized, and MetaMetrics + * is enabled, `false` otherwise. + */ +function getOnboardingCompleteFromAppState(appState) { + // during initialization after loading persisted state + if (appState.persistedState) { + return Boolean( + appState.persistedState.data?.OnboardingController?.completedOnboarding, + ); + // After initialization + } else if (appState.state) { + // UI + if (appState.state.metamask) { + return Boolean(appState.state.metamask.completedOnboarding); + } + // background + return Boolean(appState.state.OnboardingController?.completedOnboarding); + } + // during initialization, before first persisted state is read + return false; +} + export default function setupSentry({ release, getState }) { if (!release) { throw new Error('Missing release'); @@ -112,22 +229,21 @@ export default function setupSentry({ release, getState }) { } /** - * A function that returns whether MetaMetrics is enabled. This should also - * return `false` if state has not yet been initialzed. + * Returns whether MetaMetrics is enabled. If the application hasn't yet + * been initialized, the persisted state will be used (if any). * - * @returns `true` if MetaMask's state has been initialized, and MetaMetrics - * is enabled, `false` otherwise. + * @returns `true` if MetaMetrics is enabled, `false` otherwise. */ async function getMetaMetricsEnabled() { const appState = getState(); - if (Object.keys(appState) > 0) { - return Boolean(appState?.store?.metamask?.participateInMetaMetrics); + if (appState.state || appState.persistedState) { + return getMetaMetricsEnabledFromAppState(appState); } + // If we reach here, it means the error was thrown before initialization + // completed, and before we loaded the persisted state for the first time. try { const persistedState = await globalThis.stateHooks.getPersistedState(); - return Boolean( - persistedState?.data?.MetaMetricsController?.participateInMetaMetrics, - ); + return getMetaMetricsEnabledFromPersistedState(persistedState); } catch (error) { console.error(error); return false; @@ -175,17 +291,15 @@ function hideUrlIfNotInternal(url) { */ export function beforeBreadcrumb(getState) { return (breadcrumb) => { - if (getState) { - const appState = getState(); - if ( - Object.values(appState).length && - (!appState?.store?.metamask?.participateInMetaMetrics || - !appState?.store?.metamask?.completedOnboarding || - breadcrumb?.category === 'ui.input') - ) { - return null; - } - } else { + if (!getState) { + return null; + } + const appState = getState(); + if ( + !getMetaMetricsEnabledFromAppState(appState) || + !getOnboardingCompleteFromAppState(appState) || + breadcrumb?.category === 'ui.input' + ) { return null; } const newBreadcrumb = removeUrlsFromBreadCrumb(breadcrumb); diff --git a/app/scripts/lockdown-more.js b/app/scripts/lockdown-more.js index e6637602a..317052312 100644 --- a/app/scripts/lockdown-more.js +++ b/app/scripts/lockdown-more.js @@ -28,7 +28,7 @@ try { const namedIntrinsics = Reflect.ownKeys(new Compartment().globalThis); // These named intrinsics are not automatically hardened by `lockdown` - const shouldHardenManually = new Set(['eval', 'Function']); + const shouldHardenManually = new Set(['eval', 'Function', 'Symbol']); const globalProperties = new Set([ // universalPropertyNames is a constant added by lockdown to global scope diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index eaeb27816..43cb9e2bb 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -195,6 +195,7 @@ import createMetaRPCHandler from './lib/createMetaRPCHandler'; import { previousValueComparator } from './lib/util'; import createMetamaskMiddleware from './lib/createMetamaskMiddleware'; import EncryptionPublicKeyController from './controllers/encryption-public-key'; +import AppMetadataController from './controllers/app-metadata'; import { CaveatMutatorFactories, @@ -264,6 +265,8 @@ export default class MetamaskController extends EventEmitter { // instance of a class that wraps the extension's storage local API. this.localStoreApiWrapper = opts.localStore; + this.currentMigrationVersion = opts.currentMigrationVersion; + // observable state store this.store = new ComposableObservableStore({ state: initState, @@ -284,6 +287,12 @@ export default class MetamaskController extends EventEmitter { } }); + this.appMetadataController = new AppMetadataController({ + state: initState.AppMetadataController, + currentMigrationVersion: this.currentMigrationVersion, + currentAppVersion: version, + }); + // next, we will initialize the controllers // controller initialization order matters @@ -1558,6 +1567,7 @@ export default class MetamaskController extends EventEmitter { this.store.updateStructure({ AppStateController: this.appStateController.store, + AppMetadataController: this.appMetadataController.store, TransactionController: this.txController.store, KeyringController: this.keyringController.store, PreferencesController: this.preferencesController.store, @@ -1602,6 +1612,7 @@ export default class MetamaskController extends EventEmitter { this.memStore = new ComposableObservableStore({ config: { AppStateController: this.appStateController.store, + AppMetadataController: this.appMetadataController.store, NetworkController: this.networkController, CachedBalancesController: this.cachedBalancesController.store, KeyringController: this.keyringController.memStore, @@ -2696,6 +2707,9 @@ export default class MetamaskController extends EventEmitter { assetsContractController.getBalancesInSingleCall.bind( assetsContractController, ), + + // E2E testing + throwTestError: this.throwTestError.bind(this), }; } @@ -4413,6 +4427,21 @@ export default class MetamaskController extends EventEmitter { return nonceLock.nextNonce; } + /** + * Throw an artificial error in a timeout handler for testing purposes. + * + * @param message - The error message. + * @deprecated This is only mean to facilitiate E2E testing. We should not + * use this for handling errors. + */ + throwTestError(message) { + setTimeout(() => { + const error = new Error(message); + error.name = 'TestError'; + throw error; + }); + } + //============================================================================= // CONFIG //============================================================================= diff --git a/app/scripts/migrations/077.js b/app/scripts/migrations/077.js index 5a5d4f1ac..9adb293ba 100644 --- a/app/scripts/migrations/077.js +++ b/app/scripts/migrations/077.js @@ -1,4 +1,6 @@ import { cloneDeep } from 'lodash'; +import log from 'loglevel'; +import { hasProperty, isObject } from '@metamask/utils'; import transformState077For082 from './077-supplements/077-supplement-for-082'; import transformState077For084 from './077-supplements/077-supplement-for-084'; import transformState077For086 from './077-supplements/077-supplement-for-086'; @@ -29,8 +31,23 @@ export default { }; function transformState(state) { - const TokenListController = state?.TokenListController || {}; - + if (!hasProperty(state, 'TokenListController')) { + log.warn('Skipping migration, TokenListController state is missing'); + return state; + } else if (!isObject(state.TokenListController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.TokenListController is ${typeof state.TokenListController}`, + ), + ); + return state; + } else if (!hasProperty(state.TokenListController, 'tokensChainsCache')) { + log.warn( + 'Skipping migration, TokenListController.tokensChainsCache state is missing', + ); + return state; + } + const { TokenListController } = state; const { tokensChainsCache } = TokenListController; let dataCache; diff --git a/app/scripts/migrations/077.test.js b/app/scripts/migrations/077.test.js index 53efb5cd5..67de1a8fa 100644 --- a/app/scripts/migrations/077.test.js +++ b/app/scripts/migrations/077.test.js @@ -1,11 +1,22 @@ +import { cloneDeep } from 'lodash'; import migration77 from './077'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + startSession: jest.fn(), + endSession: jest.fn(), + toggleSession: jest.fn(), + captureException: sentryCaptureExceptionMock, +}; + describe('migration #77', () => { it('should update the version metadata', async () => { const oldStorage = { meta: { version: 76, }, + data: {}, }; const newStorage = await migration77.migrate(oldStorage); @@ -13,6 +24,65 @@ describe('migration #77', () => { version: 77, }); }); + + it('should return state unchanged if token list controller is missing', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + Foo: { + bar: 'baz', + }, + }, + }; + + const newStorage = await migration77.migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('should capture an exception if the TokenListController state is invalid', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: 'test', + }, + }; + + await migration77.migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokenListController is string`), + ); + }); + + it('should return state unchanged if tokenChainsCache is missing', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + it('should change the data from array to object for a single network', async () => { const oldStorage = { meta: { diff --git a/app/scripts/migrations/082.test.js b/app/scripts/migrations/082.test.js index 6f221c5c6..0060c853c 100644 --- a/app/scripts/migrations/082.test.js +++ b/app/scripts/migrations/082.test.js @@ -1,6 +1,12 @@ import { v4 } from 'uuid'; import { migrate, version } from './082'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + jest.mock('uuid', () => { const actual = jest.requireActual('uuid'); @@ -472,10 +478,72 @@ describe('migration #82', () => { }, }; const newStorage = await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalled(); expect(newStorage.data).toStrictEqual(oldStorage.data); }); - it('should not change anything if there is no frequentRpcListDetail property on PreferencesController', async () => { + it('should capture an exception if any PreferencesController.frequentRpcListDetail entries are not objects', async () => { + const oldStorage = { + meta: { + version: 81, + }, + data: { + PreferencesController: { + transactionSecurityCheckEnabled: false, + useBlockie: false, + useCurrencyRateCheck: true, + useMultiAccountBalanceChecker: true, + useNftDetection: false, + useNonceField: false, + frequentRpcListDetail: [ + { + chainId: '0x539', + nickname: 'Localhost 8545', + rpcPrefs: {}, + rpcUrl: 'http://localhost:8545', + ticker: 'ETH', + }, + 'invalid entry type', + 1, + ], + }, + NetworkController: { + network: '1', + networkDetails: { + EIPS: { + 1559: true, + }, + }, + previousProviderStore: { + chainId: '0x89', + nickname: 'Polygon Mainnet', + rpcPrefs: {}, + rpcUrl: + 'https://polygon-mainnet.infura.io/v3/373266a93aab4acda48f89d4fe77c748', + ticker: 'MATIC', + type: 'rpc', + }, + provider: { + chainId: '0x1', + nickname: '', + rpcPrefs: {}, + rpcUrl: '', + ticker: 'ETH', + type: 'mainnet', + }, + }, + }, + }; + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error( + `state.PreferencesController.frequentRpcListDetail contains an element of type string`, + ), + ); + }); + + it('should not change anything, and not capture an exception, if there is no frequentRpcListDetail property on PreferencesController but there is a networkConfigurations object', async () => { const oldStorage = { meta: { version: 81, @@ -556,9 +624,60 @@ describe('migration #82', () => { }, }; const newStorage = await migrate(oldStorage); + expect(sentryCaptureExceptionMock).not.toHaveBeenCalled(); expect(newStorage.data).toStrictEqual(oldStorage.data); }); + it('should capture an exception if there is no frequentRpcListDetail property on PreferencesController and no networkConfiguration object', async () => { + const oldStorage = { + meta: { + version: 81, + }, + data: { + PreferencesController: { + transactionSecurityCheckEnabled: false, + useBlockie: false, + useCurrencyRateCheck: true, + useMultiAccountBalanceChecker: true, + useNftDetection: false, + useNonceField: false, + }, + NetworkController: { + network: '1', + networkDetails: { + EIPS: { + 1559: true, + }, + }, + previousProviderStore: { + chainId: '0x89', + nickname: 'Polygon Mainnet', + rpcPrefs: {}, + rpcUrl: + 'https://polygon-mainnet.infura.io/v3/373266a93aab4acda48f89d4fe77c748', + ticker: 'MATIC', + type: 'rpc', + }, + provider: { + chainId: '0x1', + nickname: '', + rpcPrefs: {}, + rpcUrl: '', + ticker: 'ETH', + type: 'mainnet', + }, + }, + }, + }; + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error( + `typeof state.PreferencesController.frequentRpcListDetail is undefined`, + ), + ); + }); + it('should change nothing if PreferencesController is undefined', async () => { const oldStorage = { meta: { @@ -595,4 +714,61 @@ describe('migration #82', () => { const newStorage = await migrate(oldStorage); expect(newStorage.data).toStrictEqual(oldStorage.data); }); + + it('should capture an exception if PreferencesController is not an object', async () => { + const oldStorage = { + meta: { + version: 81, + }, + data: { + NetworkController: { + network: '1', + networkDetails: { + EIPS: { + 1559: true, + }, + }, + previousProviderStore: { + chainId: '0x89', + nickname: 'Polygon Mainnet', + rpcPrefs: {}, + rpcUrl: + 'https://polygon-mainnet.infura.io/v3/373266a93aab4acda48f89d4fe77c748', + ticker: 'MATIC', + type: 'rpc', + }, + provider: { + chainId: '0x1', + nickname: '', + rpcPrefs: {}, + rpcUrl: '', + ticker: 'ETH', + type: 'mainnet', + }, + }, + PreferencesController: false, + }, + }; + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.PreferencesController is boolean`), + ); + }); + + it('should capture an exception if NetworkController is undefined', async () => { + const oldStorage = { + meta: { + version: 81, + }, + data: { + PreferencesController: {}, + }, + }; + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is undefined`), + ); + }); }); diff --git a/app/scripts/migrations/082.ts b/app/scripts/migrations/082.ts index fdda1fb8d..29ff59eea 100644 --- a/app/scripts/migrations/082.ts +++ b/app/scripts/migrations/082.ts @@ -1,6 +1,7 @@ import { cloneDeep } from 'lodash'; import { hasProperty, isObject } from '@metamask/utils'; import { v4 } from 'uuid'; +import log from 'loglevel'; export const version = 82; @@ -25,14 +26,56 @@ export async function migrate(originalVersionedData: { } function transformState(state: Record) { + if (!hasProperty(state, 'PreferencesController')) { + log.warn(`state.PreferencesController is undefined`); + return state; + } + if (!isObject(state.PreferencesController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.PreferencesController is ${typeof state.PreferencesController}`, + ), + ); + return state; + } if ( - !hasProperty(state, 'PreferencesController') || - !isObject(state.PreferencesController) || - !isObject(state.NetworkController) || - !hasProperty(state.PreferencesController, 'frequentRpcListDetail') || - !Array.isArray(state.PreferencesController.frequentRpcListDetail) || - !state.PreferencesController.frequentRpcListDetail.every(isObject) + !hasProperty(state, 'NetworkController') || + !isObject(state.NetworkController) ) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); + return state; + } + if ( + !hasProperty(state.PreferencesController, 'frequentRpcListDetail') || + !Array.isArray(state.PreferencesController.frequentRpcListDetail) + ) { + const inPost077SupplementFor082State = + state.NetworkController.networkConfigurations && + state.PreferencesController.frequentRpcListDetail === undefined; + if (!inPost077SupplementFor082State) { + global.sentry?.captureException?.( + new Error( + `typeof state.PreferencesController.frequentRpcListDetail is ${typeof state + .PreferencesController.frequentRpcListDetail}`, + ), + ); + } + return state; + } + if (!state.PreferencesController.frequentRpcListDetail.every(isObject)) { + const erroneousElement = + state.PreferencesController.frequentRpcListDetail.find( + (element) => !isObject(element), + ); + global.sentry?.captureException?.( + new Error( + `state.PreferencesController.frequentRpcListDetail contains an element of type ${typeof erroneousElement}`, + ), + ); return state; } const { PreferencesController, NetworkController } = state; diff --git a/app/scripts/migrations/083.test.js b/app/scripts/migrations/083.test.js index c59951aef..b8f91dfcd 100644 --- a/app/scripts/migrations/083.test.js +++ b/app/scripts/migrations/083.test.js @@ -1,6 +1,12 @@ import { v4 } from 'uuid'; import { migrate, version } from './083'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + jest.mock('uuid', () => { const actual = jest.requireActual('uuid'); @@ -165,6 +171,24 @@ describe('migration #83', () => { expect(newStorage).toStrictEqual(expectedNewStorage); }); + it('should capture an exception if state.NetworkController is undefined', async () => { + const oldStorage = { + meta: { + version, + }, + data: { + testProperty: 'testValue', + }, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is undefined`), + ); + }); + it('should not modify state if state.NetworkController is not an object', async () => { const oldStorage = { meta: { @@ -190,6 +214,25 @@ describe('migration #83', () => { expect(newStorage).toStrictEqual(expectedNewStorage); }); + it('should capture an exception if state.NetworkController is not an object', async () => { + const oldStorage = { + meta: { + version, + }, + data: { + NetworkController: false, + testProperty: 'testValue', + }, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is boolean`), + ); + }); + it('should not modify state if state.NetworkController.networkConfigurations is undefined', async () => { const oldStorage = { meta: { @@ -221,6 +264,28 @@ describe('migration #83', () => { expect(newStorage).toStrictEqual(expectedNewStorage); }); + it('should capture an exception if state.NetworkController.networkConfigurations is undefined', async () => { + const oldStorage = { + meta: { + version, + }, + data: { + NetworkController: { + testNetworkControllerProperty: 'testNetworkControllerValue', + networkConfigurations: undefined, + }, + testProperty: 'testValue', + }, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof NetworkController.networkConfigurations is undefined`), + ); + }); + it('should not modify state if state.NetworkController.networkConfigurations is an empty object', async () => { const oldStorage = { meta: { diff --git a/app/scripts/migrations/083.ts b/app/scripts/migrations/083.ts index cc3e3b16b..bd7ba62e4 100644 --- a/app/scripts/migrations/083.ts +++ b/app/scripts/migrations/083.ts @@ -25,11 +25,21 @@ export async function migrate(originalVersionedData: { function transformState(state: Record) { if (!isObject(state.NetworkController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); return state; } const { NetworkController } = state; if (!isObject(NetworkController.networkConfigurations)) { + global.sentry?.captureException?.( + new Error( + `typeof NetworkController.networkConfigurations is ${typeof NetworkController.networkConfigurations}`, + ), + ); return state; } diff --git a/app/scripts/migrations/084.test.js b/app/scripts/migrations/084.test.js index 138bfacb6..bd0e1b35c 100644 --- a/app/scripts/migrations/084.test.js +++ b/app/scripts/migrations/084.test.js @@ -1,6 +1,16 @@ import { migrate } from './084'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + describe('migration #84', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('updates the version metadata', async () => { const originalVersionedData = buildOriginalVersionedData({ meta: { @@ -27,6 +37,21 @@ describe('migration #84', () => { expect(newVersionedData.data).toStrictEqual(originalVersionedData.data); }); + it('captures an exception if the network controller state does not exist', async () => { + const originalVersionedData = buildOriginalVersionedData({ + data: { + test: '123', + }, + }); + + await migrate(originalVersionedData); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is undefined`), + ); + }); + const nonObjects = [undefined, null, 'test', 1, ['test']]; for (const invalidState of nonObjects) { it(`does not change the state if the network controller state is ${invalidState}`, async () => { @@ -40,6 +65,21 @@ describe('migration #84', () => { expect(newVersionedData.data).toStrictEqual(originalVersionedData.data); }); + + it(`captures an exception if the network controller state is ${invalidState}`, async () => { + const originalVersionedData = buildOriginalVersionedData({ + data: { + NetworkController: invalidState, + }, + }); + + await migrate(originalVersionedData); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is ${typeof invalidState}`), + ); + }); } it('does not change the state if the network controller state does not include "network"', async () => { @@ -56,6 +96,38 @@ describe('migration #84', () => { expect(newVersionedData.data).toStrictEqual(originalVersionedData.data); }); + it('captures an exception if the network controller state does not include "network" and does not include "networkId"', async () => { + const originalVersionedData = buildOriginalVersionedData({ + data: { + NetworkController: { + test: '123', + }, + }, + }); + + await migrate(originalVersionedData); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController.network is undefined`), + ); + }); + + it('does not capture an exception if the network controller state does not include "network" but does include "networkId"', async () => { + const originalVersionedData = buildOriginalVersionedData({ + data: { + NetworkController: { + test: '123', + networkId: 'foobar', + }, + }, + }); + + await migrate(originalVersionedData); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(0); + }); + it('replaces "network" in the network controller state with "networkId": null, "networkStatus": "unknown" if it is "loading"', async () => { const originalVersionedData = buildOriginalVersionedData({ data: { diff --git a/app/scripts/migrations/084.ts b/app/scripts/migrations/084.ts index 66a2f45ae..7551ed2c6 100644 --- a/app/scripts/migrations/084.ts +++ b/app/scripts/migrations/084.ts @@ -25,9 +25,26 @@ export async function migrate(originalVersionedData: { function transformState(state: Record) { if ( !hasProperty(state, 'NetworkController') || - !isObject(state.NetworkController) || - !hasProperty(state.NetworkController, 'network') + !isObject(state.NetworkController) ) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); + return state; + } + if (!hasProperty(state.NetworkController, 'network')) { + const thePost077SupplementFor084HasNotModifiedState = + state.NetworkController.networkId === undefined; + if (thePost077SupplementFor084HasNotModifiedState) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController.network is ${typeof state + .NetworkController.network}`, + ), + ); + } return state; } diff --git a/app/scripts/migrations/085.test.js b/app/scripts/migrations/085.test.js index 6b7b4967d..cfe7ff1b0 100644 --- a/app/scripts/migrations/085.test.js +++ b/app/scripts/migrations/085.test.js @@ -1,5 +1,11 @@ import { migrate, version } from './085'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + jest.mock('uuid', () => { const actual = jest.requireActual('uuid'); @@ -10,6 +16,10 @@ jest.mock('uuid', () => { }); describe('migration #85', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('should update the version metadata', async () => { const oldStorage = { meta: { @@ -39,6 +49,25 @@ describe('migration #85', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should capture an exception there is no network controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: 84, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is undefined`), + ); + }); + it('should return state unaltered if there is no network controller previous provider state', async () => { const oldData = { other: 'data', diff --git a/app/scripts/migrations/085.ts b/app/scripts/migrations/085.ts index 03499d2b2..2ba22a346 100644 --- a/app/scripts/migrations/085.ts +++ b/app/scripts/migrations/085.ts @@ -24,6 +24,11 @@ export async function migrate(originalVersionedData: { function transformState(state: Record) { if (!isObject(state.NetworkController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); return state; } diff --git a/app/scripts/migrations/086.test.js b/app/scripts/migrations/086.test.js index f38f0444d..cd6a61377 100644 --- a/app/scripts/migrations/086.test.js +++ b/app/scripts/migrations/086.test.js @@ -1,5 +1,11 @@ import { migrate, version } from './086'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + jest.mock('uuid', () => { const actual = jest.requireActual('uuid'); @@ -10,6 +16,10 @@ jest.mock('uuid', () => { }); describe('migration #86', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('should update the version metadata', async () => { const oldStorage = { meta: { @@ -39,6 +49,25 @@ describe('migration #86', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should capture an exception if there is no network controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: 85, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is undefined`), + ); + }); + it('should return state unaltered if there is no network controller provider state', async () => { const oldData = { other: 'data', @@ -59,6 +88,52 @@ describe('migration #86', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should capture an exception if there is no network controller provider state and no providerConfig state', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + foo: 'bar', + }, + }, + }; + const oldStorage = { + meta: { + version: 85, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController.provider is undefined`), + ); + }); + + it('should not capture an exception if there is no network controller provider state but there is a providerConfig state', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + foo: 'bar', + }, + providerConfig: {}, + }, + }; + const oldStorage = { + meta: { + version: 85, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(0); + }); + it('should rename the provider config state', async () => { const oldData = { other: 'data', diff --git a/app/scripts/migrations/086.ts b/app/scripts/migrations/086.ts index c4b60e270..709934f50 100644 --- a/app/scripts/migrations/086.ts +++ b/app/scripts/migrations/086.ts @@ -37,5 +37,24 @@ function transformState(state: Record) { NetworkController: networkControllerState, }; } + if (!isObject(state.NetworkController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); + } else if (!hasProperty(state.NetworkController, 'provider')) { + const thePost077SupplementFor086HasNotModifiedState = + state.NetworkController.providerConfig === undefined; + if (thePost077SupplementFor086HasNotModifiedState) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController.provider is ${typeof state + .NetworkController.provider}`, + ), + ); + } + } + return state; } diff --git a/app/scripts/migrations/087.test.js b/app/scripts/migrations/087.test.js index 1d631e926..ebd9495ad 100644 --- a/app/scripts/migrations/087.test.js +++ b/app/scripts/migrations/087.test.js @@ -1,6 +1,16 @@ import { migrate, version } from './087'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + describe('migration #87', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('should update the version metadata', async () => { const oldStorage = { meta: { @@ -53,6 +63,65 @@ describe('migration #87', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should return state unaltered if TokensController state is not an object', async () => { + const oldData = { + other: 'data', + TokensController: false, + }; + const oldStorage = { + meta: { + version: 86, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should capture an exception if TokensController state is not an object', async () => { + const oldData = { + other: 'data', + TokensController: false, + }; + const oldStorage = { + meta: { + version: 86, + }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokensController is boolean`), + ); + }); + + it('should not capture an exception if TokensController state is an object', async () => { + const oldData = { + other: 'data', + TokensController: { + allDetectedTokens: {}, + allIgnoredTokens: {}, + allTokens: {}, + detectedTokens: [], + ignoredTokens: [], + suggestedAssets: [], + tokens: [], + }, + }; + const oldStorage = { + meta: { + version: 86, + }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(0); + }); + it('should remove the suggested assets state', async () => { const oldData = { other: 'data', diff --git a/app/scripts/migrations/087.ts b/app/scripts/migrations/087.ts index 92093f967..f900faab6 100644 --- a/app/scripts/migrations/087.ts +++ b/app/scripts/migrations/087.ts @@ -24,6 +24,11 @@ export async function migrate(originalVersionedData: { function transformState(state: Record) { if (!isObject(state.TokensController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.TokensController is ${typeof state.TokensController}`, + ), + ); return state; } diff --git a/app/scripts/migrations/088.test.ts b/app/scripts/migrations/088.test.ts index a33c30eff..ca672f982 100644 --- a/app/scripts/migrations/088.test.ts +++ b/app/scripts/migrations/088.test.ts @@ -1,6 +1,16 @@ import { migrate } from './088'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + describe('migration #88', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('updates the version metadata', async () => { const oldStorage = { meta: { version: 87 }, @@ -26,6 +36,24 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if the NftController property is not an object', async () => { + const oldData = { + TokenListController: {}, + TokensController: {}, + NftController: false, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NftController is boolean`), + ); + }); + it('returns the state unaltered if the NftController object has no allNftContracts property', async () => { const oldData = { NftController: { @@ -58,6 +86,26 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if it NftController.allNftContracts is not an object', async () => { + const oldData = { + TokenListController: {}, + TokensController: {}, + NftController: { + allNftContracts: 'foo', + }, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NftController.allNftContracts is string`), + ); + }); + it('returns the state unaltered if any value of the NftController.allNftContracts object is not an object itself', async () => { const oldData = { NftController: { @@ -324,6 +372,26 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if it NftController.allNfts is not an object', async () => { + const oldData = { + TokenListController: {}, + TokensController: {}, + NftController: { + allNfts: 'foo', + }, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NftController.allNfts is string`), + ); + }); + it('returns the state unaltered if any value of the NftController.allNfts object is not an object itself', async () => { const oldData = { NftController: { @@ -656,6 +724,91 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if it has no TokenListController property', async () => { + const oldData = { + TokensController: {}, + NftController: { + allNfts: { + '0x111': { + '0x10': [ + { + name: 'NFT 1', + description: 'Description for NFT 1', + image: 'nft1.jpg', + standard: 'ERC721', + tokenId: '1', + address: '0xaaa', + }, + ], + }, + }, + allNftContracts: { + '0x111': { + '0x10': [ + { + name: 'Contract 1', + address: '0xaaa', + }, + ], + }, + }, + }, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokenListController is undefined`), + ); + }); + + it('captures an exception if the TokenListController property is not an object', async () => { + const oldData = { + TokensController: {}, + NftController: { + allNfts: { + '0x111': { + '0x10': [ + { + name: 'NFT 1', + description: 'Description for NFT 1', + image: 'nft1.jpg', + standard: 'ERC721', + tokenId: '1', + address: '0xaaa', + }, + ], + }, + }, + allNftContracts: { + '0x111': { + '0x10': [ + { + name: 'Contract 1', + address: '0xaaa', + }, + ], + }, + }, + }, + TokenListController: false, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokenListController is boolean`), + ); + }); + it('returns the state unaltered if the TokenListController object has no tokensChainsCache property', async () => { const oldData = { TokenListController: { @@ -688,6 +841,25 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if the TokenListController.tokensChainsCache property is not an object', async () => { + const oldData = { + TokenListController: { + tokensChainsCache: 'foo', + }, + TokensController: {}, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokenListController.tokensChainsCache is string`), + ); + }); + it('rewrites TokenListController.tokensChainsCache so that decimal chain IDs are converted to hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -919,6 +1091,39 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if it has no TokensController property', async () => { + const oldData = { + TokenListController: {}, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokensController is undefined`), + ); + }); + + it('captures an exception if the TokensController property is not an object', async () => { + const oldData = { + TokenListController: {}, + TokensController: false, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokensController is boolean`), + ); + }); + it('returns the state unaltered if the TokensController object has no allTokens property', async () => { const oldData = { TokensController: { @@ -951,6 +1156,25 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if the TokensController.allTokens property is not an object', async () => { + const oldData = { + TokenListController: {}, + TokensController: { + allTokens: 'foo', + }, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokensController.allTokens is string`), + ); + }); + it('rewrites TokensController.allTokens so that decimal chain IDs are converted to hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -1163,6 +1387,25 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if the TokensController.allIgnoredTokens property is not an object', async () => { + const oldData = { + TokenListController: {}, + TokensController: { + allIgnoredTokens: 'foo', + }, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokensController.allIgnoredTokens is string`), + ); + }); + it('rewrites TokensController.allIgnoredTokens so that decimal chain IDs are converted to hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -1323,6 +1566,25 @@ describe('migration #88', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if the TokensController.allDetectedTokens property is not an object', async () => { + const oldData = { + TokenListController: {}, + TokensController: { + allDetectedTokens: 'foo', + }, + }; + const oldStorage = { + meta: { version: 87 }, + data: oldData, + }; + + await migrate(oldStorage); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokensController.allDetectedTokens is string`), + ); + }); + it('rewrites TokensController.allDetectedTokens so that decimal chain IDs are converted to hex strings', async () => { const oldStorage = { meta: { version: 87 }, diff --git a/app/scripts/migrations/088.ts b/app/scripts/migrations/088.ts index a4b874b2f..5ede1b0fa 100644 --- a/app/scripts/migrations/088.ts +++ b/app/scripts/migrations/088.ts @@ -1,6 +1,7 @@ import { hasProperty, Hex, isObject, isStrictHexString } from '@metamask/utils'; import { BN } from 'ethereumjs-util'; import { cloneDeep, mapKeys } from 'lodash'; +import log from 'loglevel'; type VersionedData = { meta: { version: number }; @@ -70,6 +71,16 @@ function migrateData(state: Record): void { } }); } + } else if (hasProperty(nftControllerState, 'allNftContracts')) { + global.sentry?.captureException?.( + new Error( + `typeof state.NftController.allNftContracts is ${typeof nftControllerState.allNftContracts}`, + ), + ); + } else { + log.warn( + `typeof state.NftController.allNftContracts is ${typeof nftControllerState.allNftContracts}`, + ); } // Migrate NftController.allNfts @@ -96,9 +107,25 @@ function migrateData(state: Record): void { } }); } + } else if (hasProperty(nftControllerState, 'allNfts')) { + global.sentry?.captureException?.( + new Error( + `typeof state.NftController.allNfts is ${typeof nftControllerState.allNfts}`, + ), + ); + } else { + log.warn( + `typeof state.NftController.allNfts is ${typeof nftControllerState.allNfts}`, + ); } state.NftController = nftControllerState; + } else if (hasProperty(state, 'NftController')) { + global.sentry?.captureException?.( + new Error(`typeof state.NftController is ${typeof state.NftController}`), + ); + } else { + log.warn(`typeof state.NftController is undefined`); } if ( @@ -124,7 +151,24 @@ function migrateData(state: Record): void { tokenListControllerState.tokensChainsCache, (_, chainId: string) => toHex(chainId), ); + } else if (hasProperty(tokenListControllerState, 'tokensChainsCache')) { + global.sentry?.captureException?.( + new Error( + `typeof state.TokenListController.tokensChainsCache is ${typeof state + .TokenListController.tokensChainsCache}`, + ), + ); + } else { + log.warn( + `typeof state.TokenListController.tokensChainsCache is undefined`, + ); } + } else { + global.sentry?.captureException?.( + new Error( + `typeof state.TokenListController is ${typeof state.TokenListController}`, + ), + ); } if ( @@ -150,6 +194,16 @@ function migrateData(state: Record): void { allTokens, (_, chainId: string) => toHex(chainId), ); + } else if (hasProperty(tokensControllerState, 'allTokens')) { + global.sentry?.captureException?.( + new Error( + `typeof state.TokensController.allTokens is ${typeof tokensControllerState.allTokens}`, + ), + ); + } else { + log.warn( + `typeof state.TokensController.allTokens is ${typeof tokensControllerState.allTokens}`, + ); } // Migrate TokensController.allIgnoredTokens @@ -169,6 +223,16 @@ function migrateData(state: Record): void { allIgnoredTokens, (_, chainId: string) => toHex(chainId), ); + } else if (hasProperty(tokensControllerState, 'allIgnoredTokens')) { + global.sentry?.captureException?.( + new Error( + `typeof state.TokensController.allIgnoredTokens is ${typeof tokensControllerState.allIgnoredTokens}`, + ), + ); + } else { + log.warn( + `typeof state.TokensController.allIgnoredTokens is ${typeof tokensControllerState.allIgnoredTokens}`, + ); } // Migrate TokensController.allDetectedTokens @@ -188,9 +252,25 @@ function migrateData(state: Record): void { allDetectedTokens, (_, chainId: string) => toHex(chainId), ); + } else if (hasProperty(tokensControllerState, 'allDetectedTokens')) { + global.sentry?.captureException?.( + new Error( + `typeof state.TokensController.allDetectedTokens is ${typeof tokensControllerState.allDetectedTokens}`, + ), + ); + } else { + log.warn( + `typeof state.TokensController.allDetectedTokens is ${typeof tokensControllerState.allDetectedTokens}`, + ); } state.TokensController = tokensControllerState; + } else { + global.sentry?.captureException?.( + new Error( + `typeof state.TokensController is ${typeof state.TokensController}`, + ), + ); } } diff --git a/app/scripts/migrations/089.test.ts b/app/scripts/migrations/089.test.ts index 00868ff74..91511d315 100644 --- a/app/scripts/migrations/089.test.ts +++ b/app/scripts/migrations/089.test.ts @@ -1,5 +1,14 @@ import { migrate, version } from './089'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + startSession: jest.fn(), + endSession: jest.fn(), + toggleSession: jest.fn(), + captureException: sentryCaptureExceptionMock, +}; + jest.mock('uuid', () => { const actual = jest.requireActual('uuid'); @@ -10,6 +19,10 @@ jest.mock('uuid', () => { }); describe('migration #89', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('should update the version metadata', async () => { const oldStorage = { meta: { @@ -39,6 +52,25 @@ describe('migration #89', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should capture an exception if there is no network controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is undefined`), + ); + }); + it('should return state unaltered if there is no network controller providerConfig state', async () => { const oldData = { other: 'data', @@ -61,6 +93,32 @@ describe('migration #89', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should capture an exception if there is no network controller providerConfig state', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + }, + }, + }, + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController.providerConfig is undefined`), + ); + }); + it('should return state unaltered if the providerConfig already has an id', async () => { const oldData = { other: 'data', diff --git a/app/scripts/migrations/089.ts b/app/scripts/migrations/089.ts index cc1bfa4dc..d4faebc22 100644 --- a/app/scripts/migrations/089.ts +++ b/app/scripts/migrations/089.ts @@ -66,6 +66,19 @@ function transformState(state: Record) { ...state, NetworkController: state.NetworkController, }; + } else if (!isObject(state.NetworkController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); + } else if (!isObject(state.NetworkController.providerConfig)) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController.providerConfig is ${typeof state + .NetworkController.providerConfig}`, + ), + ); } return state; } diff --git a/app/scripts/migrations/090.test.js b/app/scripts/migrations/090.test.js index 6a28c60f2..13e39e648 100644 --- a/app/scripts/migrations/090.test.js +++ b/app/scripts/migrations/090.test.js @@ -2,7 +2,17 @@ import { migrate, version } from './090'; const PREVIOUS_VERSION = version - 1; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, +}; + describe('migration #90', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('updates the version metadata', async () => { const oldStorage = { meta: { @@ -31,7 +41,7 @@ describe('migration #90', () => { expect(newStorage.data).toStrictEqual(oldStorage.data); }); - it('does not change the state if the phishing controller state is invalid', async () => { + it('captures an exception if the phishing controller state is invalid', async () => { const oldStorage = { meta: { version: PREVIOUS_VERSION, @@ -39,9 +49,12 @@ describe('migration #90', () => { data: { PhishingController: 'this is not valid' }, }; - const newStorage = await migrate(oldStorage); + await migrate(oldStorage); - expect(newStorage.data).toStrictEqual(oldStorage.data); + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.PhishingController is string`), + ); }); it('does not change the state if the listState property does not exist', async () => { diff --git a/app/scripts/migrations/090.ts b/app/scripts/migrations/090.ts index e45ec05e4..a2d50cab6 100644 --- a/app/scripts/migrations/090.ts +++ b/app/scripts/migrations/090.ts @@ -1,5 +1,6 @@ import { cloneDeep } from 'lodash'; import { hasProperty, isObject } from '@metamask/utils'; +import log from 'loglevel'; export const version = 90; @@ -23,11 +24,22 @@ export async function migrate(originalVersionedData: { } function transformState(state: Record) { - if ( - !hasProperty(state, 'PhishingController') || - !isObject(state.PhishingController) || - !hasProperty(state.PhishingController, 'listState') - ) { + if (!hasProperty(state, 'PhishingController')) { + log.warn(`typeof state.PhishingController is undefined`); + return state; + } + if (!isObject(state.PhishingController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.PhishingController is ${typeof state.PhishingController}`, + ), + ); + return state; + } + if (!hasProperty(state.PhishingController, 'listState')) { + log.warn( + `typeof state.PhishingController.listState is ${typeof state.PhishingController}`, + ); return state; } diff --git a/app/scripts/migrations/091.test.ts b/app/scripts/migrations/091.test.ts index d4836f003..6a1f14ed7 100644 --- a/app/scripts/migrations/091.test.ts +++ b/app/scripts/migrations/091.test.ts @@ -1,6 +1,15 @@ import { cloneDeep } from 'lodash'; import { migrate, version } from './091'; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + startSession: jest.fn(), + endSession: jest.fn(), + toggleSession: jest.fn(), + captureException: sentryCaptureExceptionMock, +}; + jest.mock('uuid', () => { const actual = jest.requireActual('uuid'); @@ -11,6 +20,10 @@ jest.mock('uuid', () => { }); describe('migration #91', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('should update the version metadata', async () => { const oldStorage = { meta: { @@ -40,6 +53,25 @@ describe('migration #91', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should capture an exception if there is no network controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: 90, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.NetworkController is undefined`), + ); + }); + it('should return state unaltered if there is no network controller networkConfigurations state', async () => { const oldData = { other: 'data', @@ -60,6 +92,32 @@ describe('migration #91', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('should capture an exception if there is no network controller networkConfigurations state', async () => { + const oldData = { + other: 'data', + NetworkController: { + providerConfig: { + foo: 'bar', + }, + }, + }; + const oldStorage = { + meta: { + version: 90, + }, + data: oldData, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error( + `typeof state.NetworkController.networkConfigurations is undefined`, + ), + ); + }); + it('should return state unaltered if the networkConfigurations all have a chainId', async () => { const oldData = { other: 'data', diff --git a/app/scripts/migrations/091.ts b/app/scripts/migrations/091.ts index c0661746a..874f6ed9f 100644 --- a/app/scripts/migrations/091.ts +++ b/app/scripts/migrations/091.ts @@ -50,6 +50,19 @@ function transformState(state: Record) { ...state, NetworkController: state.NetworkController, }; + } else if (!isObject(state.NetworkController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); + } else if (!isObject(state.NetworkController.networkConfigurations)) { + global.sentry?.captureException?.( + new Error( + `typeof state.NetworkController.networkConfigurations is ${typeof state + .NetworkController.networkConfigurations}`, + ), + ); } return state; } diff --git a/app/scripts/migrations/092.1.test.ts b/app/scripts/migrations/092.1.test.ts new file mode 100644 index 000000000..7ab1045ca --- /dev/null +++ b/app/scripts/migrations/092.1.test.ts @@ -0,0 +1,139 @@ +import { cloneDeep } from 'lodash'; +import { migrate, version } from './092.1'; + +const PREVIOUS_VERSION = 92; + +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + startSession: jest.fn(), + endSession: jest.fn(), + toggleSession: jest.fn(), + captureException: sentryCaptureExceptionMock, +}; + +describe('migration #92.1', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should update the version metadata', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: {}, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.meta).toStrictEqual({ + version, + }); + }); + + it('should return state unaltered if there is no TokenListController state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('captures an exception if the TokenListController state is invalid', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { TokenListController: 'this is not valid' }, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.TokenListController is string`), + ); + }); + + it('should return state unaltered if there is no TokenListController tokensChainsCache state', async () => { + const oldData = { + other: 'data', + TokenListController: { + tokenList: {}, + }, + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if the tokensChainsCache state is an unexpected type', async () => { + const oldData = { + other: 'data', + TokenListController: { + tokensChainsCache: 'unexpected string', + }, + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if the tokensChainsCache state is valid', async () => { + const oldData = { + other: 'data', + TokenListController: { + tokensChainsCache: {}, + }, + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should remove undefined tokensChainsCache state', async () => { + const oldData = { + other: 'data', + TokenListController: { + tokensChainsCache: undefined, + }, + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual({ + other: 'data', + TokenListController: {}, + }); + }); +}); diff --git a/app/scripts/migrations/092.1.ts b/app/scripts/migrations/092.1.ts new file mode 100644 index 000000000..62b47fb04 --- /dev/null +++ b/app/scripts/migrations/092.1.ts @@ -0,0 +1,53 @@ +import { cloneDeep } from 'lodash'; +import { hasProperty, isObject } from '@metamask/utils'; +import log from 'loglevel'; + +export const version = 92.1; + +/** + * Check whether the `TokenListController.tokensChainsCache` state is + * `undefined`, and delete it if so. + * + * This property was accidentally set to `undefined` by an earlier revision of + * migration #77 in some cases. + * + * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist. + * @param originalVersionedData.meta - State metadata. + * @param originalVersionedData.meta.version - The current state version. + * @param originalVersionedData.data - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ +export async function migrate(originalVersionedData: { + meta: { version: number }; + data: Record; +}) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + versionedData.data = transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record) { + if (!hasProperty(state, 'TokenListController')) { + log.warn('Skipping migration, TokenListController state is missing'); + return state; + } else if (!isObject(state.TokenListController)) { + global.sentry?.captureException?.( + new Error( + `typeof state.TokenListController is ${typeof state.TokenListController}`, + ), + ); + return state; + } else if (!hasProperty(state.TokenListController, 'tokensChainsCache')) { + log.warn( + 'Skipping migration, TokenListController.tokensChainsCache state is missing', + ); + return state; + } + + if (state.TokenListController.tokensChainsCache === undefined) { + delete state.TokenListController.tokensChainsCache; + } + + return state; +} diff --git a/app/scripts/migrations/092.test.ts b/app/scripts/migrations/092.test.ts index b44c04602..b7337c871 100644 --- a/app/scripts/migrations/092.test.ts +++ b/app/scripts/migrations/092.test.ts @@ -3,7 +3,20 @@ import { migrate, version } from './092'; const PREVIOUS_VERSION = version - 1; +const sentryCaptureExceptionMock = jest.fn(); + +global.sentry = { + startSession: jest.fn(), + endSession: jest.fn(), + toggleSession: jest.fn(), + captureException: sentryCaptureExceptionMock, +}; + describe('migration #92', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + it('should update the version metadata', async () => { const oldStorage = { meta: { @@ -33,6 +46,22 @@ describe('migration #92', () => { expect(newStorage.data).toStrictEqual(oldData); }); + it('captures an exception if the phishing controller state is invalid', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { PhishingController: 'this is not valid' }, + }; + + await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledTimes(1); + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(`typeof state.PhishingController is string`), + ); + }); + it('should return state unaltered if there is no phishing controller last fetched state', async () => { const oldData = { other: 'data', diff --git a/app/scripts/migrations/092.ts b/app/scripts/migrations/092.ts index bf5469614..44ef2a6c7 100644 --- a/app/scripts/migrations/092.ts +++ b/app/scripts/migrations/092.ts @@ -1,5 +1,6 @@ import { cloneDeep } from 'lodash'; import { hasProperty, isObject } from '@metamask/utils'; +import log from 'loglevel'; export const version = 92; @@ -30,6 +31,14 @@ function transformState(state: Record) { ) { delete state.PhishingController.stalelistLastFetched; delete state.PhishingController.hotlistLastFetched; + } else if (hasProperty(state, 'PhishingController')) { + global.sentry?.captureException?.( + new Error( + `typeof state.PhishingController is ${typeof state.PhishingController}`, + ), + ); + } else { + log.warn(`typeof state.PhishingController is undefined`); } return state; } diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index dc4fcd4e0..f5c169a1f 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -96,6 +96,7 @@ import * as m089 from './089'; import * as m090 from './090'; import * as m091 from './091'; import * as m092 from './092'; +import * as m092point1 from './092.1'; const migrations = [ m002, @@ -189,6 +190,7 @@ const migrations = [ m090, m091, m092, + m092point1, ]; export default migrations; diff --git a/app/scripts/ui.js b/app/scripts/ui.js index 565e8187a..6f923ce39 100644 --- a/app/scripts/ui.js +++ b/app/scripts/ui.js @@ -1,13 +1,15 @@ +// Disabled to allow setting up initial state hooks first + +// This import sets up global functions required for Sentry to function. +// It must be run first in case an error is thrown later during initialization. +import './lib/setup-initial-state-hooks'; + // polyfills import '@formatjs/intl-relativetimeformat/polyfill'; // 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/package.json b/package.json index 3dce2bbf4..5c7f6277b 100644 --- a/package.json +++ b/package.json @@ -354,7 +354,7 @@ "redux-thunk": "^2.3.0", "remove-trailing-slash": "^0.1.1", "reselect": "^3.0.1", - "ses": "^0.18.4", + "ses": "^0.18.7", "single-call-balance-checker-abi": "^1.0.0", "unicode-confusables": "^0.1.1", "uuid": "^8.3.2", diff --git a/shared/modules/object.utils.js b/shared/modules/object.utils.js index 24119be56..ce7eb1da1 100644 --- a/shared/modules/object.utils.js +++ b/shared/modules/object.utils.js @@ -7,6 +7,8 @@ * should be included, and a sub-mask implies the property should be further * masked according to that sub-mask. * + * If a property is not found in the last, its type is included instead. + * * @param {object} object - The object to mask * @param {Object} mask - The mask to apply to the object */ @@ -16,6 +18,8 @@ export function maskObject(object, mask) { state[key] = object[key]; } else if (mask[key]) { state[key] = maskObject(object[key], mask[key]); + } else { + state[key] = typeof object[key]; } return state; }, {}); diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index ed3e3aaac..3528ec5aa 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -724,6 +724,13 @@ class FixtureBuilder { return this; } + withBadPreferencesControllerState() { + merge(this.fixture.data, { + PreferencesController: 5, + }); + return this; + } + withTokensControllerERC20() { merge(this.fixture.data.TokensController, { tokens: [ diff --git a/test/e2e/tests/errors.spec.js b/test/e2e/tests/errors.spec.js index 960135215..5c557b256 100644 --- a/test/e2e/tests/errors.spec.js +++ b/test/e2e/tests/errors.spec.js @@ -1,8 +1,112 @@ +const { resolve } = require('path'); +const { promises: fs } = require('fs'); const { strict: assert } = require('assert'); +const { get, has, set, unset } = require('lodash'); const { Browser } = require('selenium-webdriver'); +const { format } = require('prettier'); const { convertToHexValue, withFixtures } = require('../helpers'); const FixtureBuilder = require('../fixture-builder'); +const maskedBackgroundFields = [ + 'CurrencyController.conversionDate', // This is a timestamp that changes each run +]; +const maskedUiFields = [ + 'metamask.conversionDate', // This is a timestamp that changes each run +]; + +const removedBackgroundFields = [ + // This property is timing-dependent + 'AccountTracker.currentBlockGasLimit', + // These properties are set to undefined, causing inconsistencies between Chrome and Firefox + 'AppStateController.currentPopupId', + 'AppStateController.timeoutMinutes', +]; + +const removedUiFields = [ + // This property is timing-dependent + 'metamask.currentBlockGasLimit', + // These properties are set to undefined, causing inconsistencies between Chrome and Firefox + 'metamask.currentPopupId', + 'metamask.timeoutMinutes', +]; + +/** + * Transform background state to make it consistent between test runs. + * + * @param {unknown} data - The data to transform + */ +function transformBackgroundState(data) { + for (const field of maskedBackgroundFields) { + if (has(data, field)) { + set(data, field, typeof get(data, field)); + } + } + for (const field of removedBackgroundFields) { + if (has(data, field)) { + unset(data, field); + } + } + return data; +} + +/** + * Transform UI state to make it consistent between test runs. + * + * @param {unknown} data - The data to transform + */ +function transformUiState(data) { + for (const field of maskedUiFields) { + if (has(data, field)) { + set(data, field, typeof get(data, field)); + } + } + for (const field of removedUiFields) { + if (has(data, field)) { + unset(data, field); + } + } + return data; +} + +/** + * Check that the data provided matches the snapshot. + * + * @param {object }args - Function arguments. + * @param {any} args.data - The data to compare with the snapshot. + * @param {string} args.snapshot - The name of the snapshot. + * @param {boolean} [args.update] - Whether to update the snapshot if it doesn't match. + */ +async function matchesSnapshot({ + data, + snapshot, + update = process.env.UPDATE_SNAPSHOTS === 'true', +}) { + const snapshotPath = resolve(__dirname, `./state-snapshots/${snapshot}.json`); + const rawSnapshotData = await fs.readFile(snapshotPath, { + encoding: 'utf-8', + }); + const snapshotData = JSON.parse(rawSnapshotData); + + try { + assert.deepStrictEqual(data, snapshotData); + } catch (error) { + if (update && error instanceof assert.AssertionError) { + const stringifiedData = JSON.stringify(data); + // filepath specified so that Prettier can infer which parser to use + // from the file extension + const formattedData = format(stringifiedData, { + filepath: 'something.json', + }); + await fs.writeFile(snapshotPath, formattedData, { + encoding: 'utf-8', + }); + console.log(`Snapshot '${snapshot}' updated`); + return; + } + throw error; + } +} + describe('Sentry errors', function () { const migrationError = process.env.SELENIUM_BROWSER === Browser.CHROME @@ -20,6 +124,18 @@ describe('Sentry errors', function () { }); } + async function mockSentryInvariantMigrationError(mockServer) { + return await mockServer + .forPost('https://sentry.io/api/0000000/envelope/') + .withBodyIncluding('typeof state.PreferencesController is number') + .thenCallback(() => { + return { + statusCode: 200, + json: {}, + }; + }); + } + async function mockSentryTestError(mockServer) { return await mockServer .forPost('https://sentry.io/api/0000000/envelope/') @@ -41,8 +157,8 @@ describe('Sentry errors', function () { ], }; - describe('before initialization', function () { - it('should NOT send error events when participateInMetaMetrics is false', async function () { + describe('before initialization, after opting out of metrics', function () { + it('should NOT send error events in the background', async function () { await withFixtures( { fixtures: { @@ -73,7 +189,43 @@ describe('Sentry errors', function () { }, ); }); - it('should send error events', async function () { + + it('should NOT send error events in the UI', 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.findElement('#password'); + // Erase `getSentryAppState` hook, simulating a "before initialization" state + await driver.executeScript( + 'window.stateHooks.getSentryAppState = undefined', + ); + + // Wait for Sentry request + await driver.delay(3000); + const isPending = await mockedEndpoint.isPending(); + assert.ok( + isPending, + 'A request to sentry was sent when it should not have been', + ); + }, + ); + }); + }); + + describe('before initialization, after opting into metrics', function () { + it('should send error events in background', async function () { await withFixtures( { fixtures: { @@ -112,40 +264,110 @@ describe('Sentry errors', function () { }, ); }); - }); - describe('after initialization', function () { - it('should NOT send error events when participateInMetaMetrics is false', async function () { + it('should capture background application state', async function () { await withFixtures( { - fixtures: new FixtureBuilder() - .withMetaMetricsController({ - metaMetricsId: null, - participateInMetaMetrics: false, - }) - .build(), + 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: mockSentryTestError, + testSpecificMock: mockSentryMigratorError, }, 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(); + 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 appState = mockJsonBody?.extra?.appState; + assert.deepStrictEqual(Object.keys(appState), [ + 'browser', + 'version', + 'persistedState', + ]); assert.ok( - isPending, - 'A request to sentry was sent when it should not have been', + typeof appState?.browser === 'string' && + appState?.browser.length > 0, + 'Invalid browser state', ); + assert.ok( + typeof appState?.version === 'string' && + appState?.version.length > 0, + 'Invalid version state', + ); + await matchesSnapshot({ + data: transformBackgroundState(appState.persistedState), + snapshot: 'errors-before-init-opt-in-background-state', + }); }, ); }); - it('should send error events', async function () { + + it('should capture migration log breadcrumbs when there is an invariant state error in a migration', async function () { + await withFixtures( + { + fixtures: { + ...new FixtureBuilder() + .withMetaMetricsController({ + metaMetricsId: 'fake-metrics-id', + participateInMetaMetrics: true, + }) + .withBadPreferencesControllerState() + .build(), + }, + ganacheOptions, + title: this.test.title, + failOnConsoleError: false, + testSpecificMock: mockSentryInvariantMigrationError, + }, + 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 breadcrumbs = mockJsonBody?.breadcrumbs ?? []; + const migrationLogBreadcrumbs = breadcrumbs.filter((breadcrumb) => { + return breadcrumb.message?.match(/Running migration \d+/u); + }); + const migrationLogMessages = migrationLogBreadcrumbs.map( + (breadcrumb) => + breadcrumb.message.match(/(Running migration \d+)/u)[1], + ); + const firstMigrationLog = migrationLogMessages[0]; + const lastMigrationLog = + migrationLogMessages[migrationLogMessages.length - 1]; + + assert.equal(migrationLogMessages.length, 8); + assert.equal(firstMigrationLog, 'Running migration 75'); + assert.equal(lastMigrationLog, 'Running migration 82'); + }, + ); + }); + + it('should send error events in UI', async function () { await withFixtures( { fixtures: new FixtureBuilder() @@ -161,21 +383,193 @@ describe('Sentry errors', function () { }, async ({ driver, mockedEndpoint }) => { await driver.navigate(); - await driver.fill('#password', 'correct horse battery staple'); - await driver.press('#password', driver.Key.ENTER); + await driver.findElement('#password'); + // Erase `getSentryAppState` hook, simulating a "before initialization" state + await driver.executeScript( + 'window.stateHooks.getSentryAppState = undefined', + ); + // Trigger error - driver.executeScript('window.stateHooks.throwTestError()'); + await driver.executeScript('window.stateHooks.throwTestError()'); + // Wait for Sentry request await driver.wait(async () => { const isPending = await mockedEndpoint.isPending(); return isPending === false; - }, 10000); + }, 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, 'TestError'); + assert.equal(value, 'Test Error'); + assert.equal(level, 'error'); + }, + ); + }); + + it('should capture UI application state', 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.findElement('#password'); + // Erase `getSentryAppState` hook, simulating a "before initialization" state + await driver.executeScript( + 'window.stateHooks.getSentryAppState = undefined', + ); + + // Trigger error + await driver.executeScript('window.stateHooks.throwTestError()'); + + // 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 appState = mockJsonBody?.extra?.appState; + assert.deepStrictEqual(Object.keys(appState), [ + 'browser', + 'version', + 'persistedState', + ]); + assert.ok( + typeof appState?.browser === 'string' && + appState?.browser.length > 0, + 'Invalid browser state', + ); + assert.ok( + typeof appState?.version === 'string' && + appState?.version.length > 0, + 'Invalid version state', + ); + await matchesSnapshot({ + data: transformBackgroundState(appState.persistedState), + snapshot: 'errors-before-init-opt-in-ui-state', + }); + }, + ); + }); + }); + + describe('after initialization, after opting out of metrics', function () { + it('should NOT send error events in the background', 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.findElement('#password'); + + // Trigger error + await driver.executeScript( + 'window.stateHooks.throwTestBackgroundError()', + ); + + // 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 NOT send error events in the UI', 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.findElement('#password'); + + // Trigger error + await driver.executeScript('window.stateHooks.throwTestError()'); + + // Wait for Sentry request + const isPending = await mockedEndpoint.isPending(); + assert.ok( + isPending, + 'A request to sentry was sent when it should not have been', + ); + }, + ); + }); + }); + + describe('after initialization, after opting into metrics', function () { + it('should send error events in background', 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.findElement('#password'); + + // Trigger error + await driver.executeScript( + 'window.stateHooks.throwTestBackgroundError()', + ); + + // 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, extra } = mockJsonBody; const [{ type, value }] = mockJsonBody.exception.values; - const { participateInMetaMetrics } = extra.appState.store.metamask; + const { participateInMetaMetrics } = + extra.appState.state.MetaMetricsController; // Verify request assert.equal(type, 'TestError'); assert.equal(value, 'Test Error'); @@ -184,5 +578,154 @@ describe('Sentry errors', function () { }, ); }); + + it('should capture background application state', 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.findElement('#password'); + + // Trigger error + await driver.executeScript( + 'window.stateHooks.throwTestBackgroundError()', + ); + + // 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 appState = mockJsonBody?.extra?.appState; + assert.deepStrictEqual(Object.keys(appState), [ + 'browser', + 'version', + 'state', + ]); + assert.ok( + typeof appState?.browser === 'string' && + appState?.browser.length > 0, + 'Invalid browser state', + ); + assert.ok( + typeof appState?.version === 'string' && + appState?.version.length > 0, + 'Invalid version state', + ); + await matchesSnapshot({ + data: transformBackgroundState(appState.state), + snapshot: 'errors-after-init-opt-in-background-state', + }); + }, + ); + }); + + it('should send error events in UI', 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.findElement('#password'); + + // Trigger error + await driver.executeScript('window.stateHooks.throwTestError()'); + + // 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, extra } = mockJsonBody; + const [{ type, value }] = mockJsonBody.exception.values; + const { participateInMetaMetrics } = extra.appState.state.metamask; + // Verify request + assert.equal(type, 'TestError'); + assert.equal(value, 'Test Error'); + assert.equal(level, 'error'); + assert.equal(participateInMetaMetrics, true); + }, + ); + }); + + it('should capture UI application state', 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.findElement('#password'); + + // Trigger error + await driver.executeScript('window.stateHooks.throwTestError()'); + + // 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 appState = mockJsonBody?.extra?.appState; + assert.deepStrictEqual(Object.keys(appState), [ + 'browser', + 'version', + 'state', + ]); + assert.ok( + typeof appState?.browser === 'string' && + appState?.browser.length > 0, + 'Invalid browser state', + ); + assert.ok( + typeof appState?.version === 'string' && + appState?.version.length > 0, + 'Invalid version state', + ); + await matchesSnapshot({ + data: transformUiState(appState.state), + snapshot: 'errors-after-init-opt-in-ui-state', + }); + }, + ); + }); }); }); diff --git a/test/e2e/tests/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-background-state.json new file mode 100644 index 000000000..b682921e7 --- /dev/null +++ b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-background-state.json @@ -0,0 +1,157 @@ +{ + "AccountTracker": { "accounts": "object" }, + "AddressBookController": "object", + "AlertController": { + "alertEnabledness": { "unconnectedAccount": true, "web3ShimUsage": true }, + "unconnectedAccountAlertShownOrigins": "object", + "web3ShimUsageOrigins": "object" + }, + "AnnouncementController": "object", + "AppMetadataController": { + "currentAppVersion": "10.34.5", + "previousAppVersion": "", + "previousMigrationVersion": 0, + "currentMigrationVersion": 92.1 + }, + "AppStateController": { + "connectedStatusPopoverHasBeenShown": true, + "defaultHomeActiveTabName": null, + "browserEnvironment": "object", + "popupGasPollTokens": "object", + "notificationGasPollTokens": "object", + "fullScreenGasPollTokens": "object", + "recoveryPhraseReminderHasBeenShown": "boolean", + "recoveryPhraseReminderLastShown": "number", + "outdatedBrowserWarningLastShown": "number", + "nftsDetectionNoticeDismissed": "boolean", + "showTestnetMessageInDropdown": "boolean", + "showBetaHeader": "boolean", + "showProductTour": "boolean", + "trezorModel": "object", + "nftsDropdownState": "object", + "termsOfUseLastAgreed": "number", + "qrHardware": "object", + "usedNetworks": "object", + "snapsInstallPrivacyWarningShown": "boolean", + "serviceWorkerLastActiveTime": "number" + }, + "ApprovalController": "object", + "BackupController": "undefined", + "CachedBalancesController": "object", + "CurrencyController": { + "conversionDate": "number", + "conversionRate": 1700, + "nativeCurrency": "ETH", + "currentCurrency": "usd", + "pendingCurrentCurrency": "object", + "pendingNativeCurrency": "object", + "usdConversionRate": "number" + }, + "DecryptMessageController": { + "unapprovedDecryptMsgs": "object", + "unapprovedDecryptMsgCount": 0 + }, + "EncryptionPublicKeyController": { + "unapprovedEncryptionPublicKeyMsgs": "object", + "unapprovedEncryptionPublicKeyMsgCount": 0 + }, + "EnsController": "object", + "GasFeeController": "object", + "IncomingTransactionsController": { + "incomingTransactions": "object", + "incomingTxLastFetchedBlockByChainId": { + "0x1": null, + "0xe708": null, + "0x5": null, + "0xaa36a7": null, + "0xe704": null + } + }, + "KeyringController": { + "isUnlocked": false, + "keyringTypes": "object", + "keyrings": "object", + "encryptionKey": "object" + }, + "MetaMetricsController": { + "participateInMetaMetrics": true, + "metaMetricsId": "fake-metrics-id", + "eventsBeforeMetricsOptIn": "object", + "traits": "object", + "fragments": "object", + "segmentApiCalls": "object", + "previousUserTraits": "object" + }, + "NetworkController": { + "networkId": "1337", + "networkStatus": "available", + "providerConfig": { + "chainId": "string", + "nickname": "Localhost 8545", + "rpcPrefs": "object", + "rpcUrl": "string", + "ticker": "ETH", + "type": "rpc", + "id": "string" + }, + "networkDetails": "object", + "networkConfigurations": "object" + }, + "NftController": "object", + "OnboardingController": { + "seedPhraseBackedUp": true, + "firstTimeFlowType": "import", + "completedOnboarding": true, + "onboardingTabs": "object" + }, + "PermissionController": "object", + "PermissionLogController": "object", + "PreferencesController": { + "useBlockie": false, + "useNonceField": false, + "usePhishDetect": true, + "dismissSeedBackUpReminder": "boolean", + "disabledRpcMethodPreferences": "object", + "useMultiAccountBalanceChecker": "boolean", + "useTokenDetection": "boolean", + "useNftDetection": "boolean", + "useCurrencyRateCheck": "boolean", + "openSeaEnabled": "boolean", + "advancedGasFee": "object", + "featureFlags": { "showIncomingTransactions": true }, + "knownMethodData": "object", + "currentLocale": "en", + "identities": "object", + "lostIdentities": "object", + "forgottenPassword": false, + "preferences": { + "hideZeroBalanceTokens": false, + "showFiatInTestnets": false, + "showTestNetworks": false, + "useNativeCurrencyAsPrimaryCurrency": true + }, + "ipfsGateway": "dweb.link", + "infuraBlocked": "boolean", + "ledgerTransportType": "string", + "snapRegistryList": "object", + "transactionSecurityCheckEnabled": "boolean", + "theme": "string", + "isLineaMainnetReleased": "boolean", + "selectedAddress": "string" + }, + "SignatureController": { + "unapprovedMsgs": "object", + "unapprovedPersonalMsgs": "object", + "unapprovedTypedMessages": "object", + "unapprovedMsgCount": 0, + "unapprovedPersonalMsgCount": 0, + "unapprovedTypedMessagesCount": 0 + }, + "SmartTransactionsController": "object", + "SubjectMetadataController": "object", + "SwapsController": "object", + "TokenListController": "object", + "TokenRatesController": "object", + "TokensController": "object", + "TxController": "object" +} diff --git a/test/e2e/tests/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-ui-state.json new file mode 100644 index 000000000..af2f71406 --- /dev/null +++ b/test/e2e/tests/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -0,0 +1,165 @@ +{ + "DNS": "object", + "activeTab": "object", + "appState": "object", + "confirmTransaction": "object", + "gas": { "customData": { "price": null, "limit": null } }, + "history": { "mostRecentOverviewPage": "/" }, + "invalidCustomNetwork": "object", + "localeMessages": "object", + "metamask": { + "isInitialized": true, + "isUnlocked": false, + "isAccountMenuOpen": false, + "isNetworkMenuOpen": "boolean", + "identities": "object", + "unapprovedTxs": "object", + "networkConfigurations": "object", + "addressBook": "object", + "contractExchangeRates": "object", + "pendingTokens": "object", + "customNonceValue": "", + "useBlockie": false, + "featureFlags": { "showIncomingTransactions": true }, + "welcomeScreenSeen": false, + "currentLocale": "en", + "preferences": { + "hideZeroBalanceTokens": false, + "showFiatInTestnets": false, + "showTestNetworks": false, + "useNativeCurrencyAsPrimaryCurrency": true + }, + "firstTimeFlowType": "import", + "completedOnboarding": true, + "knownMethodData": "object", + "participateInMetaMetrics": true, + "nextNonce": null, + "conversionRate": 1300, + "nativeCurrency": "ETH", + "connectedStatusPopoverHasBeenShown": true, + "defaultHomeActiveTabName": null, + "browserEnvironment": "object", + "popupGasPollTokens": "object", + "notificationGasPollTokens": "object", + "fullScreenGasPollTokens": "object", + "recoveryPhraseReminderHasBeenShown": "boolean", + "recoveryPhraseReminderLastShown": "number", + "outdatedBrowserWarningLastShown": "number", + "nftsDetectionNoticeDismissed": "boolean", + "showTestnetMessageInDropdown": "boolean", + "showBetaHeader": "boolean", + "showProductTour": "boolean", + "trezorModel": "object", + "nftsDropdownState": "object", + "termsOfUseLastAgreed": "number", + "qrHardware": "object", + "usedNetworks": "object", + "snapsInstallPrivacyWarningShown": "boolean", + "serviceWorkerLastActiveTime": "number", + "currentAppVersion": "10.34.5", + "previousAppVersion": "", + "previousMigrationVersion": 0, + "currentMigrationVersion": 92.1, + "networkId": "1337", + "networkStatus": "available", + "providerConfig": { + "chainId": "string", + "nickname": "Localhost 8545", + "rpcPrefs": "object", + "rpcUrl": "string", + "ticker": "ETH", + "type": "rpc", + "id": "string" + }, + "networkDetails": "object", + "cachedBalances": "object", + "keyringTypes": "object", + "keyrings": "object", + "encryptionKey": "object", + "useNonceField": false, + "usePhishDetect": true, + "dismissSeedBackUpReminder": "boolean", + "disabledRpcMethodPreferences": "object", + "useMultiAccountBalanceChecker": "boolean", + "useTokenDetection": "boolean", + "useNftDetection": "boolean", + "useCurrencyRateCheck": "boolean", + "openSeaEnabled": "boolean", + "advancedGasFee": "object", + "lostIdentities": "object", + "forgottenPassword": false, + "ipfsGateway": "dweb.link", + "infuraBlocked": "boolean", + "ledgerTransportType": "string", + "snapRegistryList": "object", + "transactionSecurityCheckEnabled": "boolean", + "theme": "string", + "isLineaMainnetReleased": "boolean", + "selectedAddress": "string", + "metaMetricsId": "fake-metrics-id", + "eventsBeforeMetricsOptIn": "object", + "traits": "object", + "fragments": "object", + "segmentApiCalls": "object", + "previousUserTraits": "object", + "conversionDate": "number", + "currentCurrency": "usd", + "pendingCurrentCurrency": "object", + "pendingNativeCurrency": "object", + "usdConversionRate": "number", + "alertEnabledness": { "unconnectedAccount": true, "web3ShimUsage": true }, + "unconnectedAccountAlertShownOrigins": "object", + "web3ShimUsageOrigins": "object", + "seedPhraseBackedUp": true, + "onboardingTabs": "object", + "incomingTransactions": "object", + "incomingTxLastFetchedBlockByChainId": { + "0x1": null, + "0xe708": null, + "0x5": null, + "0xaa36a7": null, + "0xe704": null + }, + "subjects": "object", + "permissionHistory": "object", + "permissionActivityLog": "object", + "subjectMetadata": "object", + "announcements": "object", + "gasFeeEstimates": "object", + "estimatedGasFeeTimeBounds": "object", + "gasEstimateType": "string", + "tokenList": "object", + "tokensChainsCache": "object", + "preventPollingOnNetworkRestart": "boolean", + "tokens": "object", + "ignoredTokens": "object", + "detectedTokens": "object", + "allTokens": "object", + "allIgnoredTokens": "object", + "allDetectedTokens": "object", + "smartTransactionsState": "object", + "allNftContracts": "object", + "allNfts": "object", + "ignoredNfts": "object", + "accounts": "object", + "currentNetworkTxList": "object", + "unapprovedDecryptMsgs": "object", + "unapprovedDecryptMsgCount": 0, + "unapprovedEncryptionPublicKeyMsgs": "object", + "unapprovedEncryptionPublicKeyMsgCount": 0, + "unapprovedMsgs": "object", + "unapprovedPersonalMsgs": "object", + "unapprovedTypedMessages": "object", + "unapprovedMsgCount": 0, + "unapprovedPersonalMsgCount": 0, + "unapprovedTypedMessagesCount": 0, + "swapsState": "object", + "ensResolutionsByAddress": "object", + "pendingApprovals": "object", + "pendingApprovalCount": "number", + "approvalFlows": "object" + }, + "send": "object", + "swaps": "object", + "unconnectedAccount": { "state": "CLOSED" } +} diff --git a/test/e2e/tests/state-snapshots/errors-before-init-opt-in-background-state.json b/test/e2e/tests/state-snapshots/errors-before-init-opt-in-background-state.json new file mode 100644 index 000000000..f3adc0cd9 --- /dev/null +++ b/test/e2e/tests/state-snapshots/errors-before-init-opt-in-background-state.json @@ -0,0 +1,110 @@ +{ + "data": { + "AlertController": { + "alertEnabledness": { "unconnectedAccount": true, "web3ShimUsage": true }, + "unconnectedAccountAlertShownOrigins": "object", + "web3ShimUsageOrigins": "object" + }, + "AnnouncementController": "object", + "AppStateController": { + "browserEnvironment": "object", + "nftsDropdownState": "object", + "connectedStatusPopoverHasBeenShown": true, + "termsOfUseLastAgreed": "number", + "defaultHomeActiveTabName": null, + "fullScreenGasPollTokens": "object", + "notificationGasPollTokens": "object", + "popupGasPollTokens": "object", + "qrHardware": "object", + "recoveryPhraseReminderHasBeenShown": "boolean", + "recoveryPhraseReminderLastShown": "number", + "showTestnetMessageInDropdown": "boolean", + "trezorModel": "object", + "usedNetworks": "object", + "snapsInstallPrivacyWarningShown": "boolean" + }, + "CachedBalancesController": "object", + "CurrencyController": { + "conversionDate": 1665507600, + "conversionRate": 1300, + "currentCurrency": "usd", + "nativeCurrency": "ETH", + "usdConversionRate": "number" + }, + "GasFeeController": "object", + "IncomingTransactionsController": { + "incomingTransactions": "object", + "incomingTxLastFetchedBlockByChainId": { + "0x1": null, + "0xe708": null, + "0x5": null, + "0xaa36a7": null, + "0xe704": null + } + }, + "KeyringController": { "vault": "string" }, + "MetaMetricsController": { + "eventsBeforeMetricsOptIn": "object", + "fragments": "object", + "metaMetricsId": "fake-metrics-id", + "participateInMetaMetrics": true, + "traits": "object" + }, + "NetworkController": { + "networkId": "1337", + "networkStatus": "available", + "providerConfig": { + "chainId": "string", + "nickname": "Localhost 8545", + "rpcPrefs": "object", + "rpcUrl": "string", + "ticker": "ETH", + "type": "rpc", + "id": "string" + }, + "networkConfigurations": "object" + }, + "OnboardingController": { + "completedOnboarding": true, + "firstTimeFlowType": "import", + "onboardingTabs": "object", + "seedPhraseBackedUp": true + }, + "PermissionController": "object", + "PreferencesController": { + "advancedGasFee": "object", + "currentLocale": "en", + "dismissSeedBackUpReminder": "boolean", + "featureFlags": { "showIncomingTransactions": true }, + "forgottenPassword": false, + "identities": "object", + "infuraBlocked": "boolean", + "ipfsGateway": "dweb.link", + "knownMethodData": "object", + "ledgerTransportType": "string", + "lostIdentities": "object", + "openSeaEnabled": "boolean", + "preferences": { + "hideZeroBalanceTokens": false, + "showFiatInTestnets": false, + "showTestNetworks": false, + "useNativeCurrencyAsPrimaryCurrency": true + }, + "selectedAddress": "string", + "theme": "string", + "useBlockie": false, + "useNftDetection": "boolean", + "useNonceField": false, + "usePhishDetect": true, + "useTokenDetection": "boolean", + "useCurrencyRateCheck": "boolean", + "useMultiAccountBalanceChecker": "boolean" + }, + "SmartTransactionsController": "object", + "SubjectMetadataController": "object", + "TokensController": "object", + "TransactionController": "object", + "config": "object", + "firstTimeInfo": "object" + } +} diff --git a/test/e2e/tests/state-snapshots/errors-before-init-opt-in-ui-state.json b/test/e2e/tests/state-snapshots/errors-before-init-opt-in-ui-state.json new file mode 100644 index 000000000..704026b0b --- /dev/null +++ b/test/e2e/tests/state-snapshots/errors-before-init-opt-in-ui-state.json @@ -0,0 +1,111 @@ +{ + "data": { + "AlertController": { + "alertEnabledness": { "unconnectedAccount": true, "web3ShimUsage": true }, + "unconnectedAccountAlertShownOrigins": "object", + "web3ShimUsageOrigins": "object" + }, + "AnnouncementController": "object", + "AppStateController": { + "browserEnvironment": "object", + "nftsDropdownState": "object", + "connectedStatusPopoverHasBeenShown": true, + "termsOfUseLastAgreed": "number", + "defaultHomeActiveTabName": null, + "fullScreenGasPollTokens": "object", + "notificationGasPollTokens": "object", + "popupGasPollTokens": "object", + "qrHardware": "object", + "recoveryPhraseReminderHasBeenShown": "boolean", + "recoveryPhraseReminderLastShown": "number", + "showTestnetMessageInDropdown": "boolean", + "trezorModel": "object", + "usedNetworks": "object", + "snapsInstallPrivacyWarningShown": "boolean" + }, + "CachedBalancesController": "object", + "CurrencyController": { + "conversionDate": 1665507600, + "conversionRate": 1300, + "currentCurrency": "usd", + "nativeCurrency": "ETH", + "usdConversionRate": "number" + }, + "GasFeeController": "object", + "IncomingTransactionsController": { + "incomingTransactions": "object", + "incomingTxLastFetchedBlockByChainId": { + "0x1": null, + "0xe708": null, + "0x5": null, + "0xaa36a7": null, + "0xe704": null + } + }, + "KeyringController": { "vault": "string" }, + "MetaMetricsController": { + "eventsBeforeMetricsOptIn": "object", + "fragments": "object", + "metaMetricsId": "fake-metrics-id", + "participateInMetaMetrics": true, + "traits": "object" + }, + "NetworkController": { + "networkId": "1337", + "networkStatus": "available", + "providerConfig": { + "chainId": "string", + "nickname": "Localhost 8545", + "rpcPrefs": "object", + "rpcUrl": "string", + "ticker": "ETH", + "type": "rpc", + "id": "string" + }, + "networkConfigurations": "object" + }, + "OnboardingController": { + "completedOnboarding": true, + "firstTimeFlowType": "import", + "onboardingTabs": "object", + "seedPhraseBackedUp": true + }, + "PermissionController": "object", + "PreferencesController": { + "advancedGasFee": "object", + "currentLocale": "en", + "dismissSeedBackUpReminder": "boolean", + "featureFlags": { "showIncomingTransactions": true }, + "forgottenPassword": false, + "identities": "object", + "infuraBlocked": "boolean", + "ipfsGateway": "dweb.link", + "knownMethodData": "object", + "ledgerTransportType": "string", + "lostIdentities": "object", + "openSeaEnabled": "boolean", + "preferences": { + "hideZeroBalanceTokens": false, + "showFiatInTestnets": false, + "showTestNetworks": false, + "useNativeCurrencyAsPrimaryCurrency": true + }, + "selectedAddress": "string", + "theme": "string", + "useBlockie": false, + "useNftDetection": "boolean", + "useNonceField": false, + "usePhishDetect": true, + "useTokenDetection": "boolean", + "useCurrencyRateCheck": "boolean", + "useMultiAccountBalanceChecker": "boolean" + }, + "SmartTransactionsController": "object", + "SubjectMetadataController": "object", + "TokensController": "object", + "TransactionController": "object", + "config": "object", + "firstTimeInfo": "object" + }, + "meta": { "version": 74 } +} diff --git a/types/global.d.ts b/types/global.d.ts index f1e8c1b18..a288e0f77 100644 --- a/types/global.d.ts +++ b/types/global.d.ts @@ -1,6 +1,7 @@ // In order for variables to be considered on the global scope they must be // declared using var and not const or let, which is why this rule is disabled /* eslint-disable no-var */ +import * as Sentry from '@sentry/browser'; declare class Platform { openTab: (opts: { url: string }) => void; @@ -11,6 +12,9 @@ declare class Platform { export declare global { var platform: Platform; + // Sentry is undefined in dev, so use optional chaining + var sentry: Sentry | undefined; + namespace jest { interface Matchers { toBeFulfilled(): Promise; diff --git a/ui/index.js b/ui/index.js index 1f1aeb281..208006de1 100644 --- a/ui/index.js +++ b/ui/index.js @@ -8,7 +8,7 @@ import browser from 'webextension-polyfill'; import { getEnvironmentType } from '../app/scripts/lib/util'; import { AlertTypes } from '../shared/constants/alerts'; import { maskObject } from '../shared/modules/object.utils'; -import { SENTRY_STATE } from '../app/scripts/lib/setupSentry'; +import { SENTRY_UI_STATE } from '../app/scripts/lib/setupSentry'; import { ENVIRONMENT_TYPE_POPUP } from '../shared/constants/app'; import switchDirection from '../shared/lib/switch-direction'; import { setupLocale } from '../shared/lib/error-utils'; @@ -27,7 +27,7 @@ import Root from './pages'; import txHelper from './helpers/utils/tx-helper'; import { _setBackgroundConnection } from './store/action-queue'; -log.setLevel(global.METAMASK_DEBUG ? 'debug' : 'warn'); +log.setLevel(global.METAMASK_DEBUG ? 'debug' : 'warn', false); let reduxStore; @@ -81,7 +81,7 @@ export default function launchMetamaskUi(opts, cb) { return; } startApp(metamaskState, backgroundConnection, opts).then((store) => { - setupDebuggingHelpers(store); + setupStateHooks(store); cb( null, store, @@ -191,16 +191,39 @@ async function startApp(metamaskState, backgroundConnection, opts) { return store; } -function setupDebuggingHelpers(store) { - /** - * The following stateHook is a method intended to throw an error, used in - * our E2E test to ensure that errors are attempted to be sent to sentry. - */ - window.stateHooks.throwTestError = async function () { - const error = new Error('Test Error'); - error.name = 'TestError'; - throw error; - }; +/** + * Setup functions on `window.stateHooks`. Some of these support + * application features, and some are just for debugging or testing. + * + * @param {object} store - The Redux store. + */ +function setupStateHooks(store) { + if (process.env.METAMASK_DEBUG || process.env.IN_TEST) { + /** + * The following stateHook is a method intended to throw an error, used in + * our E2E test to ensure that errors are attempted to be sent to sentry. + * + * @param {string} [msg] - The error message to throw, defaults to 'Test Error' + */ + window.stateHooks.throwTestError = async function (msg = 'Test Error') { + const error = new Error(msg); + error.name = 'TestError'; + throw error; + }; + /** + * The following stateHook is a method intended to throw an error in the + * background, used in our E2E test to ensure that errors are attempted to be + * sent to sentry. + * + * @param {string} [msg] - The error message to throw, defaults to 'Test Error' + */ + window.stateHooks.throwTestBackgroundError = async function ( + msg = 'Test Error', + ) { + store.dispatch(actions.throwTestBackgroundError(msg)); + }; + } + window.stateHooks.getCleanAppState = async function () { const state = clone(store.getState()); state.version = global.platform.getVersion(); @@ -210,14 +233,9 @@ function setupDebuggingHelpers(store) { }); return state; }; - window.stateHooks.getSentryState = function () { - const fullState = store.getState(); - const debugState = maskObject(fullState, SENTRY_STATE); - return { - browser: window.navigator.userAgent, - store: debugState, - version: global.platform.getVersion(), - }; + window.stateHooks.getSentryAppState = function () { + const reduxState = store.getState(); + return maskObject(reduxState, SENTRY_UI_STATE); }; } diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 46ef92d12..8040b01b8 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -4427,6 +4427,17 @@ export async function getCurrentNetworkEIP1559Compatibility(): Promise< return networkEIP1559Compatibility; } +/** + * Throw an error in the background for testing purposes. + * + * @param message - The error message. + * @deprecated This is only mean to facilitiate E2E testing. We should not use + * this for handling errors. + */ +export async function throwTestBackgroundError(message: string): Promise { + await submitRequestToBackground('throwTestError', [message]); +} + ///: BEGIN:ONLY_INCLUDE_IN(snaps) /** * Set status of popover warning for the first snap installation. diff --git a/yarn.lock b/yarn.lock index 6ab9f254b..faa9438a0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1891,6 +1891,13 @@ __metadata: languageName: node linkType: hard +"@endo/env-options@npm:^0.1.3": + version: 0.1.3 + resolution: "@endo/env-options@npm:0.1.3" + checksum: da8c66865d4d30b0053a00960657dc36f022975a888f0dd6a2f6bb37b9fe731f45a02a2cf263d93b1a40fcb37b25f8ba7076cb8af9e93fd95f496365d9382930 + languageName: node + linkType: hard + "@ensdomains/address-encoder@npm:^0.1.7": version: 0.1.9 resolution: "@ensdomains/address-encoder@npm:0.1.9" @@ -24935,7 +24942,7 @@ __metadata: selenium-webdriver: ^4.9.0 semver: ^7.3.5 serve-handler: ^6.1.2 - ses: ^0.18.4 + ses: ^0.18.7 single-call-balance-checker-abi: ^1.0.0 sinon: ^9.0.0 source-map: ^0.7.2 @@ -28697,8 +28704,8 @@ __metadata: linkType: hard "protobufjs@npm:^6.11.3": - version: 6.11.3 - resolution: "protobufjs@npm:6.11.3" + version: 6.11.4 + resolution: "protobufjs@npm:6.11.4" dependencies: "@protobufjs/aspromise": ^1.1.2 "@protobufjs/base64": ^1.1.2 @@ -28716,7 +28723,7 @@ __metadata: bin: pbjs: bin/pbjs pbts: bin/pbts - checksum: 4a6ce1964167e4c45c53fd8a312d7646415c777dd31b4ba346719947b88e61654912326101f927da387d6b6473ab52a7ea4f54d6f15d63b31130ce28e2e15070 + checksum: 6b7fd7540d74350d65c38f69f398c9995ae019da070e79d9cd464a458c6d19b40b07c9a026be4e10704c824a344b603307745863310c50026ebd661ce4da0663 languageName: node linkType: hard @@ -31499,10 +31506,12 @@ __metadata: languageName: node linkType: hard -"ses@npm:^0.18.1, ses@npm:^0.18.4": - version: 0.18.4 - resolution: "ses@npm:0.18.4" - checksum: 9afd6edcf390a693926ef728ebb5a435994bbb0f915009ad524c6588cf62e2f66f6d4b4b2694f093b2af2e92c003947ad55404750d756ba75ce70c8636a7ba02 +"ses@npm:^0.18.1, ses@npm:^0.18.7": + version: 0.18.7 + resolution: "ses@npm:0.18.7" + dependencies: + "@endo/env-options": ^0.1.3 + checksum: 75ac014771d9bc1f747193c6d0f9e7d2d7700a10311ba8d805d9bc78d4c20d4ef40537f0535b1ea6abf06babf67e70f8bd37b2ad68ad54992a0c5ce842181c87 languageName: node linkType: hard