From aeedb948cc3c219ed24eb5429b7888edee0b4e68 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 17 Sep 2021 18:19:26 +0200 Subject: [PATCH 1/5] pass logger to Storage and make it available in Transaction --- src/domain/SessionPickerViewModel.js | 8 ++++++-- src/logging/LogItem.js | 4 ++++ src/matrix/storage/idb/Storage.ts | 15 +++++++++------ src/matrix/storage/idb/StorageFactory.ts | 17 +++++++++-------- src/matrix/storage/idb/Transaction.ts | 16 +++++++++++++--- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/domain/SessionPickerViewModel.js b/src/domain/SessionPickerViewModel.js index 3bbbcef7..fc669df1 100644 --- a/src/domain/SessionPickerViewModel.js +++ b/src/domain/SessionPickerViewModel.js @@ -150,7 +150,9 @@ export class SessionPickerViewModel extends ViewModel { async _exportData(id) { const sessionInfo = await this.platform.sessionInfoStorage.get(id); - const stores = await this.platform.storageFactory.export(id); + const stores = await this.logger.run("export", log => { + return this.platform.storageFactory.export(id, log); + }); const data = {sessionInfo, stores}; return data; } @@ -161,7 +163,9 @@ export class SessionPickerViewModel extends ViewModel { const {sessionInfo} = data; sessionInfo.comment = `Imported on ${new Date().toLocaleString()} from id ${sessionInfo.id}.`; sessionInfo.id = this._createSessionContainer().createNewSessionId(); - await this.platform.storageFactory.import(sessionInfo.id, data.stores); + await this.logger.run("import", log => { + return this.platform.storageFactory.import(sessionInfo.id, data.stores, log); + }); await this.platform.sessionInfoStorage.add(sessionInfo); this._sessions.set(new SessionItemViewModel(sessionInfo, this)); } catch (err) { diff --git a/src/logging/LogItem.js b/src/logging/LogItem.js index fcb88eff..69128270 100644 --- a/src/logging/LogItem.js +++ b/src/logging/LogItem.js @@ -231,4 +231,8 @@ export class LogItem { this._children.push(item); return item; } + + get logger() { + return this._logger; + } } diff --git a/src/matrix/storage/idb/Storage.ts b/src/matrix/storage/idb/Storage.ts index bffdb642..6f7f4c90 100644 --- a/src/matrix/storage/idb/Storage.ts +++ b/src/matrix/storage/idb/Storage.ts @@ -17,20 +17,23 @@ limitations under the License. import {Transaction} from "./Transaction"; import { STORE_NAMES, StoreNames, StorageError } from "../common"; import { reqAsPromise } from "./utils"; +import { BaseLogger } from "../../../logging/BaseLogger.js"; const WEBKITEARLYCLOSETXNBUG_BOGUS_KEY = "782rh281re38-boguskey"; export class Storage { private _db: IDBDatabase; private _hasWebkitEarlyCloseTxnBug: boolean; - private _IDBKeyRange: typeof IDBKeyRange - storeNames: typeof StoreNames; + readonly logger: BaseLogger; + readonly IDBKeyRange: typeof IDBKeyRange; + readonly storeNames: typeof StoreNames; - constructor(idbDatabase: IDBDatabase, _IDBKeyRange: typeof IDBKeyRange, hasWebkitEarlyCloseTxnBug: boolean) { + constructor(idbDatabase: IDBDatabase, _IDBKeyRange: typeof IDBKeyRange, hasWebkitEarlyCloseTxnBug: boolean, logger: BaseLogger) { this._db = idbDatabase; - this._IDBKeyRange = _IDBKeyRange; + this.IDBKeyRange = _IDBKeyRange; this._hasWebkitEarlyCloseTxnBug = hasWebkitEarlyCloseTxnBug; this.storeNames = StoreNames; + this.logger = logger; } _validateStoreNames(storeNames: StoreNames[]): void { @@ -49,7 +52,7 @@ export class Storage { if (this._hasWebkitEarlyCloseTxnBug) { await reqAsPromise(txn.objectStore(storeNames[0]).get(WEBKITEARLYCLOSETXNBUG_BOGUS_KEY)); } - return new Transaction(txn, storeNames, this._IDBKeyRange); + return new Transaction(txn, storeNames, this); } catch(err) { throw new StorageError("readTxn failed", err); } @@ -64,7 +67,7 @@ export class Storage { if (this._hasWebkitEarlyCloseTxnBug) { await reqAsPromise(txn.objectStore(storeNames[0]).get(WEBKITEARLYCLOSETXNBUG_BOGUS_KEY)); } - return new Transaction(txn, storeNames, this._IDBKeyRange); + return new Transaction(txn, storeNames, this); } catch(err) { throw new StorageError("readWriteTxn failed", err); } diff --git a/src/matrix/storage/idb/StorageFactory.ts b/src/matrix/storage/idb/StorageFactory.ts index 6db78c36..38b48a17 100644 --- a/src/matrix/storage/idb/StorageFactory.ts +++ b/src/matrix/storage/idb/StorageFactory.ts @@ -20,9 +20,10 @@ import { exportSession, importSession, Export } from "./export"; import { schema } from "./schema"; import { detectWebkitEarlyCloseTxnBug } from "./quirks"; import { BaseLogger } from "../../../logging/BaseLogger.js"; +import { LogItem } from "../../../logging/LogItem.js"; const sessionName = (sessionId: string) => `hydrogen_session_${sessionId}`; -const openDatabaseWithSessionId = function(sessionId: string, idbFactory: IDBFactory, log?: BaseLogger) { +const openDatabaseWithSessionId = function(sessionId: string, idbFactory: IDBFactory, log: LogItem) { const create = (db, txn, oldVersion, version) => createStores(db, txn, oldVersion, version, log); return openDatabase(sessionName(sessionId), create, schema.length, idbFactory); } @@ -59,7 +60,7 @@ export class StorageFactory { this._IDBKeyRange = _IDBKeyRange; } - async create(sessionId: string, log?: BaseLogger): Promise { + async create(sessionId: string, log: LogItem): Promise { await this._serviceWorkerHandler?.preventConcurrentSessionAccess(sessionId); requestPersistedStorage().then(persisted => { // Firefox lies here though, and returns true even if the user denied the request @@ -70,7 +71,7 @@ export class StorageFactory { const hasWebkitEarlyCloseTxnBug = await detectWebkitEarlyCloseTxnBug(this._idbFactory); const db = await openDatabaseWithSessionId(sessionId, this._idbFactory, log); - return new Storage(db, this._IDBKeyRange, hasWebkitEarlyCloseTxnBug); + return new Storage(db, this._IDBKeyRange, hasWebkitEarlyCloseTxnBug, log.logger); } delete(sessionId: string): Promise { @@ -79,18 +80,18 @@ export class StorageFactory { return reqAsPromise(req); } - async export(sessionId: string): Promise { - const db = await openDatabaseWithSessionId(sessionId, this._idbFactory); + async export(sessionId: string, log: LogItem): Promise { + const db = await openDatabaseWithSessionId(sessionId, this._idbFactory, log); return await exportSession(db); } - async import(sessionId: string, data: Export): Promise { - const db = await openDatabaseWithSessionId(sessionId, this._idbFactory); + async import(sessionId: string, data: Export, log: LogItem): Promise { + const db = await openDatabaseWithSessionId(sessionId, this._idbFactory, log); return await importSession(db, data); } } -async function createStores(db: IDBDatabase, txn: IDBTransaction, oldVersion: number | null, version: number, log?: BaseLogger): Promise { +async function createStores(db: IDBDatabase, txn: IDBTransaction, oldVersion: number | null, version: number, log: LogItem): Promise { const startIdx = oldVersion || 0; return log.wrap({l: "storage migration", oldVersion, version}, async log => { for(let i = startIdx; i < version; ++i) { diff --git a/src/matrix/storage/idb/Transaction.ts b/src/matrix/storage/idb/Transaction.ts index ea21b745..e9b127ff 100644 --- a/src/matrix/storage/idb/Transaction.ts +++ b/src/matrix/storage/idb/Transaction.ts @@ -35,18 +35,28 @@ import {OutboundGroupSessionStore} from "./stores/OutboundGroupSessionStore"; import {GroupSessionDecryptionStore} from "./stores/GroupSessionDecryptionStore"; import {OperationStore} from "./stores/OperationStore"; import {AccountDataStore} from "./stores/AccountDataStore"; +import {BaseLogger} from "../../../logging/BaseLogger.js"; export class Transaction { private _txn: IDBTransaction; private _allowedStoreNames: StoreNames[]; private _stores: { [storeName in StoreNames]?: any }; + private _storage: Storage; - constructor(txn: IDBTransaction, allowedStoreNames: StoreNames[], IDBKeyRange) { + constructor(txn: IDBTransaction, allowedStoreNames: StoreNames[], storage: Storage) { this._txn = txn; this._allowedStoreNames = allowedStoreNames; this._stores = {}; - // @ts-ignore - this.IDBKeyRange = IDBKeyRange; + this._storage = storage; + this._writeErrors = []; + } + + get IDBKeyRange(): typeof IDBKeyRange { + return this._storage.IDBKeyRange; + } + + get logger(): BaseLogger { + return this._storage.logger; } _idbStore(name: StoreNames): Store { From 533b0f40d3acec3f5fe178ff1bb5ee6c74018423 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 17 Sep 2021 18:21:48 +0200 Subject: [PATCH 2/5] pass write errors in a store to the transaction --- src/logging/LogItem.js | 10 ++++-- src/matrix/storage/idb/Store.ts | 49 +++++++++++++++++++++++---- src/matrix/storage/idb/Transaction.ts | 40 ++++++++++++++++++++++ 3 files changed, 90 insertions(+), 9 deletions(-) diff --git a/src/logging/LogItem.js b/src/logging/LogItem.js index 69128270..90747964 100644 --- a/src/logging/LogItem.js +++ b/src/logging/LogItem.js @@ -43,12 +43,16 @@ export class LogItem { /** logs a reference to a different log item, usually obtained from runDetached. This is useful if the referenced operation can't be awaited. */ refDetached(logItem, logLevel = null) { - if (!logItem._values.refId) { - logItem.set("refId", this._logger._createRefId()); - } + logItem.ensureRefId(); return this.log({ref: logItem._values.refId}, logLevel); } + ensureRefId() { + if (!this._values.refId) { + this.set("refId", this._logger._createRefId()); + } + } + /** * Creates a new child item and runs it in `callback`. */ diff --git a/src/matrix/storage/idb/Store.ts b/src/matrix/storage/idb/Store.ts index e2da0707..42a41ae6 100644 --- a/src/matrix/storage/idb/Store.ts +++ b/src/matrix/storage/idb/Store.ts @@ -18,6 +18,7 @@ import {QueryTarget, IDBQuery} from "./QueryTarget"; import {IDBRequestAttemptError} from "./error"; import {reqAsPromise} from "./utils"; import {Transaction} from "./Transaction"; +import {LogItem} from "../../../logging/LogItem.js"; const LOG_REQUESTS = false; @@ -148,7 +149,7 @@ export class Store extends QueryTarget { return new QueryTarget(new QueryTargetWrapper(this._idbStore.index(indexName))); } - put(value: T): void { + put(value: T, log?: LogItem): void { // If this request fails, the error will bubble up to the transaction and abort it, // which is the behaviour we want. Therefore, it is ok to not create a promise for this // request and await it. @@ -159,16 +160,52 @@ export class Store extends QueryTarget { // // Note that this can still throw synchronously, like it does for TransactionInactiveError, // see https://www.w3.org/TR/IndexedDB-2/#transaction-lifetime-concept - this._idbStore.put(value); + const request = this._idbStore.put(value); + this._prepareErrorLog(request, log, "put", undefined, value); } - add(value: T): void { + add(value: T, log?: LogItem): void { // ok to not monitor result of request, see comment in `put`. - this._idbStore.add(value); + const request = this._idbStore.add(value); + this._prepareErrorLog(request, log, "add", undefined, value); } - delete(keyOrKeyRange: IDBValidKey | IDBKeyRange): Promise { + delete(keyOrKeyRange: IDBValidKey | IDBKeyRange, log?: LogItem): void { // ok to not monitor result of request, see comment in `put`. - return reqAsPromise(this._idbStore.delete(keyOrKeyRange)); + const request = this._idbStore.delete(keyOrKeyRange); + this._prepareErrorLog(request, log, "delete", keyOrKeyRange, undefined); + } + + private _prepareErrorLog(request: IDBRequest, log: LogItem | undefined, operationName: string, key: IDBValidKey | IDBKeyRange | undefined, value: T | undefined) { + if (log) { + log.ensureRefId(); + } + reqAsPromise(request).catch(err => { + try { + if (!key && value) { + key = this._getKey(value); + } + } catch { + key = "getKey failed"; + } + this._transaction.addWriteError(err, log, operationName, key); + }); + } + + private _getKey(value: T): IDBValidKey { + const {keyPath} = this._idbStore; + if (Array.isArray(keyPath)) { + let field: any = value; + for (const part of keyPath) { + if (typeof field === "object") { + field = field[part]; + } else { + break; + } + } + return field as IDBValidKey; + } else { + return value[keyPath] as IDBValidKey; + } } } diff --git a/src/matrix/storage/idb/Transaction.ts b/src/matrix/storage/idb/Transaction.ts index e9b127ff..291226c5 100644 --- a/src/matrix/storage/idb/Transaction.ts +++ b/src/matrix/storage/idb/Transaction.ts @@ -18,6 +18,7 @@ import {StoreNames} from "../common"; import {txnAsPromise} from "./utils"; import {StorageError} from "../common"; import {Store} from "./Store"; +import {Storage} from "./Storage"; import {SessionStore} from "./stores/SessionStore"; import {RoomSummaryStore} from "./stores/RoomSummaryStore"; import {InviteStore} from "./stores/InviteStore"; @@ -35,13 +36,24 @@ import {OutboundGroupSessionStore} from "./stores/OutboundGroupSessionStore"; import {GroupSessionDecryptionStore} from "./stores/GroupSessionDecryptionStore"; import {OperationStore} from "./stores/OperationStore"; import {AccountDataStore} from "./stores/AccountDataStore"; +import {LogItem} from "../../../logging/LogItem.js"; import {BaseLogger} from "../../../logging/BaseLogger.js"; +class WriteErrorInfo { + constructor( + public readonly error: StorageError, + public readonly refItem: LogItem | undefined, + public readonly operationName: string, + public readonly key: IDBValidKey | IDBKeyRange | undefined, + ) {} +} + export class Transaction { private _txn: IDBTransaction; private _allowedStoreNames: StoreNames[]; private _stores: { [storeName in StoreNames]?: any }; private _storage: Storage; + private _writeErrors: WriteErrorInfo[]; constructor(txn: IDBTransaction, allowedStoreNames: StoreNames[], storage: Storage) { this._txn = txn; @@ -154,5 +166,33 @@ export class Transaction { abort(): void { // TODO: should we wrap the exception in a StorageError? this._txn.abort(); + addWriteError(error: StorageError, refItem: LogItem | undefined, operationName: string, key: IDBValidKey | IDBKeyRange | undefined) { + // don't log subsequent `AbortError`s + if (error.errcode !== "AbortError" || this._writeErrors.length === 0) { + this._writeErrors.push(new WriteErrorInfo(error, refItem, operationName, key)); + } + } + + private _logWriteErrors(parentItem: LogItem | undefined) { + const callback = errorGroupItem => { + // we don't have context when there is no parentItem, so at least log stores + if (!parentItem) { + errorGroupItem.set("allowedStoreNames", this._allowedStoreNames); + } + for (const info of this._writeErrors) { + errorGroupItem.wrap({l: info.operationName, id: info.key}, item => { + if (info.refItem) { + item.refDetached(info.refItem); + } + item.catch(info.error); + }); + } + }; + const label = `${this._writeErrors.length} storage write operation(s) failed`; + if (parentItem) { + parentItem.wrap(label, callback); + } else { + this.logger.run(label, callback); + } } } From ad45016b87e98b570f7d983dca5f2a1e6a715af4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 17 Sep 2021 18:23:31 +0200 Subject: [PATCH 3/5] process write errors on complete or abort in transaction --- src/matrix/Sync.js | 10 +++----- src/matrix/storage/idb/Transaction.ts | 34 +++++++++++++++++++++++---- src/matrix/storage/idb/utils.ts | 17 ++------------ 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index ed9889b1..bcd29ab4 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -296,14 +296,10 @@ export class Sync { // avoid corrupting state by only // storing the sync up till the point // the exception occurred - try { - syncTxn.abort(); - } catch (abortErr) { - log.set("couldNotAbortTxn", true); - } - throw err; + syncTxn.abort(log); + throw syncTxn.getCause(err); } - await syncTxn.complete(); + await syncTxn.complete(log); } _afterSync(sessionState, inviteStates, roomStates, archivedRoomStates, log) { diff --git a/src/matrix/storage/idb/Transaction.ts b/src/matrix/storage/idb/Transaction.ts index 291226c5..84247428 100644 --- a/src/matrix/storage/idb/Transaction.ts +++ b/src/matrix/storage/idb/Transaction.ts @@ -159,13 +159,39 @@ export class Transaction { return this._store(StoreNames.accountData, idbStore => new AccountDataStore(idbStore)); } - complete(): Promise { - return txnAsPromise(this._txn); + async complete(log?: LogItem): Promise { + try { + await txnAsPromise(this._txn); + } catch (err) { + if (this._writeErrors.length) { + this._logWriteErrors(log); + throw this._writeErrors[0].error; + } + throw err; + } } - abort(): void { + getCause(error: Error) { + if (error instanceof StorageError) { + if (error.errcode === "AbortError" && this._writeErrors.length) { + return this._writeErrors[0].error; + } + } + return error; + } + + abort(log?: LogItem): void { // TODO: should we wrap the exception in a StorageError? - this._txn.abort(); + try { + this._txn.abort(); + } catch (abortErr) { + log?.set("couldNotAbortTxn", true); + } + if (this._writeErrors.length) { + this._logWriteErrors(log); + } + } + addWriteError(error: StorageError, refItem: LogItem | undefined, operationName: string, key: IDBValidKey | IDBKeyRange | undefined) { // don't log subsequent `AbortError`s if (error.errcode !== "AbortError" || this._writeErrors.length === 0) { diff --git a/src/matrix/storage/idb/utils.ts b/src/matrix/storage/idb/utils.ts index bd1683ea..f9209f56 100644 --- a/src/matrix/storage/idb/utils.ts +++ b/src/matrix/storage/idb/utils.ts @@ -17,6 +17,7 @@ limitations under the License. import { IDBRequestError } from "./error"; import { StorageError } from "../common"; +import { AbortError } from "../../../utils/error.js"; let needsSyncPromise = false; @@ -112,22 +113,8 @@ export function txnAsPromise(txn): Promise { // @ts-ignore needsSyncPromise && Promise._flush && Promise._flush(); }); - txn.addEventListener("error", event => { - const request = event.target; - // catch first error here, but don't reject yet, - // as we don't have access to the failed request in the abort event handler - if (!error && request) { - error = new IDBRequestError(request); - } - }); txn.addEventListener("abort", event => { - if (!error) { - const txn = event.target; - const dbName = txn.db.name; - const storeNames = Array.from(txn.objectStoreNames).join(", ") - error = new StorageError(`Transaction on ${dbName} with stores ${storeNames} was aborted.`); - } - reject(error); + reject(new AbortError()); // @ts-ignore needsSyncPromise && Promise._flush && Promise._flush(); }); From f5467a653cebacb347cc68014bd36c1f0b2a72f8 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 17 Sep 2021 18:24:24 +0200 Subject: [PATCH 4/5] fix stores returning the delete promise which isn't returned anymore I checked these aren't awaited in any js file --- src/matrix/storage/idb/stores/DeviceIdentityStore.ts | 8 ++++---- .../storage/idb/stores/GroupSessionDecryptionStore.ts | 4 ++-- src/matrix/storage/idb/stores/InboundGroupSessionStore.ts | 4 ++-- src/matrix/storage/idb/stores/OlmSessionStore.ts | 4 ++-- src/matrix/storage/idb/stores/OperationStore.ts | 4 ++-- .../storage/idb/stores/OutboundGroupSessionStore.ts | 4 ++-- src/matrix/storage/idb/stores/PendingEventStore.ts | 8 ++++---- src/matrix/storage/idb/stores/RoomStateStore.ts | 4 ++-- src/matrix/storage/idb/stores/RoomSummaryStore.ts | 4 ++-- src/matrix/storage/idb/stores/TimelineFragmentStore.ts | 4 ++-- src/matrix/storage/idb/stores/TimelineRelationStore.ts | 8 ++++---- src/matrix/storage/idb/stores/UserIdentityStore.ts | 4 ++-- 12 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/matrix/storage/idb/stores/DeviceIdentityStore.ts b/src/matrix/storage/idb/stores/DeviceIdentityStore.ts index 9800fcca..6b9f5332 100644 --- a/src/matrix/storage/idb/stores/DeviceIdentityStore.ts +++ b/src/matrix/storage/idb/stores/DeviceIdentityStore.ts @@ -78,14 +78,14 @@ export class DeviceIdentityStore { return this._store.index("byCurve25519Key").get(curve25519Key); } - remove(userId: string, deviceId: string): Promise { - return this._store.delete(encodeKey(userId, deviceId)); + remove(userId: string, deviceId: string): void { + this._store.delete(encodeKey(userId, deviceId)); } - removeAllForUser(userId: string): Promise { + removeAllForUser(userId: string): void { // exclude both keys as they are theoretical min and max, // but we should't have a match for just the room id, or room id with max const range = this._store.IDBKeyRange.bound(encodeKey(userId, MIN_UNICODE), encodeKey(userId, MAX_UNICODE), true, true); - return this._store.delete(range); + this._store.delete(range); } } diff --git a/src/matrix/storage/idb/stores/GroupSessionDecryptionStore.ts b/src/matrix/storage/idb/stores/GroupSessionDecryptionStore.ts index b6636f13..1627e44a 100644 --- a/src/matrix/storage/idb/stores/GroupSessionDecryptionStore.ts +++ b/src/matrix/storage/idb/stores/GroupSessionDecryptionStore.ts @@ -44,11 +44,11 @@ export class GroupSessionDecryptionStore { this._store.put(decryption as GroupSessionEntry); } - removeAllForRoom(roomId: string): Promise { + removeAllForRoom(roomId: string): void { const range = this._store.IDBKeyRange.bound( encodeKey(roomId, MIN_UNICODE, MIN_UNICODE), encodeKey(roomId, MAX_UNICODE, MAX_UNICODE) ); - return this._store.delete(range); + this._store.delete(range); } } diff --git a/src/matrix/storage/idb/stores/InboundGroupSessionStore.ts b/src/matrix/storage/idb/stores/InboundGroupSessionStore.ts index 286eaf4e..5dc0205f 100644 --- a/src/matrix/storage/idb/stores/InboundGroupSessionStore.ts +++ b/src/matrix/storage/idb/stores/InboundGroupSessionStore.ts @@ -53,11 +53,11 @@ export class InboundGroupSessionStore { this._store.put(session); } - removeAllForRoom(roomId: string): Promise { + removeAllForRoom(roomId: string) { const range = this._store.IDBKeyRange.bound( encodeKey(roomId, MIN_UNICODE, MIN_UNICODE), encodeKey(roomId, MAX_UNICODE, MAX_UNICODE) ); - return this._store.delete(range); + this._store.delete(range); } } diff --git a/src/matrix/storage/idb/stores/OlmSessionStore.ts b/src/matrix/storage/idb/stores/OlmSessionStore.ts index 1daa0b21..3310215e 100644 --- a/src/matrix/storage/idb/stores/OlmSessionStore.ts +++ b/src/matrix/storage/idb/stores/OlmSessionStore.ts @@ -71,7 +71,7 @@ export class OlmSessionStore { this._store.put(session as OlmSessionEntry); } - remove(senderKey: string, sessionId: string): Promise { - return this._store.delete(encodeKey(senderKey, sessionId)); + remove(senderKey: string, sessionId: string): void { + this._store.delete(encodeKey(senderKey, sessionId)); } } diff --git a/src/matrix/storage/idb/stores/OperationStore.ts b/src/matrix/storage/idb/stores/OperationStore.ts index cccd8e2d..0e103e8c 100644 --- a/src/matrix/storage/idb/stores/OperationStore.ts +++ b/src/matrix/storage/idb/stores/OperationStore.ts @@ -73,8 +73,8 @@ export class OperationStore { this._store.put(operation as OperationEntry); } - remove(id: string): Promise { - return this._store.delete(id); + remove(id: string): void { + this._store.delete(id); } async removeAllForScope(scope: string): Promise { diff --git a/src/matrix/storage/idb/stores/OutboundGroupSessionStore.ts b/src/matrix/storage/idb/stores/OutboundGroupSessionStore.ts index 82a87625..9712c717 100644 --- a/src/matrix/storage/idb/stores/OutboundGroupSessionStore.ts +++ b/src/matrix/storage/idb/stores/OutboundGroupSessionStore.ts @@ -28,8 +28,8 @@ export class OutboundGroupSessionStore { this._store = store; } - remove(roomId: string): Promise { - return this._store.delete(roomId); + remove(roomId: string): void { + this._store.delete(roomId); } get(roomId: string): Promise { diff --git a/src/matrix/storage/idb/stores/PendingEventStore.ts b/src/matrix/storage/idb/stores/PendingEventStore.ts index 01991ce8..d4bef05a 100644 --- a/src/matrix/storage/idb/stores/PendingEventStore.ts +++ b/src/matrix/storage/idb/stores/PendingEventStore.ts @@ -62,9 +62,9 @@ export class PendingEventStore { } } - remove(roomId: string, queueIndex: number): Promise { + remove(roomId: string, queueIndex: number) { const keyRange = this._eventStore.IDBKeyRange.only(encodeKey(roomId, queueIndex)); - return this._eventStore.delete(keyRange); + this._eventStore.delete(keyRange); } async exists(roomId: string, queueIndex: number): Promise { @@ -86,10 +86,10 @@ export class PendingEventStore { return this._eventStore.selectAll(); } - removeAllForRoom(roomId: string): Promise { + removeAllForRoom(roomId: string): void { const minKey = encodeKey(roomId, KeyLimits.minStorageKey); const maxKey = encodeKey(roomId, KeyLimits.maxStorageKey); const range = this._eventStore.IDBKeyRange.bound(minKey, maxKey); - return this._eventStore.delete(range); + this._eventStore.delete(range); } } diff --git a/src/matrix/storage/idb/stores/RoomStateStore.ts b/src/matrix/storage/idb/stores/RoomStateStore.ts index 441d61e4..b7ece0f7 100644 --- a/src/matrix/storage/idb/stores/RoomStateStore.ts +++ b/src/matrix/storage/idb/stores/RoomStateStore.ts @@ -47,10 +47,10 @@ export class RoomStateStore { this._roomStateStore.put(entry); } - removeAllForRoom(roomId: string): Promise { + removeAllForRoom(roomId: string): void { // exclude both keys as they are theoretical min and max, // but we should't have a match for just the room id, or room id with max const range = this._roomStateStore.IDBKeyRange.bound(roomId, `${roomId}|${MAX_UNICODE}`, true, true); - return this._roomStateStore.delete(range); + this._roomStateStore.delete(range); } } diff --git a/src/matrix/storage/idb/stores/RoomSummaryStore.ts b/src/matrix/storage/idb/stores/RoomSummaryStore.ts index bd911572..43c62fdb 100644 --- a/src/matrix/storage/idb/stores/RoomSummaryStore.ts +++ b/src/matrix/storage/idb/stores/RoomSummaryStore.ts @@ -55,7 +55,7 @@ export class RoomSummaryStore { return roomId === fetchedKey; } - remove(roomId: string): Promise { - return this._summaryStore.delete(roomId); + remove(roomId: string): void { + this._summaryStore.delete(roomId); } } diff --git a/src/matrix/storage/idb/stores/TimelineFragmentStore.ts b/src/matrix/storage/idb/stores/TimelineFragmentStore.ts index 813fc3f3..5753a93e 100644 --- a/src/matrix/storage/idb/stores/TimelineFragmentStore.ts +++ b/src/matrix/storage/idb/stores/TimelineFragmentStore.ts @@ -87,7 +87,7 @@ export class TimelineFragmentStore { return this._store.get(encodeKey(roomId, fragmentId)); } - removeAllForRoom(roomId: string): Promise { - return this._store.delete(this._allRange(roomId)); + removeAllForRoom(roomId: string): void { + this._store.delete(this._allRange(roomId)); } } diff --git a/src/matrix/storage/idb/stores/TimelineRelationStore.ts b/src/matrix/storage/idb/stores/TimelineRelationStore.ts index 6772864a..88ef86ae 100644 --- a/src/matrix/storage/idb/stores/TimelineRelationStore.ts +++ b/src/matrix/storage/idb/stores/TimelineRelationStore.ts @@ -43,18 +43,18 @@ export class TimelineRelationStore { this._store.add({key: encodeKey(roomId, targetEventId, relType, sourceEventId)}); } - remove(roomId: string, targetEventId: string, relType: string, sourceEventId: string): Promise { - return this._store.delete(encodeKey(roomId, targetEventId, relType, sourceEventId)); + remove(roomId: string, targetEventId: string, relType: string, sourceEventId: string): void { + this._store.delete(encodeKey(roomId, targetEventId, relType, sourceEventId)); } - removeAllForTarget(roomId: string, targetId: string): Promise { + removeAllForTarget(roomId: string, targetId: string): void { const range = this._store.IDBKeyRange.bound( encodeKey(roomId, targetId, MIN_UNICODE, MIN_UNICODE), encodeKey(roomId, targetId, MAX_UNICODE, MAX_UNICODE), true, true ); - return this._store.delete(range); + this._store.delete(range); } async getForTargetAndType(roomId: string, targetId: string, relType: string): Promise { diff --git a/src/matrix/storage/idb/stores/UserIdentityStore.ts b/src/matrix/storage/idb/stores/UserIdentityStore.ts index 1d94a666..692d8384 100644 --- a/src/matrix/storage/idb/stores/UserIdentityStore.ts +++ b/src/matrix/storage/idb/stores/UserIdentityStore.ts @@ -36,7 +36,7 @@ export class UserIdentityStore { this._store.put(userIdentity); } - remove(userId: string): Promise { - return this._store.delete(userId); + remove(userId: string): void { + this._store.delete(userId); } } From 53a0d292447c7248cf1367596f7197a3768e4240 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 17 Sep 2021 18:25:28 +0200 Subject: [PATCH 5/5] example how to get more context for the failure of a write operation --- src/matrix/room/timeline/persistence/SyncWriter.js | 2 +- src/matrix/storage/idb/stores/TimelineEventStore.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index 1ee2ce7d..07326225 100644 --- a/src/matrix/room/timeline/persistence/SyncWriter.js +++ b/src/matrix/room/timeline/persistence/SyncWriter.js @@ -162,7 +162,7 @@ export class SyncWriter { storageEntry.displayName = member.displayName; storageEntry.avatarUrl = member.avatarUrl; } - txn.timelineEvents.insert(storageEntry); + txn.timelineEvents.insert(storageEntry, log); const entry = new EventEntry(storageEntry, this._fragmentIdComparer); entries.push(entry); const updatedRelationTargetEntries = await this._relationWriter.writeRelation(entry, txn, log); diff --git a/src/matrix/storage/idb/stores/TimelineEventStore.ts b/src/matrix/storage/idb/stores/TimelineEventStore.ts index 49892771..535b8f7e 100644 --- a/src/matrix/storage/idb/stores/TimelineEventStore.ts +++ b/src/matrix/storage/idb/stores/TimelineEventStore.ts @@ -20,6 +20,7 @@ import { encodeUint32 } from "../utils"; import {KeyLimits} from "../../common"; import {Store} from "../Store"; import {TimelineEvent, StateEvent} from "../../types"; +import {LogItem} from "../../../../logging/LogItem.js"; interface Annotation { count: number; @@ -265,11 +266,10 @@ export class TimelineEventStore { * @return nothing. To wait for the operation to finish, await the transaction it's part of. * @throws {StorageError} ... */ - insert(entry: TimelineEventEntry): void { + insert(entry: TimelineEventEntry, log: LogItem): void { (entry as TimelineEventStorageEntry).key = encodeKey(entry.roomId, entry.fragmentId, entry.eventIndex); (entry as TimelineEventStorageEntry).eventIdKey = encodeEventIdKey(entry.roomId, entry.event.event_id); - // TODO: map error? or in idb/store? - this._timelineStore.add(entry as TimelineEventStorageEntry); + this._timelineStore.add(entry as TimelineEventStorageEntry, log); } /** Updates the entry into the store with the given [roomId, eventKey] combination.