Fix pre-initialization UI error state capture (#20529)

In the case where an error is thrown in the UI before initialization
has finished, we aren't capturing the application state correctly for
Sentry errors. We had a test case for this, but the test case was
broken due to a mistake in how the `network-store` was setup (it was
not matching the behavior of the real `local-tstore` module).

The pre-initialization state capture logic was updated to rely solely
upon the `localStore` instance used by Sentry to determine whether the
user had opted-in to metrics or not. This simplifies the logic a great
deal, removing the need for the `getMostRecentPersistedState` state
hook. It also ensures that state is captured corretly pre-
initialization in both the background and UI.

app/scripts/background.js

@@ -71,12 +71,6 @@ 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 };
 
@@ -100,6 +94,9 @@ 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) {

app/scripts/lib/network-store.js

@@ -31,7 +31,6 @@ 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}'`);
@@ -49,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;
   }
 

app/scripts/lib/setup-initial-state-hooks.js

@@ -5,7 +5,9 @@ import ReadOnlyNetworkStore from './network-store';
 import { SENTRY_BACKGROUND_STATE } from './setupSentry';
 
 const platform = new ExtensionPlatform();
-const localStore = process.env.IN_TEST
+
+// This instance of `localStore` is used by Sentry to get the persisted state
+const sentryLocalStore = process.env.IN_TEST
   ? new ReadOnlyNetworkStore()
   : new LocalStore();
 
@@ -15,7 +17,7 @@ const localStore = process.env.IN_TEST
  * @returns The persisted wallet state.
  */
 globalThis.stateHooks.getPersistedState = async function () {
-  return await localStore.get();
+  return await sentryLocalStore.get();
 };
 
 const persistedStateMask = {
@@ -37,25 +39,30 @@ globalThis.stateHooks.getSentryState = function () {
     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 (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;
+  } else if (
+    // This is truthy if Sentry has retrieved state at least once already. This
+    // should always be true because Sentry calls `getPersistedState` during
+    // error processing (before this function is called) if `getSentryAppState`
+    // hasn't been set yet.
+    sentryLocalStore.mostRecentRetrievedState
+  ) {
+    return {
+      ...sentryState,
+      persistedState: maskObject(
+        sentryLocalStore.mostRecentRetrievedState,
+        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, unless an error was encountered during error
+  // processing.
   return sentryState;
 };

app/scripts/ui.js

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