From 533b0f40d3acec3f5fe178ff1bb5ee6c74018423 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 17 Sep 2021 18:21:48 +0200 Subject: [PATCH] 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); + } } }