From 63a0ae765f8a7589f91d9c56c0a2f43aa2f685cb Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 16 Aug 2023 15:21:18 -0230 Subject: [PATCH] Use unflattened state for Sentry (#20428) The unflattened background state is now attached to any Sentry errors from the background process. This provides a clearer picture of the state of the wallet, and unblocks further improvements to Sentry state which will come in later PRs. --- app/scripts/background.js | 4 +- app/scripts/lib/setupSentry.js | 133 ++++++++++++------ test/e2e/tests/errors.spec.js | 37 +++-- ...rs-after-init-opt-in-background-state.json | 64 +++------ 4 files changed, 138 insertions(+), 100 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 6b29e14d7..5b97bb34d 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -886,14 +886,14 @@ browser.runtime.onInstalled.addListener(({ reason }) => { function setupSentryGetStateGlobal(store) { global.stateHooks.getSentryState = function () { - const backgroundState = store.getState(); + const backgroundState = store.memStore.getState(); const maskedBackgroundState = maskObject( backgroundState, SENTRY_BACKGROUND_STATE, ); return { browser: window.navigator.userAgent, - store: { metamask: maskedBackgroundState }, + store: maskedBackgroundState, version: platform.getVersion(), }; }; diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 2699d2e3d..384f246db 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -27,60 +27,103 @@ export const ERROR_URL_ALLOWLIST = { // 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 = { - alertEnabledness: true, - completedOnboarding: true, - connectedStatusPopoverHasBeenShown: true, - conversionDate: true, - conversionRate: true, - currentAppVersion: true, - currentBlockGasLimit: true, - currentCurrency: true, - currentLocale: true, - currentMigrationVersion: 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, - networkId: true, - networkStatus: true, - nextNonce: true, - participateInMetaMetrics: true, - preferences: true, - previousAppVersion: true, - previousMigrationVersion: true, - providerConfig: { - nickname: true, - ticker: true, - type: true, + AccountTracker: { + currentBlockGasLimit: true, + }, + AlertController: { + alertEnabledness: true, + }, + AppMetadataController: { + currentAppVersion: true, + previousAppVersion: true, + previousMigrationVersion: true, + currentMigrationVersion: true, + }, + AppStateController: { + connectedStatusPopoverHasBeenShown: true, + defaultHomeActiveTabName: true, + }, + CurrencyController: { + conversionDate: true, + conversionRate: true, + currentCurrency: 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, + providerConfig: { + nickname: true, + ticker: true, + type: true, + }, + }, + OnboardingController: { + completedOnboarding: true, + firstTimeFlowType: true, + seedPhraseBackedUp: 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, }, - seedPhraseBackedUp: true, - unapprovedDecryptMsgCount: true, - unapprovedEncryptionPublicKeyMsgCount: true, - unapprovedMsgCount: true, - unapprovedPersonalMsgCount: true, - unapprovedTypedMessagesCount: true, - useBlockie: true, - useNonceField: true, - usePhishDetect: true, - welcomeScreenSeen: 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: SENTRY_BACKGROUND_STATE, + 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, }; diff --git a/test/e2e/tests/errors.spec.js b/test/e2e/tests/errors.spec.js index 4ab80a7de..8784ef168 100644 --- a/test/e2e/tests/errors.spec.js +++ b/test/e2e/tests/errors.spec.js @@ -7,7 +7,8 @@ const { format } = require('prettier'); const { convertToHexValue, withFixtures } = require('../helpers'); const FixtureBuilder = require('../fixture-builder'); -const dateFields = ['metamask.conversionDate']; +const backgroundDateFields = ['CurrencyController.conversionDate']; +const uiDateFields = ['metamask.conversionDate']; /** * Transform date properties to value types, to ensure that state is @@ -15,8 +16,23 @@ const dateFields = ['metamask.conversionDate']; * * @param {unknown} data - The data to transform */ -function transformDates(data) { - for (const field of dateFields) { +function transformBackgroundDates(data) { + for (const field of backgroundDateFields) { + if (has(data, field)) { + set(data, field, typeof get(data, field)); + } + } + return data; +} + +/** + * Transform date properties to value types, to ensure that state is + * consistent between test runs. + * + * @param {unknown} data - The data to transform + */ +function transformUiDates(data) { + for (const field of uiDateFields) { if (has(data, field)) { set(data, field, typeof get(data, field)); } @@ -33,12 +49,10 @@ function transformDates(data) { * @param {boolean} [args.update] - Whether to update the snapshot if it doesn't match. */ async function matchesSnapshot({ - data: unprocessedData, + data, snapshot, update = process.env.UPDATE_SNAPSHOTS === 'true', }) { - const data = transformDates(unprocessedData); - const snapshotPath = resolve(__dirname, `./state-snapshots/${snapshot}.json`); const rawSnapshotData = await fs.readFile(snapshotPath, { encoding: 'utf-8', @@ -243,7 +257,7 @@ describe('Sentry errors', function () { const mockJsonBody = JSON.parse(mockTextBody[2]); const appState = mockJsonBody?.extra?.appState; await matchesSnapshot({ - data: appState, + data: transformBackgroundDates(appState), snapshot: 'errors-before-init-opt-in-background-state', }); }, @@ -328,7 +342,7 @@ describe('Sentry errors', function () { const mockJsonBody = JSON.parse(mockTextBody[2]); const appState = mockJsonBody?.extra?.appState; await matchesSnapshot({ - data: appState, + data: transformUiDates(appState), snapshot: 'errors-before-init-opt-in-ui-state', }); }, @@ -436,7 +450,8 @@ 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.store.MetaMetricsController; // Verify request assert.equal(type, 'TestError'); assert.equal(value, 'Test Error'); @@ -494,7 +509,7 @@ describe('Sentry errors', function () { 'Invalid version state', ); await matchesSnapshot({ - data: appState.store, + data: transformBackgroundDates(appState.store), snapshot: 'errors-after-init-opt-in-background-state', }); }, @@ -588,7 +603,7 @@ describe('Sentry errors', function () { 'Invalid version state', ); await matchesSnapshot({ - data: appState.store, + data: transformUiDates(appState.store), 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 index 03fc85edf..f41fee885 100644 --- 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 @@ -1,52 +1,32 @@ { - "metamask": { - "isInitialized": true, + "AccountTracker": { "currentBlockGasLimit": "0x1c9c380" }, + "AppStateController": { "connectedStatusPopoverHasBeenShown": true, - "defaultHomeActiveTabName": null, - "currentAppVersion": "10.34.4", - "previousAppVersion": "", - "previousMigrationVersion": 0, - "currentMigrationVersion": 94, + "defaultHomeActiveTabName": null + }, + "CurrencyController": { + "conversionDate": "number", + "conversionRate": 1700, + "nativeCurrency": "ETH", + "currentCurrency": "usd" + }, + "DecryptMessageController": { "unapprovedDecryptMsgCount": 0 }, + "EncryptionPublicKeyController": { + "unapprovedEncryptionPublicKeyMsgCount": 0 + }, + "MetaMetricsController": { + "participateInMetaMetrics": true, + "metaMetricsId": "fake-metrics-id" + }, + "NetworkController": { "networkId": "1337", "providerConfig": { "nickname": "Localhost 8545", "ticker": "ETH", "type": "rpc" - }, - "isUnlocked": false, - "useBlockie": false, - "useNonceField": false, - "usePhishDetect": true, - "featureFlags": { "showIncomingTransactions": true }, - "currentLocale": "en", - "forgottenPassword": false, - "preferences": { - "hideZeroBalanceTokens": false, - "showFiatInTestnets": false, - "showTestNetworks": false, - "useNativeCurrencyAsPrimaryCurrency": true - }, - "ipfsGateway": "dweb.link", - "participateInMetaMetrics": true, - "metaMetricsId": "fake-metrics-id", - "conversionDate": "number", - "conversionRate": 1700, - "nativeCurrency": "ETH", - "currentCurrency": "usd", - "alertEnabledness": { "unconnectedAccount": true, "web3ShimUsage": true }, - "seedPhraseBackedUp": true, - "firstTimeFlowType": "import", - "completedOnboarding": true, - "incomingTxLastFetchedBlockByChainId": { - "0x1": null, - "0xe708": null, - "0x5": null, - "0xaa36a7": null, - "0xe704": null - }, - "currentBlockGasLimit": "0x1c9c380", - "unapprovedDecryptMsgCount": 0, - "unapprovedEncryptionPublicKeyMsgCount": 0, + } + }, + "SignatureController": { "unapprovedMsgCount": 0, "unapprovedPersonalMsgCount": 0, "unapprovedTypedMessagesCount": 0