m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-22 09:57:02 +01:00
Code

Fix Sentry breadcrumb collection during initialization (again) (#20532)

Browse Source 
Sentry breadcrumb collection during initialization was broken in #20529
because we failed to consider that the `getSentryState` check was also
used for an opt-in check in the `beforeBreadcrumb` hook.

I had assumed that `getSentryState` was only used to get state to add
additional context to an error report. But the function has a second
purpose: to get state for the purposes of checking whether the user has
opted into MetaMetrics. In this second case, `mostRecentRetrievedState`
is sometimes unset (which violates an assumption made in #20529)

The `getMostRecentPersistedState` hook removed in #20529 has been
restored, ensuring that the `getSentryState` function returns Sentry
state after loading state for the first time, but before the first
error has occurred.

This mistake didn't cause e2e tests to fail because multiple errors are
currently thrown in the background upon initialization on `develop`
(relating to Snow scuttling). These errors were early enough that they
happened before the console logs that our breadcrumb test was testing
for. When #20529 was ported onto the v10.34.5 RC, these errors were not
present so the test failed correctly.
This commit is contained in:
Mark Stacey 2023-08-18 18:26:27 -02:30 committed by GitHub
parent dc6069a3ab
commit 885a8ce256
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
app/scripts/background.js
View File

@ -71,6 +71,12 @@ import DesktopManager from '@metamask/desktop/dist/desktop-manager';
///: END:ONLY_INCLUDE_IN ///: END:ONLY_INCLUDE_IN
/* eslint-enable import/order */ /* 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 { sentry } = global;
const firstTimeState = { ...rawFirstTimeState }; const firstTimeState = { ...rawFirstTimeState };
@ -93,10 +99,6 @@ let uiIsTriggering = false;
const openMetamaskTabsIDs = {}; const openMetamaskTabsIDs = {};
const requestAccountTabIds = {}; const requestAccountTabIds = {};
let controller; let controller;
// state persistence
const inTest = process.env.IN_TEST;
const localStore = inTest ? new ReadOnlyNetworkStore() : new LocalStore();
let versionedData; let versionedData;
if (inTest || process.env.METAMASK_DEBUG) { if (inTest || process.env.METAMASK_DEBUG) {

34
app/scripts/lib/setup-initial-state-hooks.js
View File

@ -28,9 +28,12 @@ const persistedStateMask = {
}; };
/** /**
* Get a state snapshot to include with Sentry error reports. This uses the * Get a state snapshot for Sentry. This is used to add additional context to
* persisted state pre-initialization, and the in-memory state post- * error reports, and it's used when processing errors and breadcrumbs to
* initialization. In both cases the state is anonymized. * 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. * @returns A Sentry state snapshot.
*/ */
@ -47,22 +50,27 @@ globalThis.stateHooks.getSentryState = function () {
}; };
} else if ( } else if (
// This is truthy if Sentry has retrieved state at least once already. This // This is truthy if Sentry has retrieved state at least once already. This
// should always be true because Sentry calls `getPersistedState` during // should always be true when getting context for an error report, but can
// error processing (before this function is called) if `getSentryAppState` // be unset when Sentry is performing the opt-in check.
// hasn't been set yet. sentryLocalStore.mostRecentRetrievedState ||
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 { return {
...sentryState, ...sentryState,
persistedState: maskObject( persistedState: maskObject(persistedState, persistedStateMask),
sentryLocalStore.mostRecentRetrievedState,
persistedStateMask,
),
}; };
} }
}
// This branch means that local storage has not yet been read, so we have // This branch means that local storage has not yet been read, so we have
// no choice but to omit the application state. // no choice but to omit the application state.
// This should be unreachable, unless an error was encountered during error // This should be unreachable when getting context for an error report, but
// processing. // can be false when Sentry is performing the opt-in check.
return sentryState; return sentryState;
}; };