feat: add/improve Sentry instrumentation for storage related error handling cp-13.15.0

app/scripts/background.js

@@ -15,6 +15,7 @@ import { finished } from 'readable-stream';
 import log from 'loglevel';
 import browser from 'webextension-polyfill';
 import { isObject, hasProperty } from '@metamask/utils';
+import { deriveStateFromMetadata } from '@metamask/base-controller';
 import { ExtensionPortStream } from 'extension-port-stream';
 import { withResolvers } from '../../shared/lib/promise-with-resolvers';
 import { FirstTimeFlowType } from '../../shared/constants/onboarding';
@@ -1286,9 +1287,26 @@ export function setupController(
               // already updated this one
               return;
             }
-            const state = controller.controllerMessenger.call(
+            // Get the state for this backed-up key using messenger.
+            // We filter to only persistent properties using deriveStateFromMetadata
+            // to match what ComposableObservableStore does in stateChange events.
+            // This ensures non-persistent properties (e.g., KeyringController's
+            // isUnlocked, keyrings, encryptionKey) are not written to storage.
+            const controllerConfig = controller.store.config[key];
+            if (!controllerConfig?.metadata) {
+              throw new Error(
+                `Cannot backup ${key}: controller metadata is required but not found. ` +
+                  `All controllers in backedUpStateKeys must extend BaseController and define metadata.`,
+              );
+            }
+            const fullState = controller.controllerMessenger.call(
               `${key}:getState`,
             );
+            const state = deriveStateFromMetadata(
+              fullState,
+              controllerConfig.metadata,
+              'persist',
+            );
             persistenceManager.update(key, state);
           });
         }

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

@@ -27,6 +27,16 @@ globalThis.stateHooks.getPersistedState = async function () {
   return await sentryLocalStore.get({ validateVault: false });
 };
 
+/**
+ * Get the backup state from IndexedDB.
+ * This is used as a fallback when primary storage is unavailable.
+ *
+ * @returns The backup state, or null if unavailable.
+ */
+globalThis.stateHooks.getBackupState = async function () {
+  return await sentryLocalStore.getBackup();
+};
+
 const persistedStateMask = {
   data: SENTRY_BACKGROUND_STATE,
   meta: {

app/scripts/lib/setupSentry.js

@@ -205,6 +205,16 @@ function getMetaMetricsEnabledFromPersistedState(persistedState) {
   );
 }
 
+/**
+ * Returns whether MetaMetrics is enabled, given the backup state.
+ *
+ * @param {unknown} backupState - Backup state from IndexedDB
+ * @returns `true` if MetaMetrics is enabled in the backup, `false` otherwise.
+ */
+function getMetaMetricsEnabledFromBackupState(backupState) {
+  return Boolean(backupState?.MetaMetricsController?.participateInMetaMetrics);
+}
+
 function getSentryEnvironment() {
   if (METAMASK_BUILD_TYPE === 'main') {
     return METAMASK_ENVIRONMENT;
@@ -265,8 +275,15 @@ async function getMetaMetricsEnabled() {
     const persistedState = await globalThis.stateHooks.getPersistedState();
     return getMetaMetricsEnabledFromPersistedState(persistedState);
   } catch (error) {
-    log('Error retrieving persisted state', error);
-    return false;
+    log('Error retrieving persisted state, falling back to backup', error);
+    // Primary storage failed (e.g., database corruption) - check the backup
+    try {
+      const backupState = await globalThis.stateHooks.getBackupState();
+      return getMetaMetricsEnabledFromBackupState(backupState);
+    } catch (backupError) {
+      log('Error retrieving backup state', backupError);
+      return false;
+    }
   }
 }
 

app/scripts/lib/stores/persistence-manager.test.ts

@@ -3,7 +3,10 @@
 import 'navigator.locks';
 import log from 'loglevel';
 
-import { captureException } from '../../../../shared/lib/sentry';
+import {
+  captureException,
+  captureMessage,
+} from '../../../../shared/lib/sentry';
 import { MISSING_VAULT_ERROR } from '../../../../shared/constants/errors';
 import { PersistenceManager } from './persistence-manager';
 import ExtensionStore from './extension-store';
@@ -32,8 +35,10 @@ jest.mock('loglevel', () => ({
 }));
 jest.mock('../../../../shared/lib/sentry', () => ({
   captureException: jest.fn(),
+  captureMessage: jest.fn(),
 }));
 const mockedCaptureException = jest.mocked(captureException);
+const mockedCaptureMessage = jest.mocked(captureMessage);
 
 describe('PersistenceManager', () => {
   let manager: PersistenceManager;
@@ -78,7 +83,10 @@ describe('PersistenceManager', () => {
       mockStoreSet.mockRejectedValueOnce(error);
 
       await manager.set({ appState: { broken: true } });
-      expect(mockedCaptureException).toHaveBeenCalledWith(error);
+      expect(mockedCaptureException).toHaveBeenCalledWith(error, {
+        tags: { 'persistence.error': 'set-failed' },
+        fingerprint: ['persistence-error', 'set-failed'],
+      });
       expect(log.error).toHaveBeenCalledWith(
         'error setting state in local store:',
         error,
@@ -116,6 +124,43 @@ describe('PersistenceManager', () => {
 
       expect(mockedCaptureException).toHaveBeenCalledTimes(2);
     });
+
+    it('tracks recovery with captureMessage when store.set fails then succeeds', async () => {
+      manager.setMetadata({ version: 17 });
+
+      const error = new Error('store.set error');
+      mockStoreSet.mockRejectedValueOnce(error);
+
+      // First set fails
+      await manager.set({ appState: { broken: true } });
+
+      expect(mockedCaptureException).toHaveBeenCalledTimes(1);
+      expect(mockedCaptureMessage).not.toHaveBeenCalled();
+
+      // Second set succeeds - should trigger recovery tracking
+      mockStoreSet.mockResolvedValueOnce(undefined);
+      await manager.set({ appState: { fixed: true } });
+
+      expect(mockedCaptureMessage).toHaveBeenCalledTimes(1);
+      expect(mockedCaptureMessage).toHaveBeenCalledWith(
+        'Data persistence recovered after temporary failure',
+        {
+          level: 'info',
+          tags: { 'persistence.event': 'set-recovered' },
+          fingerprint: ['persistence-event', 'set-recovered'],
+        },
+      );
+    });
+
+    it('does not track recovery if set never failed', async () => {
+      manager.setMetadata({ version: 17 });
+
+      // Set succeeds without prior failure
+      mockStoreSet.mockResolvedValueOnce(undefined);
+      await manager.set({ appState: { working: true } });
+
+      expect(mockedCaptureMessage).not.toHaveBeenCalled();
+    });
   });
 
   describe('get', () => {
@@ -264,7 +309,10 @@ describe('PersistenceManager', () => {
 
       await manager.persist();
 
-      expect(mockedCaptureException).toHaveBeenCalledWith(error);
+      expect(mockedCaptureException).toHaveBeenCalledWith(error, {
+        tags: { 'persistence.error': 'persist-failed' },
+        fingerprint: ['persistence-error', 'persist-failed'],
+      });
       expect(log.error).toHaveBeenCalledWith(
         'error setting state in local store:',
         error,
@@ -423,7 +471,10 @@ describe('PersistenceManager', () => {
       // returns `undefined`
       expect(await brokenManager.getBackup()).toBeUndefined();
 
-      expect(mockedCaptureException).toHaveBeenCalledWith(domException);
+      expect(mockedCaptureException).toHaveBeenCalledWith(domException, {
+        tags: { 'persistence.error': 'backup-db-open-failed' },
+        fingerprint: ['persistence-error', 'backup-db-open-failed'],
+      });
       expect(consoleWarnSpy).toHaveBeenCalledWith(
         'Could not open backup database; automatic vault recovery will not be available.',
       );

app/scripts/lib/stores/persistence-manager.ts

@@ -1,7 +1,10 @@
 import log from 'loglevel';
 import { isEmpty } from 'lodash';
 import { RuntimeObject, hasProperty, isObject } from '@metamask/utils';
-import { captureException } from '../../../../shared/lib/sentry';
+import {
+  captureException,
+  captureMessage,
+} from '../../../../shared/lib/sentry';
 import { MISSING_VAULT_ERROR } from '../../../../shared/constants/errors';
 import { getManifestFlags } from '../../../../shared/lib/manifestFlags';
 import { IndexedDBStore } from './indexeddb-store';
@@ -22,12 +25,16 @@ export type Backup = {
   // TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
   // eslint-disable-next-line @typescript-eslint/naming-convention
   AppMetadataController?: unknown;
+  // TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31860
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  MetaMetricsController?: unknown;
   meta?: MetaData;
 };
 
 export const backedUpStateKeys = [
   'KeyringController',
   'AppMetadataController',
+  'MetaMetricsController',
 ] as const;
 
 export type BackedUpStateKey = (typeof backedUpStateKeys)[number];
@@ -237,7 +244,13 @@ export class PersistenceManager {
           error.message ===
             'A mutation operation was attempted on a database that did not allow mutations.'
         ) {
-          captureException(error);
+          // Custom fingerprint prevents Sentry's deduplication from dropping
+          // this event when other persistence errors with the same underlying
+          // error message (e.g., "An unexpected error occurred") are reported.
+          captureException(error, {
+            tags: { 'persistence.error': 'backup-db-open-failed' },
+            fingerprint: ['persistence-error', 'backup-db-open-failed'],
+          });
           console.warn(
             'Could not open backup database; automatic vault recovery will not be available.',
           );
@@ -326,6 +339,8 @@ export class PersistenceManager {
       { mode: 'exclusive', signal: abortController.signal },
       async () => {
         this.#currentLockAbortController = undefined;
+        // Track which operation failed to use the correct Sentry tag
+        let backupFailed = false;
         try {
           // atomically set all the keys
           await this.#localStore.set({
@@ -339,19 +354,44 @@ export class PersistenceManager {
             const stringifiedBackup = JSON.stringify(backup);
             // and the backup has changed
             if (this.#backup !== stringifiedBackup) {
-              // save it to the backup DB
-              await this.#backupDb?.set(backup);
-              this.#backup = stringifiedBackup;
+              // save it to the backup DB - wrapped in try-catch to differentiate
+              // backup failures from storage.local failures in Sentry
+              try {
+                await this.#backupDb?.set(backup);
+                this.#backup = stringifiedBackup;
+              } catch (backupErr) {
+                backupFailed = true;
+                throw backupErr;
+              }
             }
           }
 
           if (this.#dataPersistenceFailing) {
             this.#dataPersistenceFailing = false;
+            // Track recovery to understand how often failures are temporary.
+            // This helps answer: "Do set calls ever fail and then succeed in the same session?"
+            captureMessage(
+              'Data persistence recovered after temporary failure',
+              {
+                level: 'info',
+                tags: { 'persistence.event': 'set-recovered' },
+                fingerprint: ['persistence-event', 'set-recovered'],
+              },
+            );
           }
         } catch (err) {
           if (!this.#dataPersistenceFailing) {
             this.#dataPersistenceFailing = true;
-            captureException(err);
+            // Use different tags to differentiate storage.local vs IndexedDB backup failures.
+            const tag = backupFailed ? 'set-backup-failed' : 'set-failed';
+
+            // Custom fingerprint prevents Sentry's deduplication from dropping
+            // this event when other persistence errors with the same underlying
+            // error message (e.g., "An unexpected error occurred") are reported.
+            captureException(err, {
+              tags: { 'persistence.error': tag },
+              fingerprint: ['persistence-error', tag],
+            });
           }
           this.#notifySetFailed();
           log.error('error setting state in local store:', err);
@@ -413,6 +453,8 @@ export class PersistenceManager {
       { mode: 'exclusive', signal: abortController.signal },
       async () => {
         this.#currentLockAbortController = undefined;
+        // Track which operation failed to use the correct Sentry tag
+        let backupFailed = false;
         try {
           const clone = structuredClone(this.#pendingPairs);
           // reset the pendingPairs
@@ -441,17 +483,44 @@ export class PersistenceManager {
           }
           if (hasVault(partialState)) {
             const backup = makeBackup(partialState, meta);
-            // save it to the backup DB
-            await this.#backupDb?.set(backup);
+            // save it to the backup DB - wrapped in try-catch to differentiate
+            // backup failures from storage.local failures in Sentry
+            try {
+              await this.#backupDb?.set(backup);
+            } catch (backupErr) {
+              backupFailed = true;
+              throw backupErr;
+            }
           }
 
           if (this.#dataPersistenceFailing) {
             this.#dataPersistenceFailing = false;
+            // Track recovery to understand how often failures are temporary.
+            // This helps answer: "Do set calls ever fail and then succeed in the same session?"
+            captureMessage(
+              'Data persistence recovered after temporary failure',
+              {
+                level: 'info',
+                tags: { 'persistence.event': 'persist-recovered' },
+                fingerprint: ['persistence-event', 'persist-recovered'],
+              },
+            );
           }
         } catch (err) {
           if (!this.#dataPersistenceFailing) {
             this.#dataPersistenceFailing = true;
-            captureException(err);
+            // Use different tags to differentiate storage.local vs IndexedDB backup failures.
+            const tag = backupFailed
+              ? 'persist-backup-failed'
+              : 'persist-failed';
+
+            // Custom fingerprint prevents Sentry's deduplication from dropping
+            // this event when other persistence errors with the same underlying
+            // error message (e.g., "An unexpected error occurred") are reported.
+            captureException(err, {
+              tags: { 'persistence.error': tag },
+              fingerprint: ['persistence-error', tag],
+            });
           }
           this.#notifySetFailed();
           log.error('error setting state in local store:', err);
@@ -493,12 +562,19 @@ export class PersistenceManager {
           ])
           .catch((error: Error): [Error, undefined] => [error, undefined]);
 
-        // Log the error if one occurred, but don't throw yet
+        // Log and capture the error if one occurred, but don't throw yet
         if (localStoreError) {
           log.error(
             'Error retrieving the current state of the local store:',
             localStoreError,
           );
+          // Custom fingerprint prevents Sentry's deduplication from dropping
+          // this event when other persistence errors with the same underlying
+          // error message (e.g., "An unexpected error occurred") are reported.
+          captureException(localStoreError, {
+            tags: { 'persistence.error': 'get-failed' },
+            fingerprint: ['persistence-error', 'get-failed'],
+          });
         }
 
         if (validateVault) {
@@ -601,12 +677,16 @@ export class PersistenceManager {
     if (!backupDb) {
       return undefined;
     }
-    const [KeyringController, AppMetadataController, meta] = await backupDb.get(
-      [...backedUpStateKeys, `meta`],
-    );
+    const [
+      KeyringController,
+      AppMetadataController,
+      MetaMetricsController,
+      meta,
+    ] = await backupDb.get([...backedUpStateKeys, `meta`]);
     return {
       KeyringController,
       AppMetadataController,
+      MetaMetricsController,
       meta: meta as MetaData | undefined,
     };
   }