Skip to content

Commit 0053df5

Browse files
feat: add workarounds for database corruption scenarios (#39010)
## **Description** This PR implements workarounds for Firefox database corruption issues that cause `browser.storage.local` operations to fail with "An unexpected error occurred". ### Problem Firefox stores extension data in SQLite databases with external files for large blobs. Some corruption scenarios, like [missing or renamed external files (file IDs mismatch)](#9196 (comment)), can break these databases. When this happens, `browser.storage.local.get()` or `browser.storage.local.set()` fail entirely, leaving users locked out of their wallet: - #10091 - #35681 ### Solution This PR adds two recovery mechanisms: 1. **Vault Recovery on `get()` failure**: When `browser.storage.local.get()` fails and a backup vault exists in IndexedDB, users are redirected to the vault recovery flow to restore their wallet. 2. **Informative Toast on `set()` failure**: When `browser.storage.local.set()` fails (even after vault recovery), a persistent toast is displayed informing users that the storage is failing and MetaMask needs to be reinstalled. The toast includes a link to save the Secret Recovery Phrase before reinstalling. [Slack thread](https://consensys.slack.com/archives/C8RSKCNCD/p1766165879957189?thread_ts=1764958704.459329&cid=C8RSKCNCD) ### Out of scope for this PR - Add instrumentation (Sentry or Segment) - Instrumentation will be added in a different PR: #39113 - Add new scenarios to storage corruption E2E tests. - Storage corruption E2E tests are currently disabled (cf. this ticket: #38080). - We'll wait until Accounts team enables BIP44 by default in the codebase (it's currently enabled enabled using a feature flag) before re-enabling storage corruption E2E tests. Otherwise these tests would be flaky (cf. this [Slack thread](https://consensys.slack.com/archives/C01U65ZUS2E/p1767717796349449?thread_ts=1767634209.723779&cid=C01U65ZUS2E)). [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/39010?quickstart=1) ## **Changelog** CHANGELOG entry: Fixes Firefox database corruption causing "An unexpected error occurred" by redirecting to vault recovery when possible, or displaying guidance to reinstall when recovery is not possible. ## **Related issues** Fixes: #10091 Fixes: #35681 ## **Manual testing steps** ### Prerequisites - Firefox browser - MetaMask installed from Firefox Add-ons store (or local build with `yarn dist:mv2`) - The `test-vault-corruption-firefox.sh` script from this [PR](#38940) ### Test 1: Vault Recovery flow started on storage `get()` failure 1. Install MetaMask and complete onboarding 2. Close Firefox completely 3. Run `./test-vault-corruption-firefox.sh` and select option **5** (Rename files off-by-one) 4. Open Firefox and click the MetaMask icon 5. **Expected**: You should see the vault recovery screen with "Restore Accounts" button 6. Click "Restore Accounts", enter your password, complete onboarding 7. **Expected**: Wallet is restored with original accounts ### Test 2: Toast displayed on storage `set()` failure 8. **Expected**: A toast is displayed: "We couldn't save your data - Back up your Secret Recovery Phrase and reinstall MetaMask if the problem continues." 9. Click "Back up Secret Recovery Phrase" 10. **Expected**: You are navigated to the SRP reveal page ## **Screenshots/Recordings** ### **Before** Users see "An unexpected error occurred" with no recovery path. <img width="426" height="410" alt="Screenshot 2026-01-05 at 06 07 46" src="https://github.com/user-attachments/assets/a888167c-55ce-4e80-9d83-a53058df06b5" /> ### **After** 1. **Vault Recovery Flow** (when reads fail): - Users see the vault recovery screen and can restore their wallet (when backup exists) <img width="384" height="578" alt="Screenshot 2026-01-03 at 16 07 30" src="https://github.com/user-attachments/assets/f64cc4e1-e3ea-4d5a-a635-93cdf687835e" /> 2. **Storage Error Toast** (when writes fail): - Toast appears with message: "We couldn't save your data" - Description: "Back up your Secret Recovery Phrase and reinstall MetaMask if the problem continues." - Action button: "Back up Secret Recovery Phrase" → navigates to SRP reveal <img width="1521" height="749" alt="Screenshot 2026-01-07 at 23 14 01" src="https://github.com/user-attachments/assets/e65be4c1-9dd2-492c-b689-13ebf86c6053" /> [Loom recording](https://www.loom.com/share/d51c11812db54a9d8aa29465798bdd33) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Introduces recovery UX and error signaling for storage failures, plus improved error context. > > - Persistence: Enhance `PersistenceManager.get()` to handle `storage.local.get` errors, trigger vault recovery via `PersistenceError` (now includes `cause`) when an IndexedDB backup exists; on `set` failures, notify UI via a new `setOnSetFailed` callback (background wires this to `AppStateController.setShowStorageErrorToast(true)`). > - State/Types: Add `appState.showStorageErrorToast` (non-persisted) and expose in Sentry snapshots and background types. > - UI: Add `StorageErrorToast` rendered globally in `ToastMaster` (gated by `selectShowStorageErrorToast`), with action to navigate to SRP reveal; add i18n strings for title/description/action. > - Error delivery: In state-corruption handling, include `causeMessage` from `PersistenceError.cause` when notifying UI. > - Tests/fixtures: Update unit/e2e/integration snapshots and mocks to cover new flags, behaviors, and logging. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 03a318d. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c4b0294 commit 0053df5

File tree

18 files changed

+294
-21
lines changed

18 files changed

+294
-21
lines changed

app/_locales/en/messages.json

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/_locales/en_GB/messages.json

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/scripts/background.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ global.stateHooks.getStorageKind = () => persistenceManager.storageKind;
131131

132132
/**
133133
* A helper function to log the current state of the vault. Useful for debugging
134-
* purposes, to, in the case of database corruption, an possible way for an end
134+
* purposes, to, in the case of storage errors, a possible way for an end
135135
* user to recover their vault. Hopefully this is never needed.
136136
*/
137137
global.logEncryptedVault = () => {
@@ -1223,6 +1223,11 @@ export function setupController(
12231223
cronjobControllerStorageManager,
12241224
});
12251225

1226+
// Wire up the callback to notify the UI when set operations fail
1227+
persistenceManager.setOnSetFailed(() => {
1228+
controller.appStateController.setShowStorageErrorToast(true);
1229+
});
1230+
12261231
/**
12271232
* @type {Array<string>} List of controller store keys that have changed since initialization.
12281233
*/

app/scripts/constants/sentry-state.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ export const SENTRY_BACKGROUND_STATE = {
9393
lastUpdatedAt: true,
9494
shieldEndingToastLastClickedOrClosed: true,
9595
shieldPausedToastLastClickedOrClosed: true,
96+
showStorageErrorToast: true,
9697
isWalletResetInProgress: false,
9798
pna25Acknowledged: false,
9899
},

app/scripts/controllers/app-state-controller.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,7 @@ describe('AppStateController', () => {
807807
"showNetworkBanner": true,
808808
"showPermissionsTour": true,
809809
"showShieldEntryModalOnce": null,
810+
"showStorageErrorToast": false,
810811
"showTestnetMessageInDropdown": true,
811812
"sidePanelGasPollTokens": [],
812813
"signatureSecurityAlertResponses": {},
@@ -900,6 +901,7 @@ describe('AppStateController', () => {
900901
"showNetworkBanner": true,
901902
"showPermissionsTour": true,
902903
"showShieldEntryModalOnce": null,
904+
"showStorageErrorToast": false,
903905
"showTestnetMessageInDropdown": true,
904906
"sidePanelGasPollTokens": [],
905907
"signatureSecurityAlertResponses": {},
@@ -1073,6 +1075,7 @@ describe('AppStateController', () => {
10731075
"showNetworkBanner": true,
10741076
"showPermissionsTour": true,
10751077
"showShieldEntryModalOnce": null,
1078+
"showStorageErrorToast": false,
10761079
"sidePanelGasPollTokens": [],
10771080
"signatureSecurityAlertResponses": {},
10781081
"slides": [],

app/scripts/controllers/app-state-controller.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,12 @@ export type AppStateControllerState = {
166166
* Whether the wallet reset is in progress.
167167
*/
168168
isWalletResetInProgress: boolean;
169+
170+
/**
171+
* Whether to show the storage error toast.
172+
* This is set to true when set operations fail (storage.local or IndexedDB).
173+
*/
174+
showStorageErrorToast: boolean;
169175
};
170176

171177
const controllerName = 'AppStateController';
@@ -326,6 +332,7 @@ const getDefaultAppStateControllerState = (): AppStateControllerState => ({
326332
pendingShieldCohortTxType: null,
327333
isWalletResetInProgress: false,
328334
dappSwapComparisonData: {},
335+
showStorageErrorToast: false,
329336
...getInitialStateOverrides(),
330337
});
331338

@@ -711,6 +718,12 @@ const controllerMetadata: StateMetadata<AppStateControllerState> = {
711718
includeInDebugSnapshot: false,
712719
usedInUi: true,
713720
},
721+
showStorageErrorToast: {
722+
includeInStateLogs: true,
723+
persist: false,
724+
includeInDebugSnapshot: true,
725+
usedInUi: true,
726+
},
714727
};
715728

716729
export class AppStateController extends BaseController<
@@ -938,6 +951,18 @@ export class AppStateController extends BaseController<
938951
});
939952
}
940953

954+
/**
955+
* Sets whether to show the storage error toast.
956+
* This is called when set operations fail (storage.local or IndexedDB).
957+
*
958+
* @param show - Whether to show the toast
959+
*/
960+
setShowStorageErrorToast(show: boolean): void {
961+
this.update((state) => {
962+
state.showStorageErrorToast = show;
963+
});
964+
}
965+
941966
/**
942967
* Replaces slides in state with new slides. If a slide with the same id
943968
* already exists, it will be merged with the new slide.

app/scripts/lib/state-corruption/state-corruption-recovery.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ const mockBrokenPersistence = (error: Error): PersistenceManager =>
4343
getBackup: jest.fn().mockRejectedValue(error),
4444
}) as unknown as PersistenceManager;
4545

46+
// The cause error message that is always included in PersistenceError
47+
// to test that causeMessage is properly extracted and passed to the UI
48+
const CAUSE_ERROR_MESSAGE = 'Error: An unexpected error occurred';
49+
4650
describe('CorruptionHandler.handleStateCorruptionError', () => {
4751
let corruptionHandler: CorruptionHandler;
4852
beforeEach(() => {
@@ -107,10 +111,14 @@ describe('CorruptionHandler.handleStateCorruptionError', () => {
107111

108112
// some cases of Corruption detection will have a `backup` already
109113
// present in the `error` object, this sets that case up.
114+
// We always include a cause to test that causeMessage is properly
115+
// extracted and passed to the UI across all scenarios.
116+
const cause = new Error(CAUSE_ERROR_MESSAGE);
110117
const error = new PersistenceError(
111118
'Corrupted',
112119
// `backup` is not always a `Backup`, but in reality that is also true
113120
backupHasErr ? (backup as Backup) : null,
121+
cause,
114122
);
115123

116124
// handle the case where `getBackup` function returns an error. We can't
@@ -172,12 +180,13 @@ describe('CorruptionHandler.handleStateCorruptionError', () => {
172180
);
173181

174182
// make sure the `corruptionFn` was called with the expected error
175-
// message
183+
// message, including the causeMessage extracted from the cause
176184
expect(corruptionFn).toHaveBeenCalledWith({
177185
error: {
178186
message: error.message,
179187
name: error.name,
180188
stack: error.stack,
189+
causeMessage: CAUSE_ERROR_MESSAGE,
181190
},
182191
...result,
183192
});

app/scripts/lib/state-corruption/state-corruption-recovery.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import {
33
METHOD_DISPLAY_STATE_CORRUPTION_ERROR,
44
METHOD_REPAIR_DATABASE,
55
} from '../../../../shared/constants/state-corruption';
6-
import { type Backup, PersistenceManager } from '../stores/persistence-manager';
6+
import {
7+
type Backup,
8+
PersistenceError,
9+
PersistenceManager,
10+
} from '../stores/persistence-manager';
711
import { ErrorLike } from '../../../../shared/constants/errors';
812
import { tryPostMessage } from '../start-up-errors/start-up-errors';
913
import { RELOAD_WINDOW } from '../../../../shared/constants/start-up-errors';
@@ -102,6 +106,25 @@ function maybeGetCurrentLocale(backup: Backup | null): string | null {
102106
return null;
103107
}
104108

109+
/**
110+
* Attempts to get the cause message from a PersistenceError.
111+
* This provides additional context about the original error that caused
112+
* the persistence failure (e.g., Firefox's "Error: An unexpected error occurred").
113+
*
114+
* @param error - The error to extract the cause message from.
115+
* @returns The cause message if available, otherwise null.
116+
*/
117+
function maybeGetCauseMessage(error: ErrorLike): string | null {
118+
if (
119+
error instanceof PersistenceError &&
120+
error.cause instanceof Error &&
121+
error.cause.message
122+
) {
123+
return error.cause.message;
124+
}
125+
return null;
126+
}
127+
105128
/**
106129
* Checks if the backup object has a vault.
107130
*
@@ -156,13 +179,17 @@ export class CorruptionHandler {
156179
// it is not worth claiming we have a backup if the vault doesn't actually
157180
// exist
158181
const hasBackup = Boolean(hasVault(backup));
182+
// Extract cause message if available (e.g., Firefox's "Error: An unexpected error occurred")
183+
// This helps users and customer support debug issues
184+
const causeMessage = maybeGetCauseMessage(error);
159185

160186
// send the `error` to the UI for this port
161187
const sent = tryPostMessage(port, METHOD_DISPLAY_STATE_CORRUPTION_ERROR, {
162188
error: {
163189
message: error.message,
164190
name: error.name,
165191
stack: error.stack,
192+
causeMessage,
166193
},
167194
currentLocale,
168195
hasBackup,

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ jest.mock('./extension-store', () => {
2121
});
2222
jest.mock('loglevel', () => ({
2323
error: jest.fn(),
24+
info: jest.fn(),
2425
}));
2526
jest.mock('../../../../shared/lib/sentry', () => ({
2627
captureException: jest.fn(),
@@ -109,14 +110,14 @@ describe('PersistenceManager', () => {
109110

110111
describe('get', () => {
111112
it('returns undefined and clears mostRecentRetrievedState if store returns empty', async () => {
112-
mockStoreGet.mockReturnValueOnce({});
113+
mockStoreGet.mockResolvedValueOnce({});
113114
const result = await manager.get({ validateVault: false });
114115
expect(result).toBeUndefined();
115116
expect(manager.mostRecentRetrievedState).toBeNull();
116117
});
117118

118119
it('returns undefined if store returns null', async () => {
119-
mockStoreGet.mockReturnValueOnce(null);
120+
mockStoreGet.mockResolvedValueOnce(null);
120121
const result = await manager.get({ validateVault: false });
121122
expect(result).toBeUndefined();
122123
expect(manager.mostRecentRetrievedState).toBeNull();

0 commit comments

Comments
 (0)