diff --git a/app/scripts/background.js b/app/scripts/background.js index 86d4b5525..7b45cdfd5 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'; @@ -69,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 }; @@ -92,9 +100,6 @@ 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) { @@ -899,17 +904,9 @@ browser.runtime.onInstalled.addListener(({ reason }) => { }); function setupSentryGetStateGlobal(store) { - global.stateHooks.getSentryState = function () { + global.stateHooks.getSentryAppState = function () { const backgroundState = store.memStore.getState(); - const maskedBackgroundState = maskObject( - backgroundState, - SENTRY_BACKGROUND_STATE, - ); - return { - browser: window.navigator.userAgent, - store: maskedBackgroundState, - version: platform.getVersion(), - }; + return maskObject(backgroundState, SENTRY_BACKGROUND_STATE); }; } 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/network-store.js b/app/scripts/lib/network-store.js index ea6ba5876..2f4c0a1b0 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; } /** @@ -30,6 +31,7 @@ export default class ReadOnlyNetworkStore { const response = await fetchWithTimeout(FIXTURE_SERVER_URL); if (response.ok) { this._state = await response.json(); + this.mostRecentRetrievedState = this._state; } } catch (error) { log.debug(`Error loading network state: '${error.message}'`); 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..fa02987a7 --- /dev/null +++ b/app/scripts/lib/setup-initial-state-hooks.js @@ -0,0 +1,61 @@ +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(); +const localStore = 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 localStore.get(); +}; + +const persistedStateMask = { + data: SENTRY_BACKGROUND_STATE, + meta: { + version: true, + }, +}; + +/** + * Get a state snapshot to include with Sentry error reports. 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 (globalThis.stateHooks.getSentryAppState) { + return { + ...sentryState, + state: globalThis.stateHooks.getSentryAppState(), + }; + } else if (globalThis.stateHooks.getMostRecentPersistedState) { + const persistedState = globalThis.stateHooks.getMostRecentPersistedState(); + if (persistedState) { + return { + ...sentryState, + persistedState: maskObject( + // `getMostRecentPersistedState` is used here instead of + // `getPersistedState` to avoid making this an asynchronous function. + globalThis.stateHooks.getMostRecentPersistedState(), + persistedStateMask, + ), + }; + } + return sentryState; + } + 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/ui.js b/app/scripts/ui.js index 50f37c430..6be5c7758 100644 --- a/app/scripts/ui.js +++ b/app/scripts/ui.js @@ -1,10 +1,12 @@ +// 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'; + // 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'; @@ -31,6 +33,14 @@ import ExtensionPlatform from './platforms/extension'; import { setupMultiplex } from './lib/stream-utils'; import { getEnvironmentType, getPlatform } from './lib/util'; import metaRPCClientFactory from './lib/metaRPCClientFactory'; +import LocalStore from './lib/local-store'; +import ReadOnlyNetworkStore from './lib/network-store'; + +// 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 container = document.getElementById('app-content'); diff --git a/test/e2e/tests/errors.spec.js b/test/e2e/tests/errors.spec.js index ff3b2e56f..36f1921b3 100644 --- a/test/e2e/tests/errors.spec.js +++ b/test/e2e/tests/errors.spec.js @@ -195,9 +195,9 @@ describe('Sentry errors', function () { async ({ driver, mockedEndpoint }) => { await driver.navigate(); await driver.findElement('#password'); - // Erase `getSentryState` hook, simulating a "before initialization" state + // Erase `getSentryAppState` hook, simulating a "before initialization" state await driver.executeScript( - 'window.stateHooks.getSentryState = undefined', + 'window.stateHooks.getSentryAppState = undefined', ); // Wait for Sentry request @@ -284,8 +284,23 @@ describe('Sentry errors', function () { 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), + data: transformBackgroundState(appState.persistedState), snapshot: 'errors-before-init-opt-in-background-state', }); }, @@ -309,9 +324,9 @@ describe('Sentry errors', function () { async ({ driver, mockedEndpoint }) => { await driver.navigate(); await driver.findElement('#password'); - // Erase `getSentryState` hook, simulating a "before initialization" state + // Erase `getSentryAppState` hook, simulating a "before initialization" state await driver.executeScript( - 'window.stateHooks.getSentryState = undefined', + 'window.stateHooks.getSentryAppState = undefined', ); // Trigger error @@ -352,9 +367,9 @@ describe('Sentry errors', function () { async ({ driver, mockedEndpoint }) => { await driver.navigate(); await driver.findElement('#password'); - // Erase `getSentryState` hook, simulating a "before initialization" state + // Erase `getSentryAppState` hook, simulating a "before initialization" state await driver.executeScript( - 'window.stateHooks.getSentryState = undefined', + 'window.stateHooks.getSentryAppState = undefined', ); // Trigger error @@ -369,8 +384,23 @@ describe('Sentry errors', function () { 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: transformUiState(appState), + data: transformBackgroundState(appState.persistedState), snapshot: 'errors-before-init-opt-in-ui-state', }); }, @@ -479,7 +509,7 @@ describe('Sentry errors', function () { const { level, extra } = mockJsonBody; const [{ type, value }] = mockJsonBody.exception.values; const { participateInMetaMetrics } = - extra.appState.store.MetaMetricsController; + extra.appState.state.MetaMetricsController; // Verify request assert.equal(type, 'TestError'); assert.equal(value, 'Test Error'); @@ -523,8 +553,8 @@ describe('Sentry errors', function () { const appState = mockJsonBody?.extra?.appState; assert.deepStrictEqual(Object.keys(appState), [ 'browser', - 'store', 'version', + 'state', ]); assert.ok( typeof appState?.browser === 'string' && @@ -537,7 +567,7 @@ describe('Sentry errors', function () { 'Invalid version state', ); await matchesSnapshot({ - data: transformBackgroundState(appState.store), + data: transformBackgroundState(appState.state), snapshot: 'errors-after-init-opt-in-background-state', }); }, @@ -575,7 +605,7 @@ describe('Sentry errors', function () { 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.metamask; // Verify request assert.equal(type, 'TestError'); assert.equal(value, 'Test Error'); @@ -617,8 +647,8 @@ describe('Sentry errors', function () { const appState = mockJsonBody?.extra?.appState; assert.deepStrictEqual(Object.keys(appState), [ 'browser', - 'store', 'version', + 'state', ]); assert.ok( typeof appState?.browser === 'string' && @@ -631,7 +661,7 @@ describe('Sentry errors', function () { 'Invalid version state', ); await matchesSnapshot({ - data: transformUiState(appState.store), + data: transformUiState(appState.state), snapshot: 'errors-after-init-opt-in-ui-state', }); }, 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 index 0967ef424..422bcb0e2 100644 --- 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 @@ -1 +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", + "selectedNetworkClientId": "string", + "networksMetadata": "object", + "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 index 0967ef424..0a9fac1d0 100644 --- 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 @@ -1 +1,112 @@ -{} +{ + "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", + "selectedNetworkClientId": "string", + "networksMetadata": "object", + "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/ui/index.js b/ui/index.js index 6f9a148ed..de1432a80 100644 --- a/ui/index.js +++ b/ui/index.js @@ -233,14 +233,9 @@ function setupStateHooks(store) { }); return state; }; - window.stateHooks.getSentryState = function () { + window.stateHooks.getSentryAppState = function () { const reduxState = store.getState(); - const maskedReduxState = maskObject(reduxState, SENTRY_UI_STATE); - return { - browser: window.navigator.userAgent, - store: maskedReduxState, - version: global.platform.getVersion(), - }; + return maskObject(reduxState, SENTRY_UI_STATE); }; }