From 249b7a5c4a51fd73b671f3fe34bd6cc5278af20a Mon Sep 17 00:00:00 2001 From: Balaji Viswanathan Date: Fri, 29 Sep 2023 16:38:38 -0600 Subject: [PATCH 1/3] Add log statements to IndexedDbProvider open --- src/IndexedDbProvider.ts | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/IndexedDbProvider.ts b/src/IndexedDbProvider.ts index 2022f23..5098116 100644 --- a/src/IndexedDbProvider.ts +++ b/src/IndexedDbProvider.ts @@ -155,20 +155,28 @@ export class IndexedDbProvider extends DbProvider { if (!this._dbFactory) { // Couldn't even find a supported indexeddb object on the browser... - return Promise.reject("No support for IndexedDB in this browser"); + const message = `No support for IndexedDB in this browser, returning`; + this.logger.error(message); + return Promise.reject(message); } if (wipeIfExists) { + this.logger.log(`Wiping db: ${dbName}`); try { let req = this._dbFactory.deleteDatabase(dbName); await IndexedDbProvider.WrapRequest(req); - } catch (e) { + } catch (e: any) { // Don't care + this.logger.error( + `Wiping db: ${dbName} failed, message: ${e?.message}. Ignoring and proceeding further` + ); } + this.logger.log(`Wiping db: ${dbName} success`); } this._lockHelper = new TransactionLockHelper(schema, true); + this.logger.log(`Opening db: ${dbName}, version: ${schema.version}`); const dbOpen = this._dbFactory.open(dbName, schema.version); let migrationPutters: Promise[] = []; @@ -179,9 +187,12 @@ export class IndexedDbProvider extends DbProvider { const trans = target.transaction; if (!trans) { + this.logger.error(`No transaction, unable to do upgrade`); throw new Error("onupgradeneeded: target is null!"); } + this.logger.log(`Upgrade needed for db: ${dbName}`); + // Avoid clearing object stores when event.oldVersion returns 0. // oldVersion returns 0 if db doesn't exist yet: https://developer.mozilla.org/en-US/docs/Web/API/IDBVersionChangeEvent/oldVersion if (event.oldVersion) { @@ -207,6 +218,7 @@ export class IndexedDbProvider extends DbProvider { } // Create all stores + this.logger.log(`Creating stores as part of upgrade process`); each(schema.stores, (storeSchema) => { let store: IDBObjectStore; const storeExistedBefore = includes( @@ -345,6 +357,9 @@ export class IndexedDbProvider extends DbProvider { const cursorReq = store.openCursor(); let thisIndexPutters: Promise[] = []; + this.logger.log( + `Adding store: ${storeSchema.name} to migrationPutters` + ); migrationPutters.push( IndexedDbIndex.iterateOverCursorRequest(cursorReq, (cursor) => { const err = attempt(() => { @@ -358,7 +373,14 @@ export class IndexedDbProvider extends DbProvider { if (err) { thisIndexPutters.push(Promise.reject(err)); } - }).then(() => Promise.all(thisIndexPutters).then(noop)) + }).then( + () => Promise.all(thisIndexPutters).then(noop), + (err) => { + this.logger.error( + `Error when iterating over cursor on idb index, message: ${err?.message}` + ); + } + ) ); } }); @@ -368,7 +390,11 @@ export class IndexedDbProvider extends DbProvider { return promise.then( (db) => { + this.logger.log( + `Waiting for migrationPutters: ${migrationPutters.length} to complete for db: ${dbName}` + ); return Promise.all(migrationPutters).then(() => { + this.logger.log(`Opening db: ${dbName} success`); this._db = db; this._db.onclose = (event: Event) => { if (this._handleOnClose) { @@ -417,6 +443,9 @@ export class IndexedDbProvider extends DbProvider { return this.open(dbName, schema, true, verbose); } } + this.logger.error( + `Error opening db: ${dbName}, message: ${err?.message}` + ); return Promise.reject(err); } ); From 8887bee156e1f4c6a4971d3502222a6eb6a4c3cd Mon Sep 17 00:00:00 2001 From: Automated Version Bump Date: Fri, 29 Sep 2023 22:50:57 +0000 Subject: [PATCH 2/3] ci: version bump to 0.6.43 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a57ddab..c81b230 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/objectstoreprovider", - "version": "0.6.42", + "version": "0.6.43", "description": "A cross-browser object store library", "author": "Mukundan Kavanur Kidambi ", "scripts": { From bacd97578ab6475e057ed6c071ddfa04c8b51e2f Mon Sep 17 00:00:00 2001 From: Balaji Viswanathan Date: Fri, 6 Oct 2023 14:24:40 -0600 Subject: [PATCH 3/3] Use LogWriter to write messages to logs in IndexedDbProvider --- karma.browser.conf.js | 1 + src/IndexedDbProvider.ts | 104 +++++++++++++++++++++++++----------- src/LogWriter.ts | 43 +++++++++++++++ src/tests/LogWriter.spec.ts | 71 ++++++++++++++++++++++++ webpack.config.js | 1 + 5 files changed, 188 insertions(+), 32 deletions(-) create mode 100644 src/LogWriter.ts create mode 100644 src/tests/LogWriter.spec.ts diff --git a/karma.browser.conf.js b/karma.browser.conf.js index d553dde..7bb0f56 100644 --- a/karma.browser.conf.js +++ b/karma.browser.conf.js @@ -6,6 +6,7 @@ module.exports = (config) => { files: [ { pattern: "dist/ObjectStoreProvider.spec.js" }, { pattern: "dist/SortedBTree.spec.js" }, + { pattern: "dist/LogWriter.spec.js" }, ], customLaunchers: { FirefoxHeadless: { diff --git a/src/IndexedDbProvider.ts b/src/IndexedDbProvider.ts index 5098116..ec75eec 100644 --- a/src/IndexedDbProvider.ts +++ b/src/IndexedDbProvider.ts @@ -57,6 +57,7 @@ import { TransactionToken, TransactionLockHelper, } from "./TransactionLockHelper"; +import { LogWriter } from "./LogWriter"; const IndexPrefix = "nsp_i_"; @@ -80,7 +81,7 @@ export class IndexedDbProvider extends DbProvider { private _handleOnClose: OnCloseHandler | undefined = undefined; private _lockHelper: TransactionLockHelper | undefined; - private logger: IObjectStoreProviderLogger; + private logWriter: LogWriter; // By default, it uses the in-browser indexed db factory, but you can pass in an explicit factory. Currently only used for unit tests. constructor( @@ -91,7 +92,7 @@ export class IndexedDbProvider extends DbProvider { ) { super(); - this.logger = logger ? logger : console; + this.logWriter = new LogWriter(logger ? logger : console); if (explicitDbFactory) { this._dbFactory = explicitDbFactory; @@ -156,27 +157,28 @@ export class IndexedDbProvider extends DbProvider { if (!this._dbFactory) { // Couldn't even find a supported indexeddb object on the browser... const message = `No support for IndexedDB in this browser, returning`; - this.logger.error(message); + this.logWriter.error(message); return Promise.reject(message); } if (wipeIfExists) { - this.logger.log(`Wiping db: ${dbName}`); + this.logWriter.log(`Wiping db`, { dbName }); try { let req = this._dbFactory.deleteDatabase(dbName); await IndexedDbProvider.WrapRequest(req); } catch (e: any) { // Don't care - this.logger.error( - `Wiping db: ${dbName} failed, message: ${e?.message}. Ignoring and proceeding further` + this.logWriter.error( + `Wiping db failed, message: ${e?.message}. Ignoring and proceeding further`, + { dbName } ); } - this.logger.log(`Wiping db: ${dbName} success`); + this.logWriter.log(`Wiping db success`, { dbName }); } this._lockHelper = new TransactionLockHelper(schema, true); - this.logger.log(`Opening db: ${dbName}, version: ${schema.version}`); + this.logWriter.log(`Opening db, version: ${schema.version}`, { dbName }); const dbOpen = this._dbFactory.open(dbName, schema.version); let migrationPutters: Promise[] = []; @@ -187,11 +189,11 @@ export class IndexedDbProvider extends DbProvider { const trans = target.transaction; if (!trans) { - this.logger.error(`No transaction, unable to do upgrade`); + this.logWriter.error(`No transaction, unable to do upgrade`); throw new Error("onupgradeneeded: target is null!"); } - this.logger.log(`Upgrade needed for db: ${dbName}`); + this.logWriter.log(`Upgrade needed for db`, { dbName }); // Avoid clearing object stores when event.oldVersion returns 0. // oldVersion returns 0 if db doesn't exist yet: https://developer.mozilla.org/en-US/docs/Web/API/IDBVersionChangeEvent/oldVersion @@ -201,8 +203,8 @@ export class IndexedDbProvider extends DbProvider { event.oldVersion < schema.lastUsableVersion ) { // Clear all stores if it's past the usable version - this.logger.log( - "Old version detected (" + event.oldVersion + "), clearing all data" + this.logWriter.log( + `Old version detected (${event.oldVersion}), clearing all data` ); each(db.objectStoreNames, (name) => { db.deleteObjectStore(name); @@ -215,10 +217,12 @@ export class IndexedDbProvider extends DbProvider { db.deleteObjectStore(storeName); } }); + + this.logWriter.log(`Deleted all object stores`, { dbName }); } // Create all stores - this.logger.log(`Creating stores as part of upgrade process`); + this.logWriter.log(`Creating stores as part of upgrade process`); each(schema.stores, (storeSchema) => { let store: IDBObjectStore; const storeExistedBefore = includes( @@ -234,6 +238,10 @@ export class IndexedDbProvider extends DbProvider { } // Any is to fix a lib.d.ts issue in TS 2.0.3 - it doesn't realize that keypaths can be compound for some reason... + this.logWriter.log(`Store doesn't exist, creating object store`, { + dbName, + storeName: storeSchema.name, + }); store = db.createObjectStore(storeSchema.name, { keyPath: primaryKeyPath, } as any); @@ -273,6 +281,11 @@ export class IndexedDbProvider extends DbProvider { } if (nuke) { + this.logWriter.log(`Index no longer exists, deleting`, { + dbName, + storeName: storeSchema.name, + indexName, + }); store.deleteIndex(indexName); } }); @@ -291,6 +304,10 @@ export class IndexedDbProvider extends DbProvider { throw new Error("Can't use multiEntry and compound keys"); } else { // Create an object store for the index + this.logWriter.log(`Creating object store and index`, { + dbName, + storeName: storeSchema.name, + }); let indexStore = db.createObjectStore( storeSchema.name + "_" + indexSchema.name, { autoIncrement: true } @@ -304,6 +321,11 @@ export class IndexedDbProvider extends DbProvider { } } else if (isCompoundKeyPath(keyPath)) { // Going to have to hack the compound index into a column, so here it is. + this.logWriter.log(`Creating index`, { + dbName, + storeName: storeSchema.name, + indexName: indexSchema.name, + }); store.createIndex( indexSchema.name, IndexPrefix + indexSchema.name, @@ -312,11 +334,21 @@ export class IndexedDbProvider extends DbProvider { } ); } else { + this.logWriter.log(`Creating index`, { + dbName, + storeName: storeSchema.name, + indexName: indexSchema.name, + }); store.createIndex(indexSchema.name, keyPath, { unique: indexSchema.unique, }); } } else if (indexSchema.fullText) { + this.logWriter.log(`Creating index`, { + dbName, + storeName: storeSchema.name, + indexName: indexSchema.name, + }); store.createIndex( indexSchema.name, IndexPrefix + indexSchema.name, @@ -330,6 +362,11 @@ export class IndexedDbProvider extends DbProvider { needsMigrate = true; } } else { + this.logWriter.log(`Creating index`, { + dbName, + storeName: storeSchema.name, + indexName: indexSchema.name, + }); store.createIndex(indexSchema.name, keyPath, { unique: indexSchema.unique, multiEntry: indexSchema.multiEntry, @@ -351,15 +388,15 @@ export class IndexedDbProvider extends DbProvider { fakeToken, schema, this._fakeComplicatedKeys, - this.logger + this.logWriter ); const tStore = iTrans.getStore(storeSchema.name); const cursorReq = store.openCursor(); let thisIndexPutters: Promise[] = []; - this.logger.log( - `Adding store: ${storeSchema.name} to migrationPutters` - ); + this.logWriter.log(`Adding store to migrationPutters`, { + storeName: storeSchema.name, + }); migrationPutters.push( IndexedDbIndex.iterateOverCursorRequest(cursorReq, (cursor) => { const err = attempt(() => { @@ -376,8 +413,9 @@ export class IndexedDbProvider extends DbProvider { }).then( () => Promise.all(thisIndexPutters).then(noop), (err) => { - this.logger.error( - `Error when iterating over cursor on idb index, message: ${err?.message}` + this.logWriter.error( + `Error when iterating over cursor on idb index, message: ${err?.message}`, + { storeName: storeSchema.name } ); } ) @@ -390,11 +428,12 @@ export class IndexedDbProvider extends DbProvider { return promise.then( (db) => { - this.logger.log( - `Waiting for migrationPutters: ${migrationPutters.length} to complete for db: ${dbName}` + this.logWriter.log( + `Waiting for migrationPutters: ${migrationPutters.length} to complete for db`, + { dbName } ); return Promise.all(migrationPutters).then(() => { - this.logger.log(`Opening db: ${dbName} success`); + this.logWriter.log(`Opening db success`, { dbName }); this._db = db; this._db.onclose = (event: Event) => { if (this._handleOnClose) { @@ -435,17 +474,18 @@ export class IndexedDbProvider extends DbProvider { err.target.error.name === "VersionError" ) { if (!wipeIfExists) { - this.logger.log( - "Database version too new, Wiping: " + - (err.target.error.message || err.target.error.name) + this.logWriter.log( + `Database version too new, Wiping: ${ + err.target.error.message || err.target.error.name + }` ); return this.open(dbName, schema, true, verbose); } } - this.logger.error( - `Error opening db: ${dbName}, message: ${err?.message}` - ); + this.logWriter.error(`Error opening db, message: ${err?.message}`, { + dbName, + }); return Promise.reject(err); } ); @@ -549,7 +589,7 @@ export class IndexedDbProvider extends DbProvider { transToken, this._schema!!!, this._fakeComplicatedKeys, - this.logger + this.logWriter ) ); } @@ -567,7 +607,7 @@ class IndexedDbTransaction implements DbTransaction { private _transToken: TransactionToken, private _schema: DbSchema, private _fakeComplicatedKeys: boolean, - private logger: IObjectStoreProviderLogger + private logWriter: LogWriter ) { this._stores = map(this._transToken.storeNames, (storeName) => this._trans.objectStore(storeName) @@ -595,7 +635,7 @@ class IndexedDbTransaction implements DbTransaction { ); if (history.length > 1) { - this.logger.warn( + this.logWriter.warn( "IndexedDbTransaction Errored after Resolution, Swallowing. Error: " + (this._trans.error ? this._trans.error.message : undefined) + ", History: " + @@ -619,7 +659,7 @@ class IndexedDbTransaction implements DbTransaction { ); if (history.length > 1) { - this.logger.warn( + this.logWriter.warn( "IndexedDbTransaction Aborted after Resolution, Swallowing. Error: " + (this._trans.error ? this._trans.error.message : undefined) + ", History: " + diff --git a/src/LogWriter.ts b/src/LogWriter.ts new file mode 100644 index 0000000..e5c42f4 --- /dev/null +++ b/src/LogWriter.ts @@ -0,0 +1,43 @@ +import { IObjectStoreProviderLogger } from "./ObjectStoreProvider"; + +export interface ILoggerContext { + dbName?: string; + storeName?: string; + indexName?: string; +} + +export class LogWriter { + constructor(public logger: IObjectStoreProviderLogger) {} + + public log(message: string, context?: ILoggerContext) { + const messageToWrite = this.computeMessageToWrite(message, context); + this.logger.log(messageToWrite); + } + + public error(message: string, context?: ILoggerContext) { + const messageToWrite = this.computeMessageToWrite(message, context); + this.logger.error(messageToWrite); + } + + public warn(message: string, context?: ILoggerContext) { + const messageToWrite = this.computeMessageToWrite(message, context); + this.logger.warn(messageToWrite); + } + + private computeMessageToWrite(message: string, context?: ILoggerContext) { + let contextMessages: string[] = []; + if (context) { + for (const key in context) { + const value = context[key as keyof ILoggerContext]; + if (!value) { + continue; + } + contextMessages.push(`${key}: ${value}`); + } + } + if (!contextMessages.length) { + return message; + } + return `${message}. ${contextMessages.join(", ")}`; + } +} diff --git a/src/tests/LogWriter.spec.ts b/src/tests/LogWriter.spec.ts new file mode 100644 index 0000000..e5be243 --- /dev/null +++ b/src/tests/LogWriter.spec.ts @@ -0,0 +1,71 @@ +import { assert } from "chai"; +import { LogWriter } from "../LogWriter"; + +describe("LogWriter", () => { + it("basic", () => { + let output: any = ""; + const logger = { + log: (message: string) => { + output = message; + }, + warn: (message: string) => { + output = message; + }, + error: (message: string) => { + output = message; + }, + }; + const logWriter = new LogWriter(logger); + + // log + logWriter.log(`Message 1`); + assert(output === `Message 1`); + logWriter.log(`Db open`, { dbName: "settings" }); + assert(output === `Db open. dbName: settings`); + logWriter.log(`Create store`, { dbName: "settings", storeName: "flags" }); + assert(output === `Create store. dbName: settings, storeName: flags`); + logWriter.log(`Create store`, { + dbName: "settings", + storeName: "flags", + indexName: "test", + }); + assert( + output === + `Create store. dbName: settings, storeName: flags, indexName: test` + ); + + // warn + logWriter.warn(`Message 1`); + assert(output === `Message 1`); + logWriter.warn(`Db open`, { dbName: "settings" }); + assert(output === `Db open. dbName: settings`); + logWriter.warn(`Create store`, { dbName: "settings", storeName: "flags" }); + assert(output === `Create store. dbName: settings, storeName: flags`); + logWriter.warn(`Create store`, { + dbName: "settings", + storeName: "flags", + indexName: "test", + }); + assert( + output === + `Create store. dbName: settings, storeName: flags, indexName: test` + ); + + // error + logWriter.error(`Message 1`); + assert(output === `Message 1`); + logWriter.error(`Db open`, { dbName: "settings" }); + assert(output === `Db open. dbName: settings`); + logWriter.error(`Create store`, { dbName: "settings", storeName: "flags" }); + assert(output === `Create store. dbName: settings, storeName: flags`); + logWriter.error(`Create store`, { + dbName: "settings", + storeName: "flags", + indexName: "test", + }); + assert( + output === + `Create store. dbName: settings, storeName: flags, indexName: test` + ); + }); +}); diff --git a/webpack.config.js b/webpack.config.js index db052d9..b4615dd 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -6,6 +6,7 @@ var webpackConfig = { entry: { ObjectStoreProvider: "./src/tests/ObjectStoreProvider.spec.ts", SortedBTree: "./src/tests/SortedBTree.spec.ts", + LogWriter: "./src/tests/LogWriter.spec.ts", }, output: {