From 2d2521cd9af7008f1a06c6d367afeaea7e6e340e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 21 Sep 2021 20:58:16 +0200 Subject: [PATCH 01/10] add prototype to show we can prevent the txn from being aborted on error --- .../idb-continue-on-constrainterror.html | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 prototypes/idb-continue-on-constrainterror.html diff --git a/prototypes/idb-continue-on-constrainterror.html b/prototypes/idb-continue-on-constrainterror.html new file mode 100644 index 00000000..71e56c27 --- /dev/null +++ b/prototypes/idb-continue-on-constrainterror.html @@ -0,0 +1,100 @@ + + + + + + + + + + + From 0d486a14f666276f1ad425344026eccbc506725c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 21 Sep 2021 20:58:50 +0200 Subject: [PATCH 02/10] add the logger property to the null logger as well, forgot this before --- src/logging/NullLogger.js | 6 +++++- src/mocks/Storage.js | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/logging/NullLogger.js b/src/logging/NullLogger.js index 202d01f3..da04f16b 100644 --- a/src/logging/NullLogger.js +++ b/src/logging/NullLogger.js @@ -20,7 +20,7 @@ function noop () {} export class NullLogger { constructor() { - this.item = new NullLogItem(); + this.item = new NullLogItem(this); } log() {} @@ -51,6 +51,10 @@ export class NullLogger { } export class NullLogItem { + constructor(logger) { + this.logger = logger; + } + wrap(_, callback) { return callback(this); } diff --git a/src/mocks/Storage.js b/src/mocks/Storage.js index 359ffa2c..876cc009 100644 --- a/src/mocks/Storage.js +++ b/src/mocks/Storage.js @@ -16,8 +16,8 @@ limitations under the License. import {FDBFactory, FDBKeyRange} from "../../lib/fake-indexeddb/index.js"; import {StorageFactory} from "../matrix/storage/idb/StorageFactory"; -import {NullLogItem} from "../logging/NullLogger.js"; +import {Instance as nullLogger} from "../logging/NullLogger.js"; export function createMockStorage() { - return new StorageFactory(null, new FDBFactory(), FDBKeyRange).create(1, new NullLogItem()); + return new StorageFactory(null, new FDBFactory(), FDBKeyRange).create(1, nullLogger.item); } From 12add19c314a9d8f708d44f976718913e9adaac4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 21 Sep 2021 21:02:39 +0200 Subject: [PATCH 03/10] add Store.tryAdd, which prevent abort on ConstraintError --- src/matrix/storage/idb/Store.ts | 15 ++++++++++++++- src/matrix/storage/idb/error.ts | 12 ++++++++++-- src/matrix/storage/idb/utils.ts | 6 +++--- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/matrix/storage/idb/Store.ts b/src/matrix/storage/idb/Store.ts index 42a41ae6..4a018d23 100644 --- a/src/matrix/storage/idb/Store.ts +++ b/src/matrix/storage/idb/Store.ts @@ -15,7 +15,7 @@ limitations under the License. */ import {QueryTarget, IDBQuery} from "./QueryTarget"; -import {IDBRequestAttemptError} from "./error"; +import {IDBRequestError, IDBRequestAttemptError} from "./error"; import {reqAsPromise} from "./utils"; import {Transaction} from "./Transaction"; import {LogItem} from "../../../logging/LogItem.js"; @@ -169,6 +169,19 @@ export class Store extends QueryTarget { const request = this._idbStore.add(value); this._prepareErrorLog(request, log, "add", undefined, value); } + + async tryAdd(value: T, log: LogItem): Promise { + try { + await reqAsPromise(this._idbStore.add(value)); + return true; + } catch (err) { + if (err instanceof IDBRequestError) { + log.log({l: "could not write", id: this._getKey(value), e: err}, log.level.Warn); + err.preventTransactionAbort(); + } + return false; + } + } delete(keyOrKeyRange: IDBValidKey | IDBKeyRange, log?: LogItem): void { // ok to not monitor result of request, see comment in `put`. diff --git a/src/matrix/storage/idb/error.ts b/src/matrix/storage/idb/error.ts index 388ad4c0..fb602168 100644 --- a/src/matrix/storage/idb/error.ts +++ b/src/matrix/storage/idb/error.ts @@ -57,10 +57,18 @@ export class IDBError extends StorageError { } export class IDBRequestError extends IDBError { - constructor(request: IDBRequest, message: string = "IDBRequest failed") { + private errorEvent: Event; + + constructor(errorEvent: Event) { + const request = errorEvent.target as IDBRequest; const source = request.source; const cause = request.error; - super(message, source, cause); + super("IDBRequest failed", source, cause); + this.errorEvent = errorEvent; + } + + preventTransactionAbort() { + this.errorEvent.preventDefault(); } } diff --git a/src/matrix/storage/idb/utils.ts b/src/matrix/storage/idb/utils.ts index f9209f56..a9432139 100644 --- a/src/matrix/storage/idb/utils.ts +++ b/src/matrix/storage/idb/utils.ts @@ -97,7 +97,7 @@ export function reqAsPromise(req: IDBRequest): Promise { needsSyncPromise && Promise._flush && Promise._flush(); }); req.addEventListener("error", event => { - const error = new IDBRequestError(event.target as IDBRequest); + const error = new IDBRequestError(event); reject(error); // @ts-ignore needsSyncPromise && Promise._flush && Promise._flush(); @@ -143,8 +143,8 @@ type CursorIterator = (value: I extends IDBCursorWithVal export function iterateCursor(cursorRequest: IDBRequest, processValue: CursorIterator): Promise { // TODO: does cursor already have a value here?? return new Promise((resolve, reject) => { - cursorRequest.onerror = () => { - reject(new IDBRequestError(cursorRequest)); + cursorRequest.onerror = event => { + reject(new IDBRequestError(event)); // @ts-ignore needsSyncPromise && Promise._flush && Promise._flush(); }; From 6cded5319a213c21ca48e90b93cd7ee3c690f9a3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 21 Sep 2021 21:04:10 +0200 Subject: [PATCH 04/10] change timelineEventStore.insert to tryInsert --- .../session/room/timeline/ReactionsViewModel.js | 6 +++--- src/matrix/room/timeline/Timeline.js | 4 ++-- src/matrix/room/timeline/persistence/GapWriter.js | 9 ++++++--- .../room/timeline/persistence/RelationWriter.js | 12 ++++++------ src/matrix/room/timeline/persistence/SyncWriter.js | 6 +++++- .../storage/idb/stores/TimelineEventStore.ts | 14 ++++++++------ 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/domain/session/room/timeline/ReactionsViewModel.js b/src/domain/session/room/timeline/ReactionsViewModel.js index 3fd9f15f..a9ac4f21 100644 --- a/src/domain/session/room/timeline/ReactionsViewModel.js +++ b/src/domain/session/room/timeline/ReactionsViewModel.js @@ -247,8 +247,8 @@ export function tests() { storage.storeNames.timelineFragments ]); txn.timelineFragments.add({id: 1, roomId}); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 3, event: myReactionEvent, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 3, event: myReactionEvent, roomId}); await relationWriter.writeRelation(myReactionEntry, txn, new NullLogItem()); await txn.complete(); // 2. setup queue & timeline @@ -309,7 +309,7 @@ export function tests() { storage.storeNames.timelineFragments ]); txn.timelineFragments.add({id: 1, roomId}); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}); await txn.complete(); // 2. setup queue & timeline const queue = new SendQueue({roomId, storage, hsApi: new MockHomeServer().api}); diff --git a/src/matrix/room/timeline/Timeline.js b/src/matrix/room/timeline/Timeline.js index 54eabf96..1677a840 100644 --- a/src/matrix/room/timeline/Timeline.js +++ b/src/matrix/room/timeline/Timeline.js @@ -447,7 +447,7 @@ export function tests() { // 1. put event and reaction into storage const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.insert({ + txn.timelineEvents.tryInsert({ event: withContent(createAnnotation(messageId, "👋"), createEvent("m.reaction", reactionId, bob)), fragmentId: 1, eventIndex: 1, roomId }); @@ -543,7 +543,7 @@ export function tests() { // 1. put reaction in storage const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.insert({ + txn.timelineEvents.tryInsert({ event: withContent(createAnnotation(messageId, "👋"), createEvent("m.reaction", reactionId, bob)), fragmentId: 1, eventIndex: 3, roomId }); diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js index e133d713..be14922a 100644 --- a/src/matrix/room/timeline/persistence/GapWriter.js +++ b/src/matrix/room/timeline/persistence/GapWriter.js @@ -124,9 +124,12 @@ export class GapWriter { if (updatedRelationTargetEntries) { updatedEntries.push(...updatedRelationTargetEntries); } - txn.timelineEvents.insert(eventStorageEntry); - const eventEntry = new EventEntry(eventStorageEntry, this._fragmentIdComparer); - directionalAppend(entries, eventEntry, direction); + if (await txn.timelineEvents.tryInsert(eventStorageEntry)) { + const eventEntry = new EventEntry(eventStorageEntry, this._fragmentIdComparer); + directionalAppend(entries, eventEntry, direction); + } else { + log.log({l: `could not write event`, id: event.event_id}, log.level.Warn); + } } return {entries, updatedEntries}; } diff --git a/src/matrix/room/timeline/persistence/RelationWriter.js b/src/matrix/room/timeline/persistence/RelationWriter.js index 4116b775..d8619cf9 100644 --- a/src/matrix/room/timeline/persistence/RelationWriter.js +++ b/src/matrix/room/timeline/persistence/RelationWriter.js @@ -275,7 +275,7 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 2, event, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); const updatedEntries = await relationWriter.writeRelation(redactionEntry, txn, new NullLogItem()); await txn.complete(); @@ -300,7 +300,7 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 2, event, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); const updatedEntries = await relationWriter.writeRelation(reactionEntry, txn, new NullLogItem()); await txn.complete(); @@ -329,7 +329,7 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 2, event, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); await relationWriter.writeRelation(reaction1Entry, txn, new NullLogItem()); const updatedEntries = await relationWriter.writeRelation(reaction2Entry, txn, new NullLogItem()); await txn.complete(); @@ -358,10 +358,10 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 2, event, roomId}); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 3, event: myReaction, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 3, event: myReaction, roomId}); await relationWriter.writeRelation(myReactionEntry, txn, new NullLogItem()); - txn.timelineEvents.insert({fragmentId: 1, eventIndex: 4, event: bobReaction, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 4, event: bobReaction, roomId}); await relationWriter.writeRelation(bobReactionEntry, txn, new NullLogItem()); const updatedEntries = await relationWriter.writeRelation(myReactionRedactionEntry, txn, new NullLogItem()); await txn.complete(); diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index 07326225..36d3c913 100644 --- a/src/matrix/room/timeline/persistence/SyncWriter.js +++ b/src/matrix/room/timeline/persistence/SyncWriter.js @@ -162,7 +162,11 @@ export class SyncWriter { storageEntry.displayName = member.displayName; storageEntry.avatarUrl = member.avatarUrl; } - txn.timelineEvents.insert(storageEntry, log); + const couldInsert = await txn.timelineEvents.tryInsert(storageEntry, log); + if (!couldInsert) { + log.log({l: `could not write event, likely #504`, id: event.event_id}, log.level.Warn); + continue; + } 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 535b8f7e..3d27e8a7 100644 --- a/src/matrix/storage/idb/stores/TimelineEventStore.ts +++ b/src/matrix/storage/idb/stores/TimelineEventStore.ts @@ -261,15 +261,17 @@ export class TimelineEventStore { return firstFoundKey && decodeEventIdKey(firstFoundKey).eventId; } - /** Inserts a new entry into the store. The combination of roomId and eventKey should not exist yet, or an error is thrown. - * @param entry the entry to insert - * @return nothing. To wait for the operation to finish, await the transaction it's part of. - * @throws {StorageError} ... + /** Inserts a new entry into the store. + * + * If the event already exists in the store (either the eventKey or the event id + * are already known for the given roomId), this operation has no effect. + * + * Returns if the event was not yet known and the entry was written. */ - insert(entry: TimelineEventEntry, log: LogItem): void { + tryInsert(entry: TimelineEventEntry, log: LogItem): Promise { (entry as TimelineEventStorageEntry).key = encodeKey(entry.roomId, entry.fragmentId, entry.eventIndex); (entry as TimelineEventStorageEntry).eventIdKey = encodeEventIdKey(entry.roomId, entry.event.event_id); - this._timelineStore.add(entry as TimelineEventStorageEntry, log); + return this._timelineStore.tryAdd(entry as TimelineEventStorageEntry, log); } /** Updates the entry into the store with the given [roomId, eventKey] combination. From 704d7b32da1c39d973afa781da40c9312e31c0f1 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 21 Sep 2021 21:04:29 +0200 Subject: [PATCH 05/10] add tests --- .../room/timeline/persistence/SyncWriter.js | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index 36d3c913..35a5dbef 100644 --- a/src/matrix/room/timeline/persistence/SyncWriter.js +++ b/src/matrix/room/timeline/persistence/SyncWriter.js @@ -256,3 +256,35 @@ export class SyncWriter { return this._lastLiveKey; } } + +import {createMockStorage} from "../../../../mocks/Storage.js"; +import {createEvent, withTextBody} from "../../../../mocks/event.js"; +import {Instance as nullLogger} from "../../../../logging/NullLogger.js"; +export function tests() { + const roomId = "!abc:hs.tld"; + return { + "calling timelineEvents.tryInsert with the same event id a second time fails": async assert => { + const storage = await createMockStorage(); + const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents]); + const event = withTextBody("hello!", createEvent("m.room.message", "$abc", "@alice:hs.tld")); + const entry1 = createEventEntry(EventKey.defaultLiveKey, roomId, event); + assert.equal(await txn.timelineEvents.tryInsert(entry1, nullLogger.item), true); + const entry2 = createEventEntry(EventKey.defaultLiveKey.nextKey(), roomId, event); + assert.equal(await txn.timelineEvents.tryInsert(entry2, nullLogger.item), false); + // fake-indexeddb still aborts the transaction when preventDefault is called by tryInsert, so don't await as it will abort + // await txn.complete(); + }, + "calling timelineEvents.tryInsert with the same event key a second time fails": async assert => { + const storage = await createMockStorage(); + const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents]); + const event1 = withTextBody("hello!", createEvent("m.room.message", "$abc", "@alice:hs.tld")); + const entry1 = createEventEntry(EventKey.defaultLiveKey, roomId, event1); + assert.equal(await txn.timelineEvents.tryInsert(entry1, nullLogger.item), true); + const event2 = withTextBody("hello!", createEvent("m.room.message", "$def", "@alice:hs.tld")); + const entry2 = createEventEntry(EventKey.defaultLiveKey, roomId, event2); + assert.equal(await txn.timelineEvents.tryInsert(entry2, nullLogger.item), false); + // fake-indexeddb still aborts the transaction when preventDefault is called by tryInsert, so don't await as it will abort + // await txn.complete(); + }, + } +} From a19d93dbeff76af81a29f23cd51bd2e59120fe1d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 22 Sep 2021 09:36:26 +0200 Subject: [PATCH 06/10] don't swallow anything that isn't a request error --- src/matrix/storage/idb/Store.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/matrix/storage/idb/Store.ts b/src/matrix/storage/idb/Store.ts index 4a018d23..83627047 100644 --- a/src/matrix/storage/idb/Store.ts +++ b/src/matrix/storage/idb/Store.ts @@ -169,7 +169,7 @@ export class Store extends QueryTarget { const request = this._idbStore.add(value); this._prepareErrorLog(request, log, "add", undefined, value); } - + async tryAdd(value: T, log: LogItem): Promise { try { await reqAsPromise(this._idbStore.add(value)); @@ -178,8 +178,10 @@ export class Store extends QueryTarget { if (err instanceof IDBRequestError) { log.log({l: "could not write", id: this._getKey(value), e: err}, log.level.Warn); err.preventTransactionAbort(); + return false; + } else { + throw err; } - return false; } } From 1963635dd72e9bf7a31662f1042848b3b04166e1 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 22 Sep 2021 10:22:52 +0200 Subject: [PATCH 07/10] also log index keys for a value when write fails in Store --- src/matrix/storage/idb/Store.ts | 45 +++++++++++++++++++++------ src/matrix/storage/idb/Transaction.ts | 10 +++--- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/matrix/storage/idb/Store.ts b/src/matrix/storage/idb/Store.ts index 83627047..e3d6e3f7 100644 --- a/src/matrix/storage/idb/Store.ts +++ b/src/matrix/storage/idb/Store.ts @@ -17,7 +17,7 @@ limitations under the License. import {QueryTarget, IDBQuery} from "./QueryTarget"; import {IDBRequestError, IDBRequestAttemptError} from "./error"; import {reqAsPromise} from "./utils"; -import {Transaction} from "./Transaction"; +import {Transaction, IDBKey} from "./Transaction"; import {LogItem} from "../../../logging/LogItem.js"; const LOG_REQUESTS = false; @@ -126,6 +126,10 @@ class QueryTargetWrapper { throw new IDBRequestAttemptError("index", this._qt, err, [name]); } } + + get indexNames(): string[] { + return Array.from(this._qtStore.indexNames); + } } export class Store extends QueryTarget { @@ -176,7 +180,7 @@ export class Store extends QueryTarget { return true; } catch (err) { if (err instanceof IDBRequestError) { - log.log({l: "could not write", id: this._getKey(value), e: err}, log.level.Warn); + log.log({l: "could not write", id: this._getKeys(value), e: err}, log.level.Warn); err.preventTransactionAbort(); return false; } else { @@ -191,24 +195,45 @@ export class Store extends QueryTarget { this._prepareErrorLog(request, log, "delete", keyOrKeyRange, undefined); } - private _prepareErrorLog(request: IDBRequest, log: LogItem | undefined, operationName: string, key: IDBValidKey | IDBKeyRange | undefined, value: T | undefined) { + private _prepareErrorLog(request: IDBRequest, log: LogItem | undefined, operationName: string, key: IDBKey | undefined, value: T | undefined) { if (log) { log.ensureRefId(); } reqAsPromise(request).catch(err => { + let keys : IDBKey[] | undefined = undefined; try { - if (!key && value) { - key = this._getKey(value); + if (value) { + keys = this._getKeys(value); + } else if (key) { + keys = [key]; } - } catch { - key = "getKey failed"; + } catch (err) { + console.error("_getKeys failed", err); } - this._transaction.addWriteError(err, log, operationName, key); + this._transaction.addWriteError(err, log, operationName, keys); }); } - private _getKey(value: T): IDBValidKey { + private _getKeys(value: T): IDBValidKey[] { + const keys: IDBValidKey[] = []; const {keyPath} = this._idbStore; + try { + keys.push(this._readKeyPath(value, keyPath)); + } catch (err) { + console.warn("could not read keyPath", keyPath); + } + for (const indexName of this._idbStore.indexNames) { + try { + const index = this._idbStore.index(indexName); + keys.push(this._readKeyPath(value, index.keyPath)); + } catch (err) { + console.warn("could not read index", indexName); + } + } + return keys; + } + + private _readKeyPath(value: T, keyPath: string[] | string): IDBValidKey { if (Array.isArray(keyPath)) { let field: any = value; for (const part of keyPath) { @@ -221,6 +246,6 @@ export class Store extends QueryTarget { 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 84247428..cd68457c 100644 --- a/src/matrix/storage/idb/Transaction.ts +++ b/src/matrix/storage/idb/Transaction.ts @@ -39,12 +39,14 @@ import {AccountDataStore} from "./stores/AccountDataStore"; import {LogItem} from "../../../logging/LogItem.js"; import {BaseLogger} from "../../../logging/BaseLogger.js"; +export type IDBKey = IDBValidKey | IDBKeyRange; + class WriteErrorInfo { constructor( public readonly error: StorageError, public readonly refItem: LogItem | undefined, public readonly operationName: string, - public readonly key: IDBValidKey | IDBKeyRange | undefined, + public readonly keys: IDBKey[] | undefined, ) {} } @@ -192,10 +194,10 @@ export class Transaction { } } - addWriteError(error: StorageError, refItem: LogItem | undefined, operationName: string, key: IDBValidKey | IDBKeyRange | undefined) { + addWriteError(error: StorageError, refItem: LogItem | undefined, operationName: string, keys: IDBKey[] | undefined) { // don't log subsequent `AbortError`s if (error.errcode !== "AbortError" || this._writeErrors.length === 0) { - this._writeErrors.push(new WriteErrorInfo(error, refItem, operationName, key)); + this._writeErrors.push(new WriteErrorInfo(error, refItem, operationName, keys)); } } @@ -206,7 +208,7 @@ export class Transaction { errorGroupItem.set("allowedStoreNames", this._allowedStoreNames); } for (const info of this._writeErrors) { - errorGroupItem.wrap({l: info.operationName, id: info.key}, item => { + errorGroupItem.wrap({l: info.operationName, id: info.keys}, item => { if (info.refItem) { item.refDetached(info.refItem); } From b58e10521f56988230440c7f9235c32a16ebbf6d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 22 Sep 2021 10:23:28 +0200 Subject: [PATCH 08/10] don't log tryInsert failures anymore as everything is logged in Store --- src/matrix/room/timeline/persistence/GapWriter.js | 2 -- src/matrix/room/timeline/persistence/SyncWriter.js | 1 - 2 files changed, 3 deletions(-) diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js index be14922a..cfcebf7e 100644 --- a/src/matrix/room/timeline/persistence/GapWriter.js +++ b/src/matrix/room/timeline/persistence/GapWriter.js @@ -127,8 +127,6 @@ export class GapWriter { if (await txn.timelineEvents.tryInsert(eventStorageEntry)) { const eventEntry = new EventEntry(eventStorageEntry, this._fragmentIdComparer); directionalAppend(entries, eventEntry, direction); - } else { - log.log({l: `could not write event`, id: event.event_id}, log.level.Warn); } } return {entries, updatedEntries}; diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index 35a5dbef..af6f55bc 100644 --- a/src/matrix/room/timeline/persistence/SyncWriter.js +++ b/src/matrix/room/timeline/persistence/SyncWriter.js @@ -164,7 +164,6 @@ export class SyncWriter { } const couldInsert = await txn.timelineEvents.tryInsert(storageEntry, log); if (!couldInsert) { - log.log({l: `could not write event, likely #504`, id: event.event_id}, log.level.Warn); continue; } const entry = new EventEntry(storageEntry, this._fragmentIdComparer); From ac5a4c2bc6c2e88bbfc05aed27deab051ef4b044 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 22 Sep 2021 10:33:40 +0200 Subject: [PATCH 09/10] pass log item everywhere to tryInsert --- .../session/room/timeline/ReactionsViewModel.js | 6 +++--- src/matrix/room/timeline/Timeline.js | 4 ++-- src/matrix/room/timeline/persistence/GapWriter.js | 2 +- .../room/timeline/persistence/RelationWriter.js | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/domain/session/room/timeline/ReactionsViewModel.js b/src/domain/session/room/timeline/ReactionsViewModel.js index a9ac4f21..f0dcf79f 100644 --- a/src/domain/session/room/timeline/ReactionsViewModel.js +++ b/src/domain/session/room/timeline/ReactionsViewModel.js @@ -247,8 +247,8 @@ export function tests() { storage.storeNames.timelineFragments ]); txn.timelineFragments.add({id: 1, roomId}); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 3, event: myReactionEvent, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}, new NullLogItem()); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 3, event: myReactionEvent, roomId}, new NullLogItem()); await relationWriter.writeRelation(myReactionEntry, txn, new NullLogItem()); await txn.complete(); // 2. setup queue & timeline @@ -309,7 +309,7 @@ export function tests() { storage.storeNames.timelineFragments ]); txn.timelineFragments.add({id: 1, roomId}); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event: messageEvent, roomId}, new NullLogItem()); await txn.complete(); // 2. setup queue & timeline const queue = new SendQueue({roomId, storage, hsApi: new MockHomeServer().api}); diff --git a/src/matrix/room/timeline/Timeline.js b/src/matrix/room/timeline/Timeline.js index 1677a840..9169b029 100644 --- a/src/matrix/room/timeline/Timeline.js +++ b/src/matrix/room/timeline/Timeline.js @@ -450,7 +450,7 @@ export function tests() { txn.timelineEvents.tryInsert({ event: withContent(createAnnotation(messageId, "👋"), createEvent("m.reaction", reactionId, bob)), fragmentId: 1, eventIndex: 1, roomId - }); + }, new NullLogItem()); txn.timelineRelations.add(roomId, messageId, ANNOTATION_RELATION_TYPE, reactionId); await txn.complete(); // 2. setup the timeline @@ -546,7 +546,7 @@ export function tests() { txn.timelineEvents.tryInsert({ event: withContent(createAnnotation(messageId, "👋"), createEvent("m.reaction", reactionId, bob)), fragmentId: 1, eventIndex: 3, roomId - }); + }, new NullLogItem()); await txn.complete(); // 2. setup timeline const pendingEvents = new ObservableArray(); diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js index cfcebf7e..ec23db5a 100644 --- a/src/matrix/room/timeline/persistence/GapWriter.js +++ b/src/matrix/room/timeline/persistence/GapWriter.js @@ -124,7 +124,7 @@ export class GapWriter { if (updatedRelationTargetEntries) { updatedEntries.push(...updatedRelationTargetEntries); } - if (await txn.timelineEvents.tryInsert(eventStorageEntry)) { + if (await txn.timelineEvents.tryInsert(eventStorageEntry, log)) { const eventEntry = new EventEntry(eventStorageEntry, this._fragmentIdComparer); directionalAppend(entries, eventEntry, direction); } diff --git a/src/matrix/room/timeline/persistence/RelationWriter.js b/src/matrix/room/timeline/persistence/RelationWriter.js index d8619cf9..0466b3da 100644 --- a/src/matrix/room/timeline/persistence/RelationWriter.js +++ b/src/matrix/room/timeline/persistence/RelationWriter.js @@ -275,7 +275,7 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}, new NullLogItem()); const updatedEntries = await relationWriter.writeRelation(redactionEntry, txn, new NullLogItem()); await txn.complete(); @@ -300,7 +300,7 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}, new NullLogItem()); const updatedEntries = await relationWriter.writeRelation(reactionEntry, txn, new NullLogItem()); await txn.complete(); @@ -329,7 +329,7 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}, new NullLogItem()); await relationWriter.writeRelation(reaction1Entry, txn, new NullLogItem()); const updatedEntries = await relationWriter.writeRelation(reaction2Entry, txn, new NullLogItem()); await txn.complete(); @@ -358,10 +358,10 @@ export function tests() { const storage = await createMockStorage(); const txn = await storage.readWriteTxn([storage.storeNames.timelineEvents, storage.storeNames.timelineRelations]); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 3, event: myReaction, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 2, event, roomId}, new NullLogItem()); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 3, event: myReaction, roomId}, new NullLogItem()); await relationWriter.writeRelation(myReactionEntry, txn, new NullLogItem()); - txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 4, event: bobReaction, roomId}); + txn.timelineEvents.tryInsert({fragmentId: 1, eventIndex: 4, event: bobReaction, roomId}, new NullLogItem()); await relationWriter.writeRelation(bobReactionEntry, txn, new NullLogItem()); const updatedEntries = await relationWriter.writeRelation(myReactionRedactionEntry, txn, new NullLogItem()); await txn.complete(); From 498c00fe3c0b81dada2f72b4cc8c34740422ed9a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 22 Sep 2021 10:38:29 +0200 Subject: [PATCH 10/10] no need for try catch here as we already catch in getKeys --- src/matrix/storage/idb/Store.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/matrix/storage/idb/Store.ts b/src/matrix/storage/idb/Store.ts index e3d6e3f7..84e02e60 100644 --- a/src/matrix/storage/idb/Store.ts +++ b/src/matrix/storage/idb/Store.ts @@ -201,14 +201,10 @@ export class Store extends QueryTarget { } reqAsPromise(request).catch(err => { let keys : IDBKey[] | undefined = undefined; - try { - if (value) { - keys = this._getKeys(value); - } else if (key) { - keys = [key]; - } - } catch (err) { - console.error("_getKeys failed", err); + if (value) { + keys = this._getKeys(value); + } else if (key) { + keys = [key]; } this._transaction.addWriteError(err, log, operationName, keys); });