Skip to content

Commit f351319

Browse files
Krishna2323Krishna2323
committed
address review comments.
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
1 parent e824ef7 commit f351319

2 files changed

Lines changed: 33 additions & 21 deletions

File tree

‎lib/storage/providers/IDBKeyValProvider/createStore.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,24 @@
11
import * as IDB from 'idb-keyval';
22
import type {UseStore} from 'idb-keyval';
3-
import {logAlert, logInfo} from '../../../Logger';
3+
import * as Logger from '../../../Logger';
4+
5+
type ConnectionClosedReason = 'browser' | 'versionchange' | 'verifyStoreExists' | 'unknown';
46

57
// This is a copy of the createStore function from idb-keyval, we need a custom implementation
68
// because we need to create the database manually in order to ensure that the store exists before we use it.
79
// If the store does not exist, idb-keyval will throw an error
810
// source: https://github.com/jakearchibald/idb-keyval/blob/9d19315b4a83897df1e0193dccdc29f78466a0f3/src/index.ts#L12
911
function createStore(dbName: string, storeName: string): UseStore {
1012
let dbp: Promise<IDBDatabase> | undefined;
11-
let closedBy: 'browser' | 'versionchange' | 'verifyStoreExists' | 'unknown' = 'unknown';
13+
let closedBy: ConnectionClosedReason = 'unknown';
1214

1315
const attachHandlers = (db: IDBDatabase) => {
14-
// It seems like Safari sometimes likes to just close the connection.
15-
// It's supposed to fire this event when that happens. Let's hope it does!
16+
// Browsers may close idle IDB connections at any time, especially Safari.
17+
// We clear the cached promise so the next operation opens a fresh connection.
18+
// https://developer.mozilla.org/en-US/docs/Web/API/IDBDatabase/close_event
1619
// eslint-disable-next-line no-param-reassign
1720
db.onclose = () => {
18-
logInfo('IDB connection closed by browser', {dbName, storeName});
21+
Logger.logInfo('IDB connection closed by browser', {dbName, storeName});
1922
closedBy = 'browser';
2023
dbp = undefined;
2124
};
@@ -25,7 +28,7 @@ function createStore(dbName: string, storeName: string): UseStore {
2528
// https://developer.mozilla.org/en-US/docs/Web/API/IDBDatabase/versionchange_event
2629
// eslint-disable-next-line no-param-reassign
2730
db.onversionchange = () => {
28-
logInfo('IDB connection closing due to versionchange', {dbName, storeName});
31+
Logger.logInfo('IDB connection closing due to version change', {dbName, storeName});
2932
closedBy = 'versionchange';
3033
db.close();
3134
dbp = undefined;
@@ -53,7 +56,7 @@ function createStore(dbName: string, storeName: string): UseStore {
5356
return db;
5457
}
5558

56-
logInfo(`Store ${storeName} does not exist in database ${dbName}.`);
59+
Logger.logInfo(`Store ${storeName} does not exist in database ${dbName}.`);
5760
const nextVersion = db.version + 1;
5861
closedBy = 'verifyStoreExists';
5962
db.close();
@@ -65,7 +68,7 @@ function createStore(dbName: string, storeName: string): UseStore {
6568
return;
6669
}
6770

68-
logInfo(`Creating store ${storeName} in database ${dbName}.`);
71+
Logger.logInfo(`Creating store ${storeName} in database ${dbName}.`);
6972
updatedDatabase.createObjectStore(storeName);
7073
};
7174

@@ -81,10 +84,12 @@ function createStore(dbName: string, storeName: string): UseStore {
8184
.then((db) => callback(db.transaction(storeName, txMode).objectStore(storeName)));
8285
}
8386

87+
// If the connection was closed between getDB() resolving and db.transaction() executing,
88+
// the transaction throws InvalidStateError. We catch it and retry once with a fresh connection.
8489
return (txMode, callback) =>
8590
executeTransaction(txMode, callback).catch((error) => {
8691
if (error instanceof DOMException && error.name === 'InvalidStateError') {
87-
logAlert('IDB InvalidStateError, retrying with fresh connection', {
92+
Logger.logAlert('IDB InvalidStateError, retrying with fresh connection', {
8893
dbName,
8994
storeName,
9095
txMode,

‎tests/unit/storage/providers/createStoreTest.ts‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ function uniqueDBName() {
1414
* Captures the internal IDBDatabase instance used by a store by intercepting
1515
* the first db.transaction() call.
1616
*/
17-
async function captureDB(store: ReturnType<typeof createStore>): Promise<IDBDatabase> {
17+
async function captureDB(store: ReturnType<typeof createStore>): Promise<IDBDatabase | undefined> {
1818
const captured: {db?: IDBDatabase} = {};
1919
const original = IDBDatabase.prototype.transaction;
2020
const spy = jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(function (this: IDBDatabase, ...args) {
@@ -23,10 +23,10 @@ async function captureDB(store: ReturnType<typeof createStore>): Promise<IDBData
2323
return original.apply(this, args);
2424
});
2525
await store('readonly', (s) => IDB.promisifyRequest(s.getAllKeys()));
26-
return captured.db!;
26+
return captured.db;
2727
}
2828

29-
describe('createStore - connection resilience', () => {
29+
describe('createStore', () => {
3030
let logAlertSpy: jest.SpyInstance;
3131
let logInfoSpy: jest.SpyInstance;
3232

@@ -188,7 +188,8 @@ describe('createStore - connection resilience', () => {
188188
const store = createStore(dbName, STORE_NAME);
189189

190190
const db = await captureDB(store);
191-
db.onclose!.call(db, new Event('close'));
191+
expect(db).toBeDefined();
192+
db?.onclose?.call(db, new Event('close'));
192193

193194
const original = IDBDatabase.prototype.transaction;
194195
let callCount = 0;
@@ -210,8 +211,9 @@ describe('createStore - connection resilience', () => {
210211
const store = createStore(dbName, STORE_NAME);
211212

212213
const db = await captureDB(store);
214+
expect(db).toBeDefined();
213215
// @ts-expect-error -- our handler ignores the event argument
214-
db.onversionchange!.call(db, new Event('versionchange'));
216+
db?.onversionchange?.call(db, new Event('versionchange'));
215217

216218
const original = IDBDatabase.prototype.transaction;
217219
let callCount = 0;
@@ -233,7 +235,8 @@ describe('createStore - connection resilience', () => {
233235
const store = createStore(dbName, STORE_NAME);
234236

235237
const db = await captureDB(store);
236-
db.onclose!.call(db, new Event('close'));
238+
expect(db).toBeDefined();
239+
db?.onclose?.call(db, new Event('close'));
237240

238241
const original = IDBDatabase.prototype.transaction;
239242
let callCount = 0;
@@ -265,7 +268,8 @@ describe('createStore - connection resilience', () => {
265268
const store = createStore(dbName, STORE_NAME);
266269

267270
const db = await captureDB(store);
268-
db.onclose!.call(db, new Event('close'));
271+
expect(db).toBeDefined();
272+
db?.onclose?.call(db, new Event('close'));
269273

270274
expect(logInfoSpy).toHaveBeenCalledWith('IDB connection closed by browser', {dbName, storeName: STORE_NAME});
271275
});
@@ -279,7 +283,8 @@ describe('createStore - connection resilience', () => {
279283
});
280284

281285
const db = await captureDB(store);
282-
db.onclose!.call(db, new Event('close'));
286+
expect(db).toBeDefined();
287+
db?.onclose?.call(db, new Event('close'));
283288

284289
const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1')));
285290
expect(result).toBe('value');
@@ -292,13 +297,14 @@ describe('createStore - connection resilience', () => {
292297
const store = createStore(dbName, STORE_NAME);
293298

294299
const db = await captureDB(store);
295-
const closeSpy = jest.spyOn(db, 'close');
300+
expect(db).toBeDefined();
301+
const closeSpy = jest.spyOn(db!, 'close');
296302

297303
// @ts-expect-error -- our handler ignores the event argument
298-
db.onversionchange!.call(db, new Event('versionchange'));
304+
db?.onversionchange?.call(db, new Event('versionchange'));
299305

300306
expect(closeSpy).toHaveBeenCalled();
301-
expect(logInfoSpy).toHaveBeenCalledWith('IDB connection closing due to versionchange', {dbName, storeName: STORE_NAME});
307+
expect(logInfoSpy).toHaveBeenCalledWith('IDB connection closing due to version change', {dbName, storeName: STORE_NAME});
302308
});
303309

304310
it('should recover with a fresh connection after versionchange', async () => {
@@ -310,8 +316,9 @@ describe('createStore - connection resilience', () => {
310316
});
311317

312318
const db = await captureDB(store);
319+
expect(db).toBeDefined();
313320
// @ts-expect-error -- our handler ignores the event argument
314-
db.onversionchange!.call(db, new Event('versionchange'));
321+
db?.onversionchange?.call(db, new Event('versionchange'));
315322

316323
const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1')));
317324
expect(result).toBe('value');

0 commit comments

Comments
 (0)