From 59087d703e99046bdf7d429c17b07e15f74a8087 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 16 Mar 2023 12:06:38 +0000 Subject: [PATCH 1/6] Fire `closed` event when IndexedDB closes unexpectedly --- src/store/index.ts | 2 +- src/store/indexeddb-backend.ts | 2 +- src/store/indexeddb-local-backend.ts | 12 ++++++++++-- src/store/indexeddb-remote-backend.ts | 9 +++++++-- src/store/indexeddb-store-worker.ts | 10 ++++++++-- src/store/indexeddb.ts | 12 +++++++++++- 6 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/store/index.ts b/src/store/index.ts index 84f613cf4dd..fec5c0af4ac 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -40,7 +40,7 @@ export interface IStore { // XXX: The indexeddb store exposes a non-standard emitter for the "degraded" event // for when it falls back to being a memory store due to errors. - on?: (event: EventEmitterEvents | "degraded", handler: (...args: any[]) => void) => void; + on?: (event: EventEmitterEvents | "degraded" | "closed", handler: (...args: any[]) => void) => void; /** @returns whether or not the database was newly created in this session. */ isNewlyCreated(): Promise; diff --git a/src/store/indexeddb-backend.ts b/src/store/indexeddb-backend.ts index 5da164c555b..008867dfc33 100644 --- a/src/store/indexeddb-backend.ts +++ b/src/store/indexeddb-backend.ts @@ -19,7 +19,7 @@ import { IEvent, IStateEventWithRoomId, IStoredClientOpts, ISyncResponse } from import { IndexedToDeviceBatch, ToDeviceBatchWithTxnId } from "../models/ToDeviceMessage"; export interface IIndexedDBBackend { - connect(): Promise; + connect(onClose?: () => void): Promise; syncToDatabase(userTuples: UserTuple[]): Promise; isNewlyCreated(): Promise; setSyncData(syncData: ISyncResponse): Promise; diff --git a/src/store/indexeddb-local-backend.ts b/src/store/indexeddb-local-backend.ts index 7e84ad621a5..c1495680328 100644 --- a/src/store/indexeddb-local-backend.ts +++ b/src/store/indexeddb-local-backend.ts @@ -153,7 +153,7 @@ export class LocalIndexedDBStoreBackend implements IIndexedDBBackend { * grant permission. * @returns Promise which resolves if successfully connected. */ - public connect(): Promise { + public connect(onClose?: () => void): Promise { if (!this.disconnected) { logger.log(`LocalIndexedDBStoreBackend.connect: already connected or connecting`); return Promise.resolve(); @@ -188,7 +188,15 @@ export class LocalIndexedDBStoreBackend implements IIndexedDBBackend { // add a poorly-named listener for when deleteDatabase is called // so we can close our db connections. this.db.onversionchange = (): void => { - this.db?.close(); + this.db?.close(); // this does not call onclose + this.disconnected = true; + this.db = undefined; + onClose?.(); + }; + this.db.onclose = (): void => { + this.disconnected = true; + this.db = undefined; + onClose?.(); }; await this.init(); diff --git a/src/store/indexeddb-remote-backend.ts b/src/store/indexeddb-remote-backend.ts index 7406d3a6198..7e2aa0ccbe9 100644 --- a/src/store/indexeddb-remote-backend.ts +++ b/src/store/indexeddb-remote-backend.ts @@ -30,6 +30,8 @@ export class RemoteIndexedDBStoreBackend implements IIndexedDBBackend { // Once we start connecting, we keep the promise and re-use it // if we try to connect again private startPromise?: Promise; + // Callback for when the IndexedDB gets closed unexpectedly + private onClose?(): void; /** * An IndexedDB store backend where the actual backend sits in a web @@ -48,7 +50,8 @@ export class RemoteIndexedDBStoreBackend implements IIndexedDBBackend { * grant permission. * @returns Promise which resolves if successfully connected. */ - public connect(): Promise { + public connect(onClose?: () => void): Promise { + this.onClose = onClose; return this.ensureStarted().then(() => this.doCmd("connect")); } @@ -171,7 +174,9 @@ export class RemoteIndexedDBStoreBackend implements IIndexedDBBackend { private onWorkerMessage = (ev: MessageEvent): void => { const msg = ev.data; - if (msg.command == "cmd_success" || msg.command == "cmd_fail") { + if (msg.command == "closed") { + this.onClose?.(); + } else if (msg.command == "cmd_success" || msg.command == "cmd_fail") { if (msg.seq === undefined) { logger.error("Got reply from worker with no seq"); return; diff --git a/src/store/indexeddb-store-worker.ts b/src/store/indexeddb-store-worker.ts index df508ffa7ba..52a7fa6bf32 100644 --- a/src/store/indexeddb-store-worker.ts +++ b/src/store/indexeddb-store-worker.ts @@ -49,6 +49,12 @@ export class IndexedDBStoreWorker { */ public constructor(private readonly postMessage: InstanceType["postMessage"]) {} + private onClose = (): void => { + this.postMessage.call(null, { + command: "closed", + }); + }; + /** * Passes a message event from the main script into the class. This method * can be directly assigned to the web worker `onmessage` variable. @@ -57,7 +63,7 @@ export class IndexedDBStoreWorker { */ public onMessage = (ev: MessageEvent): void => { const msg: ICmd = ev.data; - let prom; + let prom: Promise | undefined; switch (msg.command) { case "setupWorker": @@ -67,7 +73,7 @@ export class IndexedDBStoreWorker { prom = Promise.resolve(); break; case "connect": - prom = this.backend?.connect(); + prom = this.backend?.connect(this.onClose); break; case "isNewlyCreated": prom = this.backend?.isNewlyCreated(); diff --git a/src/store/indexeddb.ts b/src/store/indexeddb.ts index 1f0961e2bc0..cc77bf9c80f 100644 --- a/src/store/indexeddb.ts +++ b/src/store/indexeddb.ts @@ -51,7 +51,12 @@ interface IOpts extends IBaseOpts { } type EventHandlerMap = { + // Fired when an IDB command fails on a degradable path, and the store falls back to MemoryStore + // This signals the potential for data volatility. degraded: (e: Error) => void; + // Fired when the IndexedDB gets closed unexpectedly, for example, if the underlying storage is removed or + // if the user clears the database in the browser's history preferences. + closed: () => void; }; export class IndexedDBStore extends MemoryStore { @@ -127,7 +132,7 @@ export class IndexedDBStore extends MemoryStore { logger.log(`IndexedDBStore.startup: connecting to backend`); return this.backend - .connect() + .connect(this.onClose) .then(() => { logger.log(`IndexedDBStore.startup: loading presence events`); return this.backend.getUserPresenceEvents(); @@ -142,9 +147,14 @@ export class IndexedDBStore extends MemoryStore { this.userModifiedMap[u.userId] = u.getLastModifiedTime(); this.storeUser(u); }); + this.startedUp = true; }); } + private onClose = (): void => { + this.emitter.emit("closed"); + }; + /** * @returns Promise which resolves with a sync response to restore the * client state to where it was at the last save, or null if there From ebc0f381bfa70da8390e57afd3779e8ad3fe12e2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 16 Mar 2023 16:20:39 +0000 Subject: [PATCH 2/6] Add test --- spec/unit/stores/indexeddb.spec.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/unit/stores/indexeddb.spec.ts b/spec/unit/stores/indexeddb.spec.ts index f2ce29e47e6..5121e47e78b 100644 --- a/spec/unit/stores/indexeddb.spec.ts +++ b/spec/unit/stores/indexeddb.spec.ts @@ -166,4 +166,20 @@ describe("IndexedDBStore", () => { await expect(store.isNewlyCreated()).resolves.toBeFalsy(); }); + + it("should emit 'closed' if database is unexpectedly closed", async () => { + const store = new IndexedDBStore({ + indexedDB: indexedDB, + dbName: "database", + localStorage, + }); + await store.startup(); + + const deferred = defer(); + store.on("closed", deferred.resolve); + + // @ts-ignore - private field access + (store.backend as LocalIndexedDBStoreBackend).db!.onclose!({} as Event); + await deferred.promise; + }); }); From 4046a211e67c7b012cade3efb1eb2a7994c854e7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 17 Mar 2023 15:10:57 +0000 Subject: [PATCH 3/6] Add tests --- .../stores/indexeddb-store-worker.spec.ts | 47 +++++++++++++++++++ src/store/index.ts | 5 +- 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 spec/unit/stores/indexeddb-store-worker.spec.ts diff --git a/spec/unit/stores/indexeddb-store-worker.spec.ts b/spec/unit/stores/indexeddb-store-worker.spec.ts new file mode 100644 index 00000000000..536327d2e38 --- /dev/null +++ b/spec/unit/stores/indexeddb-store-worker.spec.ts @@ -0,0 +1,47 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import "fake-indexeddb/auto"; + +import { LocalIndexedDBStoreBackend } from "../../../src/store/indexeddb-local-backend"; +import { IndexedDBStoreWorker } from "../../../src/store/indexeddb-store-worker"; +import { defer } from "../../../src/utils"; + +function setupWorker(worker: IndexedDBStoreWorker): void { + worker.onMessage({ data: { command: "setupWorker", args: [] } } as any); + worker.onMessage({ data: { command: "connect", seq: 1 } } as any); +} + +describe("IndexedDBStore Worker", () => { + it("should pass 'closed' event via postMessage", async () => { + const deferred = defer(); + const postMessage = jest.fn().mockImplementation(({ seq, command }) => { + if (seq === 1 && command === "cmd_success") { + deferred.resolve(); + } + }); + const worker = new IndexedDBStoreWorker(postMessage); + setupWorker(worker); + + await deferred.promise; + + // @ts-ignore - private field access + (worker.backend as LocalIndexedDBStoreBackend).db!.onclose!({} as Event); + expect(postMessage).toHaveBeenCalledWith({ + command: "closed", + }); + }); +}); diff --git a/src/store/index.ts b/src/store/index.ts index fec5c0af4ac..b172d386f36 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -38,8 +38,9 @@ export interface ISavedSync { export interface IStore { readonly accountData: Record; // type : content - // XXX: The indexeddb store exposes a non-standard emitter for the "degraded" event - // for when it falls back to being a memory store due to errors. + // XXX: The indexeddb store exposes a non-standard emitter for: + // "degraded" event for when it falls back to being a memory store due to errors. + // "closed" event for when the database closes unexpectedly on?: (event: EventEmitterEvents | "degraded" | "closed", handler: (...args: any[]) => void) => void; /** @returns whether or not the database was newly created in this session. */ From 07fa04bce20e170e7519cbb62dbbed85c432d6c3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 27 Mar 2023 09:19:19 +0100 Subject: [PATCH 4/6] Add test --- spec/unit/stores/indexeddb.spec.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/unit/stores/indexeddb.spec.ts b/spec/unit/stores/indexeddb.spec.ts index 5121e47e78b..c6baa6c7851 100644 --- a/spec/unit/stores/indexeddb.spec.ts +++ b/spec/unit/stores/indexeddb.spec.ts @@ -182,4 +182,24 @@ describe("IndexedDBStore", () => { (store.backend as LocalIndexedDBStoreBackend).db!.onclose!({} as Event); await deferred.promise; }); + + it("should use remote backend if workerFactory passed", async () => { + const deferred = defer(); + class MockWorker { + postMessage(data: any) { + if (data.command === "setupWorker") { + deferred.resolve(); + } + } + } + + const store = new IndexedDBStore({ + indexedDB: indexedDB, + dbName: "database", + localStorage, + workerFactory: () => new MockWorker() as Worker, + }); + store.startup(); + await deferred.promise; + }); }); From 693aa4e8eb72544e976dacd66553d1e55c89fa95 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 27 Mar 2023 09:31:51 +0100 Subject: [PATCH 5/6] Add test --- spec/unit/stores/indexeddb.spec.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/unit/stores/indexeddb.spec.ts b/spec/unit/stores/indexeddb.spec.ts index c6baa6c7851..cb83b5be82c 100644 --- a/spec/unit/stores/indexeddb.spec.ts +++ b/spec/unit/stores/indexeddb.spec.ts @@ -202,4 +202,23 @@ describe("IndexedDBStore", () => { store.startup(); await deferred.promise; }); + + it("remote worker should pass closed event", async () => { + const worker = new (class MockWorker { + postMessage(data: any) {} + })() as Worker; + + const store = new IndexedDBStore({ + indexedDB: indexedDB, + dbName: "database", + localStorage, + workerFactory: () => worker, + }); + store.startup(); + + const deferred = defer(); + store.on("closed", deferred.resolve); + (worker as any).onmessage({ data: { command: "closed" } }); + await deferred.promise; + }); }); From 621d0e6b6227e3b97d09d560b78b9772f81e0cb1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 31 Mar 2023 09:24:57 +0100 Subject: [PATCH 6/6] Add test coverage --- spec/unit/stores/indexeddb.spec.ts | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/unit/stores/indexeddb.spec.ts b/spec/unit/stores/indexeddb.spec.ts index cb83b5be82c..544a3d75c5b 100644 --- a/spec/unit/stores/indexeddb.spec.ts +++ b/spec/unit/stores/indexeddb.spec.ts @@ -221,4 +221,37 @@ describe("IndexedDBStore", () => { (worker as any).onmessage({ data: { command: "closed" } }); await deferred.promise; }); + + it("remote worker should pass command failures", async () => { + const worker = new (class MockWorker { + private onmessage!: (data: any) => void; + postMessage(data: any) { + if (data.command === "setupWorker" || data.command === "connect") { + this.onmessage({ + data: { + command: "cmd_success", + seq: data.seq, + }, + }); + return; + } + + this.onmessage({ + data: { + command: "cmd_fail", + seq: data.seq, + error: new Error("Test"), + }, + }); + } + })() as unknown as Worker; + + const store = new IndexedDBStore({ + indexedDB: indexedDB, + dbName: "database", + localStorage, + workerFactory: () => worker, + }); + await expect(store.startup()).rejects.toThrow("Test"); + }); });