From 0abff79fd6d91b6524067d166fc5fb6d848c40ce Mon Sep 17 00:00:00 2001 From: Danica Shen Date: Tue, 24 Sep 2024 23:02:29 +0100 Subject: [PATCH] Fix the error when loading with deleted indexDB --- .../lib/state-management/local-store.test.js | 110 +++++++++++++--- .../lib/state-management/local-store.ts | 119 +++++++++++++----- 2 files changed, 181 insertions(+), 48 deletions(-) diff --git a/app/scripts/lib/state-management/local-store.test.js b/app/scripts/lib/state-management/local-store.test.js index e4c2b5ccc5c5..57820a4dcefb 100644 --- a/app/scripts/lib/state-management/local-store.test.js +++ b/app/scripts/lib/state-management/local-store.test.js @@ -1,3 +1,4 @@ +import log from 'loglevel'; import LocalStore from './local-store'; const mockIDBRequest = (result, isError) => { @@ -35,6 +36,9 @@ const createEmptySetup = () => ), }); +const MOCK_STATE = { appState: { test: true } }; +const MOCK_VERSION_DATA = { version: 74 }; + describe('LocalStore', () => { let setup; beforeEach(() => { @@ -45,9 +49,7 @@ describe('LocalStore', () => { mockIDBRequest({ transaction: jest.fn(() => ({ objectStore: jest.fn(() => ({ - get: jest.fn(() => - mockIDBRequest({ appState: { test: true } }), - ), + get: jest.fn(() => mockIDBRequest(MOCK_STATE)), put: jest.fn(() => mockIDBRequest({})), })), })), @@ -75,9 +77,31 @@ describe('LocalStore', () => { }); }); + describe('_getObjectStore', () => { + it('should reinitialize IndexedDB and return the object store when INVALID_STATE_ERROR occurs', async () => { + const localStore = setup(); + + // Mock initial failure with INVALID_STATE_ERROR + const error = new Error('Mock InvalidStateError'); + error.name = 'InvalidStateError'; + localStore.dbReady = Promise.reject(error); + + // Mock the _init function to resolve successfully after reinitialization + const mockDb = { + transaction: jest.fn(() => ({ + objectStore: jest.fn(() => MOCK_STATE), + })), + }; + jest.spyOn(localStore, '_init').mockResolvedValueOnce(mockDb); + const objectStore = await localStore._getObjectStore('readonly'); + expect(localStore._init).toHaveBeenCalled(); + expect(objectStore).toStrictEqual(MOCK_STATE); + }); + }); + describe('setMetadata', () => { it('should set the metadata property on LocalStore', () => { - const metadata = { version: 74 }; + const metadata = MOCK_VERSION_DATA; const localStore = setup(); localStore.setMetadata(metadata); @@ -95,27 +119,70 @@ describe('LocalStore', () => { it('should throw an error if passed a valid argument but metadata has not yet been set', async () => { const localStore = setup(); - await expect(() => - localStore.set({ appState: { test: true } }), - ).rejects.toThrow( + await expect(() => localStore.set(MOCK_STATE)).rejects.toThrow( 'MetaMask - metadata must be set on instance of ExtensionStore before calling "set"', ); }); it('should not throw if passed a valid argument and metadata has been set', async () => { const localStore = setup(); - localStore.setMetadata({ version: 74 }); + localStore.setMetadata(MOCK_VERSION_DATA); await expect(async () => { - await localStore.set({ appState: { test: true } }); + await localStore.set(MOCK_STATE); }).not.toThrow(); }); it('should set isExtensionInitialized if data is set with no error', async () => { const localStore = setup(); - localStore.setMetadata({ version: 74 }); - await localStore.set({ appState: { test: true } }); + localStore.setMetadata(MOCK_VERSION_DATA); + await localStore.set(MOCK_STATE); expect(localStore.isExtensionInitialized).toBeTruthy(); }); + + it('should fallback to in-memory cache if IndexedDB is not available', async () => { + const localStore = setup(); + localStore.setMetadata(MOCK_VERSION_DATA); + jest.spyOn(localStore, '_writeToDB').mockImplementationOnce(() => { + const error = new Error('Mock error'); + error.name = 'InvalidStateError'; + throw error; + }); + + await localStore.set({ appState: { test: true } }); + expect(localStore.inMemoryCache).toStrictEqual({ + id: 'metamaskState', + data: MOCK_STATE, + meta: MOCK_VERSION_DATA, + }); + }); + + it('should handle IndexedDB error and log the error', async () => { + const localStore = setup(); + localStore.setMetadata(MOCK_VERSION_DATA); + const logSpy = jest + .spyOn(log, 'error') + .mockImplementation(() => undefined); + jest + .spyOn(localStore, '_writeToDB') + .mockRejectedValueOnce(new Error('Mock error')); + + await localStore.set(MOCK_STATE); + expect(logSpy).toHaveBeenCalledWith( + 'Error setting state in IndexedDB:', + expect.any(Error), + ); + }); + + it('should set dataPersistenceFailing to true when IndexedDB fails', async () => { + const localStore = setup(); + localStore.setMetadata(MOCK_VERSION_DATA); + jest + .spyOn(localStore, '_writeToDB') + .mockRejectedValueOnce(new Error('InvalidStateError')); + + await localStore.set(MOCK_STATE); + expect(localStore.dataPersistenceFailing).toBe(true); + }); }); describe('get', () => { @@ -135,9 +202,7 @@ describe('LocalStore', () => { await localStore.get(); - expect(localStore.mostRecentRetrievedState).toStrictEqual({ - appState: { test: true }, - }); + expect(localStore.mostRecentRetrievedState).toStrictEqual(MOCK_STATE); }); it('should reset mostRecentRetrievedState to null if storage is empty', async () => { @@ -153,11 +218,24 @@ describe('LocalStore', () => { it('should set mostRecentRetrievedState to current state if isExtensionInitialized is true', async () => { const localStore = setup(); - localStore.setMetadata({ version: 74 }); - await localStore.set({ appState: { test: true } }); + localStore.setMetadata(MOCK_VERSION_DATA); + await localStore.set(MOCK_STATE); await localStore.get(); expect(localStore.mostRecentRetrievedState).toStrictEqual(null); }); + + it('should fallback to in-memory cache if IndexedDB is not available', async () => { + const localStore = setup(); + jest.spyOn(localStore, '_readFromDB').mockResolvedValueOnce(null); + // Set the in-memory cache + localStore.inMemoryCache = { + id: 'metamaskState', + data: MOCK_STATE, + meta: MOCK_VERSION_DATA, + }; + const result = await localStore.get(); + expect(result).toStrictEqual(localStore.inMemoryCache); + }); }); describe('cleanUpMostRecentRetrievedState', () => { diff --git a/app/scripts/lib/state-management/local-store.ts b/app/scripts/lib/state-management/local-store.ts index 6d3944831ffd..a9c7c5635062 100644 --- a/app/scripts/lib/state-management/local-store.ts +++ b/app/scripts/lib/state-management/local-store.ts @@ -48,6 +48,9 @@ enum TransactionMode { READ_WRITE = 'readwrite', } +enum DatabaseError { + INVALID_STATE_ERROR = 'InvalidStateError', // happens when changing the database schema (e.g., delete an object store) and then try to access the deleted store in an existing connection, +} /** * A wrapper around the extension's storage using IndexedDB API. */ @@ -64,6 +67,10 @@ export default class ExtensionStore { private isExtensionInitialized: boolean; + private dbReady: Promise; + + private inMemoryCache: Record | null = null; + /** * Creates an instance of the ExtensionStore. * @@ -77,7 +84,7 @@ export default class ExtensionStore { this.mostRecentRetrievedState = null; this.isExtensionInitialized = false; this.metadata = null; - this._init(); + this.dbReady = this._init(); } /** @@ -85,19 +92,27 @@ export default class ExtensionStore { * * @private */ - private _init() { - const request = indexedDB.open(this.storeName, this.dbVersion); + private _init(): Promise { + return new Promise((resolve, reject) => { + const request = indexedDB.open(this.storeName, this.dbVersion); - request.onupgradeneeded = (event) => { - const db = (event.target as IDBOpenDBRequest).result; - if (!db.objectStoreNames.contains(this.storeName)) { - db.createObjectStore(this.storeName, { keyPath: 'id' }); - } - }; + request.onupgradeneeded = (event) => { + const db = (event.target as IDBOpenDBRequest).result; + if (!db.objectStoreNames.contains(this.storeName)) { + db.createObjectStore(this.storeName, { keyPath: 'id' }); + } + }; - request.onerror = () => { - log.error('IndexedDB not supported or initialization failed.'); - }; + request.onerror = () => { + log.error('IndexedDB initialization failed.'); + reject(new Error('Failed to open IndexedDB.')); + }; + + request.onsuccess = (event) => { + const db = (event.target as IDBOpenDBRequest).result; + resolve(db); + }; + }); } /** @@ -107,23 +122,31 @@ export default class ExtensionStore { * @returns A promise that resolves to the object store. * @private */ - private _getObjectStore( + private async _getObjectStore( mode: IDBTransactionMode = TransactionMode.READ_ONLY, ): Promise { - return new Promise((resolve, reject) => { - const request = indexedDB.open(this.storeName, this.dbVersion); - - request.onerror = () => { - reject(new Error('Failed to open IndexedDB.')); - }; - - request.onsuccess = (event) => { - const db = (event.target as IDBOpenDBRequest).result; + try { + const db = await this.dbReady; // Wait for the DB to be ready + const transaction = db.transaction([this.storeName], mode); + return transaction.objectStore(this.storeName); + } catch (error) { + if ( + error instanceof Error && + error.name === DatabaseError.INVALID_STATE_ERROR + ) { + // Handle the case where the connection is closing + log.info( + 'Database connection was closed. Attempting to reinitialize IndexedDB.', + error, + ); + // Re-initialize the database connection + this.dbReady = this._init(); + const db = await this.dbReady; const transaction = db.transaction([this.storeName], mode); - const objectStore = transaction.objectStore(this.storeName); - resolve(objectStore); - }; - }); + return transaction.objectStore(this.storeName); + } + throw error; // Re-throw any other errors + } } /** @@ -156,10 +179,28 @@ export default class ExtensionStore { try { const dataToStore = { id: STATE_KEY, data: state, meta: this.metadata }; await this._writeToDB(dataToStore); + // Cache in memory for fallback + this.inMemoryCache = dataToStore; if (this.dataPersistenceFailing) { this.dataPersistenceFailing = false; } } catch (err) { + // When indexDB is deleted manually and we want to recover the previous recently saved state + if ( + err instanceof Error && + err.name === DatabaseError.INVALID_STATE_ERROR + ) { + log.info( + 'IndexedDB is not available. Falling back to in-memory cache.', + ); + this.inMemoryCache = { + id: STATE_KEY, + data: state, + meta: this.metadata, + }; + this.mostRecentRetrievedState = this.inMemoryCache; + } + if (!this.dataPersistenceFailing) { this.dataPersistenceFailing = true; captureException(err); @@ -177,15 +218,29 @@ export default class ExtensionStore { */ async get(): Promise | undefined> { try { + // Attempt to get state from IndexedDB const result = await this._readFromDB(STATE_KEY); - if (!result || this.isEmpty(result)) { - this.mostRecentRetrievedState = null; - return undefined; + + if (result && !this.isEmpty(result)) { + if (!this.isExtensionInitialized) { + this.mostRecentRetrievedState = result; + } + return result; } - if (!this.isExtensionInitialized) { - this.mostRecentRetrievedState = result; + + // If IndexedDB is empty, clear mostRecentRetrievedState + this.mostRecentRetrievedState = null; + + // Fallback to in-memory cache if IndexedDB is empty + if (this.inMemoryCache) { + log.info('Loaded state from in-memory cache fallback.'); + + // Set mostRecentRetrievedState to the in-memory cached state + this.mostRecentRetrievedState = this.inMemoryCache; + return this.inMemoryCache; } - return result; + // Return undefined if neither storage contains the state + return undefined; } catch (err) { log.error('Error getting state from IndexedDB:', err); return undefined;