m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-25 03:20:23 +01:00
Code

Improve Sentry state pre-initialization (#20491)

Browse Source 
* Improve Sentry state pre-initialization

Previously the masked state snapshot sent to Sentry would be blank for
errors that occured during initialization. Instead we'll now include
some basic information in all cases, and a masked copy of the persisted
state if it happens after the first time the persisted state is read.

* Add test

* Fix crash when persisted state not yet fetched

* Add descriptions for initial state hooks

* Update comments to reflect recent changes

* Re-order imports to follow conventions

* Move initial state hooks back to module-level

The initial state hooks are now setup at the top-level of their module.
This ensures that they're setup prior to later imports. Calling a
function to setup these hooks in the entrypoint module wouldn't
accomplish this even if it was run "before" the imports because ES6
imports always get hoisted to the top of the file.

The `localStore` instance wasn't available statically, so a new state
hook was introduced for retrieving the most recent retrieved persisted
state.

* Fix error e2e tests
This commit is contained in:
Mark Stacey 2023-08-17 09:29:05 -02:30 committed by Dan Miller
parent 805ce29e11
commit 80746e67b5
11 changed files with 408 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
app/scripts/background.js
View File

@ -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) {
@ -889,17 +894,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);
};
}

3
app/scripts/lib/local-store.js
View File

@ -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;
}

57
app/scripts/lib/local-store.test.js
View File

@ -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);
});
});
});

2
app/scripts/lib/network-store.js
View File

@ -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}'`);

61
app/scripts/lib/setup-initial-state-hooks.js Normal file
View File

@ -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;
};

10
app/scripts/lib/setup-persisted-state-hook.js
View File

@ -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();
};

18
app/scripts/ui.js
View File

@ -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';
@ -34,6 +36,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');

58
test/e2e/tests/errors.spec.js
View File

@ -197,9 +197,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
@ -286,8 +286,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',
});
},
@ -311,9 +326,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
@ -354,9 +369,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
@ -371,8 +386,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',
});
},
@ -481,7 +511,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');
@ -525,8 +555,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' &&
@ -539,7 +569,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',
});
},
@ -577,7 +607,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');
@ -619,8 +649,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' &&
@ -633,7 +663,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',
});
},

112
test/e2e/tests/state-snapshots/errors-before-init-opt-in-background-state.json
View File

@ -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"
}
}

113
test/e2e/tests/state-snapshots/errors-before-init-opt-in-ui-state.json
View File

@ -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 }
}

9
ui/index.js
View File

@ -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);
};
}