From 76381fbca15a3308ed4516b5b04da17f94244ec1 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 25 Sep 2020 16:42:41 +0200 Subject: [PATCH 01/28] open storage transactions synchronously this (almost) makes it work in some browsers that otherwise have throw a TransactionInactiveError on the first operation you try to do on a store. --- src/matrix/DeviceMessageHandler.js | 4 ++-- src/matrix/Session.js | 12 ++++++------ src/matrix/Sync.js | 12 ++++++------ src/matrix/e2ee/Account.js | 4 ++-- src/matrix/e2ee/DeviceTracker.js | 10 +++++----- src/matrix/e2ee/RoomEncryption.js | 10 +++++----- src/matrix/e2ee/megolm/Encryption.js | 2 +- src/matrix/e2ee/olm/Decryption.js | 2 +- src/matrix/e2ee/olm/Encryption.js | 6 +++--- src/matrix/room/Room.js | 14 +++++++------- src/matrix/room/RoomSummary.js | 2 +- src/matrix/room/members/load.js | 4 ++-- src/matrix/room/sending/SendQueue.js | 4 ++-- .../room/timeline/persistence/TimelineReader.js | 4 ++-- src/matrix/ssss/index.js | 2 +- src/matrix/storage/idb/Storage.js | 4 ++-- 16 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/matrix/DeviceMessageHandler.js b/src/matrix/DeviceMessageHandler.js index 854c901b..e0904133 100644 --- a/src/matrix/DeviceMessageHandler.js +++ b/src/matrix/DeviceMessageHandler.js @@ -80,7 +80,7 @@ export class DeviceMessageHandler { if (!this._olmDecryption) { return; } - const readTxn = await this._storage.readTxn([this._storage.storeNames.session]); + const readTxn = this._storage.readTxn([this._storage.storeNames.session]); const pendingEvents = await this._getPendingEvents(readTxn); if (pendingEvents.length === 0) { return; @@ -91,7 +91,7 @@ export class DeviceMessageHandler { for (const err of decryptChanges.errors) { console.warn("decryption failed for event", err, err.event); } - const txn = await this._storage.readWriteTxn([ + const txn = this._storage.readWriteTxn([ // both to remove the pending events and to modify the olm account this._storage.storeNames.session, this._storage.storeNames.olmSessions, diff --git a/src/matrix/Session.js b/src/matrix/Session.js index ef535fc7..dbb95c24 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -164,13 +164,13 @@ export class Session { } const key = await ssssKeyFromCredential(type, credential, this._storage, this._cryptoDriver, this._olm); // and create session backup, which needs to read from accountData - const readTxn = await this._storage.readTxn([ + const readTxn = this._storage.readTxn([ this._storage.storeNames.accountData, ]); await this._createSessionBackup(key, readTxn); // only after having read a secret, write the key // as we only find out if it was good if the MAC verification succeeds - const writeTxn = await this._storage.readWriteTxn([ + const writeTxn = this._storage.readWriteTxn([ this._storage.storeNames.session, ]); try { @@ -217,7 +217,7 @@ export class Session { await this._e2eeAccount.uploadKeys(this._storage); await this._deviceMessageHandler.decryptPending(this.rooms); - const txn = await this._storage.readTxn([ + const txn = this._storage.readTxn([ this._storage.storeNames.session, this._storage.storeNames.accountData, ]); @@ -231,7 +231,7 @@ export class Session { } async load() { - const txn = await this._storage.readTxn([ + const txn = this._storage.readTxn([ this._storage.storeNames.session, this._storage.storeNames.roomSummary, this._storage.storeNames.roomMembers, @@ -276,7 +276,7 @@ export class Session { async start(lastVersionResponse) { if (lastVersionResponse) { // store /versions response - const txn = await this._storage.readWriteTxn([ + const txn = this._storage.readWriteTxn([ this._storage.storeNames.session ]); txn.session.set("serverVersions", lastVersionResponse); @@ -284,7 +284,7 @@ export class Session { await txn.complete(); } - const opsTxn = await this._storage.readWriteTxn([ + const opsTxn = this._storage.readWriteTxn([ this._storage.storeNames.operations ]); const operations = await opsTxn.operations.getAll(); diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 14655b8e..2edc91f2 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -184,7 +184,7 @@ export class Sync { const roomStates = this._parseRoomsResponse(response.rooms, isInitialSync); await this._prepareRooms(roomStates); let sessionChanges; - const syncTxn = await this._openSyncTxn(); + const syncTxn = this._openSyncTxn(); try { await Promise.all(roomStates.map(async rs => { console.log(` * applying sync response to room ${rs.room.id} ...`); @@ -221,24 +221,24 @@ export class Sync { }; } - async _openPrepareSyncTxn() { + _openPrepareSyncTxn() { const storeNames = this._storage.storeNames; - return await this._storage.readTxn([ + return this._storage.readTxn([ storeNames.inboundGroupSessions, ]); } async _prepareRooms(roomStates) { - const prepareTxn = await this._openPrepareSyncTxn(); + const prepareTxn = this._openPrepareSyncTxn(); await Promise.all(roomStates.map(async rs => { rs.preparation = await rs.room.prepareSync(rs.roomResponse, rs.membership, prepareTxn); })); await Promise.all(roomStates.map(rs => rs.room.afterPrepareSync(rs.preparation))); } - async _openSyncTxn() { + _openSyncTxn() { const storeNames = this._storage.storeNames; - return await this._storage.readWriteTxn([ + return this._storage.readWriteTxn([ storeNames.session, storeNames.roomSummary, storeNames.roomState, diff --git a/src/matrix/e2ee/Account.js b/src/matrix/e2ee/Account.js index c0cfd8de..7b4a5b0d 100644 --- a/src/matrix/e2ee/Account.js +++ b/src/matrix/e2ee/Account.js @@ -45,7 +45,7 @@ export class Account { } const pickledAccount = account.pickle(pickleKey); const areDeviceKeysUploaded = false; - const txn = await storage.readWriteTxn([ + const txn = storage.readWriteTxn([ storage.storeNames.session ]); try { @@ -212,7 +212,7 @@ export class Account { } async _updateSessionStorage(storage, callback) { - const txn = await storage.readWriteTxn([ + const txn = storage.readWriteTxn([ storage.storeNames.session ]); try { diff --git a/src/matrix/e2ee/DeviceTracker.js b/src/matrix/e2ee/DeviceTracker.js index cc3a4215..bf6f1403 100644 --- a/src/matrix/e2ee/DeviceTracker.js +++ b/src/matrix/e2ee/DeviceTracker.js @@ -68,7 +68,7 @@ export class DeviceTracker { } const memberList = await room.loadMemberList(); try { - const txn = await this._storage.readWriteTxn([ + const txn = this._storage.readWriteTxn([ this._storage.storeNames.roomSummary, this._storage.storeNames.userIdentities, ]); @@ -149,7 +149,7 @@ export class DeviceTracker { }).response(); const verifiedKeysPerUser = this._filterVerifiedDeviceKeys(deviceKeyResponse["device_keys"]); - const txn = await this._storage.readWriteTxn([ + const txn = this._storage.readWriteTxn([ this._storage.storeNames.userIdentities, this._storage.storeNames.deviceIdentities, ]); @@ -252,7 +252,7 @@ export class DeviceTracker { * @return {[type]} [description] */ async devicesForTrackedRoom(roomId, hsApi) { - const txn = await this._storage.readTxn([ + const txn = this._storage.readTxn([ this._storage.storeNames.roomMembers, this._storage.storeNames.userIdentities, ]); @@ -268,7 +268,7 @@ export class DeviceTracker { } async devicesForRoomMembers(roomId, userIds, hsApi) { - const txn = await this._storage.readTxn([ + const txn = this._storage.readTxn([ this._storage.storeNames.userIdentities, ]); return await this._devicesForUserIds(roomId, userIds, txn, hsApi); @@ -298,7 +298,7 @@ export class DeviceTracker { queriedDevices = await this._queryKeys(outdatedIdentities.map(i => i.userId), hsApi); } - const deviceTxn = await this._storage.readTxn([ + const deviceTxn = this._storage.readTxn([ this._storage.storeNames.deviceIdentities, ]); const devicesPerUser = await Promise.all(upToDateIdentities.map(identity => { diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 94af64d1..6063a06c 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -183,7 +183,7 @@ export class RoomEncryption { console.warn("Got session key back from backup with different sender key, ignoring", {session, senderKey}); return; } - const txn = await this._storage.readWriteTxn([this._storage.storeNames.inboundGroupSessions]); + const txn = this._storage.readWriteTxn([this._storage.storeNames.inboundGroupSessions]); let roomKey; try { roomKey = await this._megolmDecryption.addRoomKeyFromBackup( @@ -273,7 +273,7 @@ export class RoomEncryption { const userIds = Array.from(devices.reduce((set, device) => set.add(device.userId), new Set())); // store operation for room key share, in case we don't finish here - const writeOpTxn = await this._storage.readWriteTxn([this._storage.storeNames.operations]); + const writeOpTxn = this._storage.readWriteTxn([this._storage.storeNames.operations]); let operationId; try { operationId = this._writeRoomKeyShareOperation(roomKeyMessage, userIds, writeOpTxn); @@ -290,7 +290,7 @@ export class RoomEncryption { await this._sendRoomKey(roomKeyMessage, devices, hsApi); // remove the operation - const removeOpTxn = await this._storage.readWriteTxn([this._storage.storeNames.operations]); + const removeOpTxn = this._storage.readWriteTxn([this._storage.storeNames.operations]); try { removeOpTxn.operations.remove(operationId); } catch (err) { @@ -329,7 +329,7 @@ export class RoomEncryption { this._isFlushingRoomKeyShares = true; try { if (!operations) { - const txn = await this._storage.readTxn([this._storage.storeNames.operations]); + const txn = this._storage.readTxn([this._storage.storeNames.operations]); operations = await txn.operations.getAllByTypeAndScope("share_room_key", this._room.id); } for (const operation of operations) { @@ -339,7 +339,7 @@ export class RoomEncryption { } const devices = await this._deviceTracker.devicesForRoomMembers(this._room.id, operation.userIds, hsApi); await this._sendRoomKey(operation.roomKeyMessage, devices, hsApi); - const removeTxn = await this._storage.readWriteTxn([this._storage.storeNames.operations]); + const removeTxn = this._storage.readWriteTxn([this._storage.storeNames.operations]); try { removeTxn.operations.remove(operation.id); } catch (err) { diff --git a/src/matrix/e2ee/megolm/Encryption.js b/src/matrix/e2ee/megolm/Encryption.js index cb0dddf8..a0769ba1 100644 --- a/src/matrix/e2ee/megolm/Encryption.js +++ b/src/matrix/e2ee/megolm/Encryption.js @@ -54,7 +54,7 @@ export class Encryption { async encrypt(roomId, type, content, encryptionParams) { let session = new this._olm.OutboundGroupSession(); try { - const txn = await this._storage.readWriteTxn([ + const txn = this._storage.readWriteTxn([ this._storage.storeNames.inboundGroupSessions, this._storage.storeNames.outboundGroupSessions, ]); diff --git a/src/matrix/e2ee/olm/Decryption.js b/src/matrix/e2ee/olm/Decryption.js index a193e016..7c4ef7e6 100644 --- a/src/matrix/e2ee/olm/Decryption.js +++ b/src/matrix/e2ee/olm/Decryption.js @@ -67,7 +67,7 @@ export class Decryption { return this._senderKeyLock.takeLock(senderKey); })); try { - const readSessionsTxn = await this._storage.readTxn([this._storage.storeNames.olmSessions]); + const readSessionsTxn = this._storage.readTxn([this._storage.storeNames.olmSessions]); // decrypt events for different sender keys in parallel const senderKeyOperations = await Promise.all(Array.from(eventsPerSenderKey.entries()).map(([senderKey, events]) => { return this._decryptAllForSenderKey(senderKey, events, timestamp, readSessionsTxn); diff --git a/src/matrix/e2ee/olm/Encryption.js b/src/matrix/e2ee/olm/Encryption.js index 919acc45..ff5f1a8f 100644 --- a/src/matrix/e2ee/olm/Encryption.js +++ b/src/matrix/e2ee/olm/Encryption.js @@ -98,7 +98,7 @@ export class Encryption { } async _findExistingSessions(devices) { - const txn = await this._storage.readTxn([this._storage.storeNames.olmSessions]); + const txn = this._storage.readTxn([this._storage.storeNames.olmSessions]); const sessionIdsForDevice = await Promise.all(devices.map(async device => { return await txn.olmSessions.getSessionIds(device.curve25519Key); })); @@ -213,7 +213,7 @@ export class Encryption { } async _loadSessions(encryptionTargets) { - const txn = await this._storage.readTxn([this._storage.storeNames.olmSessions]); + const txn = this._storage.readTxn([this._storage.storeNames.olmSessions]); // given we run loading in parallel, there might still be some // storage requests that will finish later once one has failed. // those should not allocate a session anymore. @@ -239,7 +239,7 @@ export class Encryption { } async _storeSessions(encryptionTargets, timestamp) { - const txn = await this._storage.readWriteTxn([this._storage.storeNames.olmSessions]); + const txn = this._storage.readWriteTxn([this._storage.storeNames.olmSessions]); try { for (const target of encryptionTargets) { const sessionEntry = createSessionEntry( diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 369836ee..f12da45b 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -82,7 +82,7 @@ export class Room extends EventEmitter { let retryEntries; if (retryEventIds) { retryEntries = []; - txn = await this._storage.readTxn(stores); + txn = this._storage.readTxn(stores); for (const eventId of retryEventIds) { const storageEntry = await txn.timelineEvents.getByEventId(this._roomId, eventId); if (storageEntry) { @@ -99,7 +99,7 @@ export class Room extends EventEmitter { // check we have not already decrypted the most recent event in the room // otherwise we know that the messages for this room key will not update the room summary if (!sinceEventKey || !sinceEventKey.equals(this._syncWriter.lastMessageKey)) { - txn = await this._storage.readTxn(stores.concat(this._storage.storeNames.timelineFragments)); + txn = this._storage.readTxn(stores.concat(this._storage.storeNames.timelineFragments)); const candidateEntries = await this._readRetryDecryptCandidateEntries(sinceEventKey, txn); retryEntries = this._roomEncryption.findAndCacheEntriesForRoomKey(roomKey, candidateEntries); } @@ -138,7 +138,7 @@ export class Room extends EventEmitter { _decryptEntries(source, entries, inboundSessionTxn = null) { const request = new DecryptionRequest(async r => { if (!inboundSessionTxn) { - inboundSessionTxn = await this._storage.readTxn([this._storage.storeNames.inboundGroupSessions]); + inboundSessionTxn = this._storage.readTxn([this._storage.storeNames.inboundGroupSessions]); } if (r.cancelled) return; const events = entries.filter(entry => { @@ -155,7 +155,7 @@ export class Room extends EventEmitter { // read to fetch devices if timeline is open stores.push(this._storage.storeNames.deviceIdentities); } - const writeTxn = await this._storage.readWriteTxn(stores); + const writeTxn = this._storage.readWriteTxn(stores); let decryption; try { decryption = await changes.write(writeTxn); @@ -387,7 +387,7 @@ export class Room extends EventEmitter { } }).response(); - const txn = await this._storage.readWriteTxn([ + const txn = this._storage.readWriteTxn([ this._storage.storeNames.pendingEvents, this._storage.storeNames.timelineEvents, this._storage.storeNames.timelineFragments, @@ -490,7 +490,7 @@ export class Room extends EventEmitter { async _getLastEventId() { const lastKey = this._syncWriter.lastMessageKey; if (lastKey) { - const txn = await this._storage.readTxn([ + const txn = this._storage.readTxn([ this._storage.storeNames.timelineEvents, ]); const eventEntry = await txn.timelineEvents.get(this._roomId, lastKey); @@ -511,7 +511,7 @@ export class Room extends EventEmitter { async clearUnread() { if (this.isUnread || this.notificationCount) { - const txn = await this._storage.readWriteTxn([ + const txn = this._storage.readWriteTxn([ this._storage.storeNames.roomSummary, ]); let data; diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index 81a68b8f..39b1d1b3 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -272,7 +272,7 @@ export class RoomSummary { if (data === this._data) { return false; } - const txn = await storage.readWriteTxn([ + const txn = storage.readWriteTxn([ storage.storeNames.roomSummary, ]); try { diff --git a/src/matrix/room/members/load.js b/src/matrix/room/members/load.js index a8648eac..aa14d2fb 100644 --- a/src/matrix/room/members/load.js +++ b/src/matrix/room/members/load.js @@ -18,7 +18,7 @@ limitations under the License. import {RoomMember} from "./RoomMember.js"; async function loadMembers({roomId, storage}) { - const txn = await storage.readTxn([ + const txn = storage.readTxn([ storage.storeNames.roomMembers, ]); const memberDatas = await txn.roomMembers.getAll(roomId); @@ -33,7 +33,7 @@ async function fetchMembers({summary, syncToken, roomId, hsApi, storage, setChan const memberResponse = await hsApi.members(roomId, {at: syncToken}).response(); - const txn = await storage.readWriteTxn([ + const txn = storage.readWriteTxn([ storage.storeNames.roomSummary, storage.storeNames.roomMembers, ]); diff --git a/src/matrix/room/sending/SendQueue.js b/src/matrix/room/sending/SendQueue.js index 52c2e7b8..e9664bc1 100644 --- a/src/matrix/room/sending/SendQueue.js +++ b/src/matrix/room/sending/SendQueue.js @@ -129,7 +129,7 @@ export class SendQueue { } async _tryUpdateEvent(pendingEvent) { - const txn = await this._storage.readWriteTxn([this._storage.storeNames.pendingEvents]); + const txn = this._storage.readWriteTxn([this._storage.storeNames.pendingEvents]); console.log("_tryUpdateEvent: got txn"); try { // pendingEvent might have been removed already here @@ -151,7 +151,7 @@ export class SendQueue { async _createAndStoreEvent(eventType, content) { console.log("_createAndStoreEvent"); - const txn = await this._storage.readWriteTxn([this._storage.storeNames.pendingEvents]); + const txn = this._storage.readWriteTxn([this._storage.storeNames.pendingEvents]); let pendingEvent; try { const pendingEventsStore = txn.pendingEvents; diff --git a/src/matrix/room/timeline/persistence/TimelineReader.js b/src/matrix/room/timeline/persistence/TimelineReader.js index 37b15574..24ad4127 100644 --- a/src/matrix/room/timeline/persistence/TimelineReader.js +++ b/src/matrix/room/timeline/persistence/TimelineReader.js @@ -108,14 +108,14 @@ export class TimelineReader { readFrom(eventKey, direction, amount) { return new ReaderRequest(async r => { - const txn = await this._openTxn(); + const txn = this._openTxn(); return await this._readFrom(eventKey, direction, amount, r, txn); }); } readFromEnd(amount) { return new ReaderRequest(async r => { - const txn = await this._openTxn(); + const txn = this._openTxn(); const liveFragment = await txn.timelineFragments.liveFragment(this._roomId); let entries; // room hasn't been synced yet diff --git a/src/matrix/ssss/index.js b/src/matrix/ssss/index.js index 286744fc..d5bcfe97 100644 --- a/src/matrix/ssss/index.js +++ b/src/matrix/ssss/index.js @@ -19,7 +19,7 @@ import {keyFromPassphrase} from "./passphrase.js"; import {keyFromRecoveryKey} from "./recoveryKey.js"; async function readDefaultKeyDescription(storage) { - const txn = await storage.readTxn([ + const txn = storage.readTxn([ storage.storeNames.accountData ]); const defaultKeyEvent = await txn.accountData.get("m.secret_storage.default_key"); diff --git a/src/matrix/storage/idb/Storage.js b/src/matrix/storage/idb/Storage.js index 9c79b92f..03c2c8ef 100644 --- a/src/matrix/storage/idb/Storage.js +++ b/src/matrix/storage/idb/Storage.js @@ -34,7 +34,7 @@ export class Storage { } } - async readTxn(storeNames) { + readTxn(storeNames) { this._validateStoreNames(storeNames); try { const txn = this._db.transaction(storeNames, "readonly"); @@ -44,7 +44,7 @@ export class Storage { } } - async readWriteTxn(storeNames) { + readWriteTxn(storeNames) { this._validateStoreNames(storeNames); try { const txn = this._db.transaction(storeNames, "readwrite"); From c68bafde5473e0a4c0928dc77cf5494df0a58bfc Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 25 Sep 2020 16:55:02 +0200 Subject: [PATCH 02/28] prototype using await to see if it makes any difference ...to the es5 one in gnome web. Alas, it does not, and I can't recreate the failure mode I see in the app when awaiting opening txns. --- prototypes/idb-promises-es6.html | 112 +++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 prototypes/idb-promises-es6.html diff --git a/prototypes/idb-promises-es6.html b/prototypes/idb-promises-es6.html new file mode 100644 index 00000000..ef7d357d --- /dev/null +++ b/prototypes/idb-promises-es6.html @@ -0,0 +1,112 @@ + + + + + + + + + + + + + From ee4c132fb449a2a5c14e40ebc5c47e61c86a81ee Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Sat, 26 Sep 2020 09:12:34 +0200 Subject: [PATCH 03/28] add todo --- src/matrix/e2ee/RoomEncryption.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 6063a06c..90b6fd23 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -251,6 +251,7 @@ export class RoomEncryption { await this._deviceTracker.trackRoom(this._room); const megolmResult = await this._megolmEncryption.encrypt(this._room.id, type, content, this._encryptionParams); if (megolmResult.roomKeyMessage) { + // TODO: should we await this?? this._shareNewRoomKey(megolmResult.roomKeyMessage, hsApi); } return { From b1f9cfd972b7f9ee4917d7eb8c33e02a960cfbd7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 09:17:03 +0200 Subject: [PATCH 04/28] cleanup storage errors a bit --- src/matrix/storage/common.js | 21 ++----------- src/matrix/storage/idb/Store.js | 19 ++++++------ src/matrix/storage/idb/error.js | 55 +++++++++++++++++++++++++++++++++ src/matrix/storage/idb/utils.js | 12 +------ 4 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 src/matrix/storage/idb/error.js diff --git a/src/matrix/storage/common.js b/src/matrix/storage/common.js index 88238a11..d96dc359 100644 --- a/src/matrix/storage/common.js +++ b/src/matrix/storage/common.js @@ -38,29 +38,12 @@ export const STORE_MAP = Object.freeze(STORE_NAMES.reduce((nameMap, name) => { }, {})); export class StorageError extends Error { - constructor(message, cause, value) { - let fullMessage = message; - if (cause) { - fullMessage += ": "; - if (typeof cause.name === "string") { - fullMessage += `(name: ${cause.name}) `; - } - if (typeof cause.code === "number") { - fullMessage += `(code: ${cause.code}) `; - } - } - if (value) { - fullMessage += `(value: ${JSON.stringify(value)}) `; - } - if (cause) { - fullMessage += cause.message; - } - super(fullMessage); + constructor(message, cause) { + super(message); if (cause) { this.errcode = cause.name; } this.cause = cause; - this.value = value; } get name() { diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index a0ca0b96..b26582bc 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -15,8 +15,7 @@ limitations under the License. */ import {QueryTarget} from "./QueryTarget.js"; -import { reqAsPromise } from "./utils.js"; -import { StorageError } from "../common.js"; +import {IDBRequestAttemptError} from "./utils.js"; class QueryTargetWrapper { constructor(qt) { @@ -43,7 +42,7 @@ class QueryTargetWrapper { try { return this._qt.openKeyCursor(...params); } catch(err) { - throw new StorageError("openKeyCursor failed", err); + throw new IDBRequestAttemptError("openKeyCursor", this._qt, err, params); } } @@ -51,7 +50,7 @@ class QueryTargetWrapper { try { return this._qt.openCursor(...params); } catch(err) { - throw new StorageError("openCursor failed", err); + throw new IDBRequestAttemptError("openCursor", this._qt, err, params); } } @@ -59,7 +58,7 @@ class QueryTargetWrapper { try { return this._qt.put(...params); } catch(err) { - throw new StorageError("put failed", err); + throw new IDBRequestAttemptError("put", this._qt, err, params); } } @@ -67,7 +66,7 @@ class QueryTargetWrapper { try { return this._qt.add(...params); } catch(err) { - throw new StorageError("add failed", err); + throw new IDBRequestAttemptError("add", this._qt, err, params); } } @@ -75,7 +74,7 @@ class QueryTargetWrapper { try { return this._qt.get(...params); } catch(err) { - throw new StorageError("get failed", err); + throw new IDBRequestAttemptError("get", this._qt, err, params); } } @@ -83,7 +82,7 @@ class QueryTargetWrapper { try { return this._qt.getKey(...params); } catch(err) { - throw new StorageError("getKey failed", err); + throw new IDBRequestAttemptError("getKey", this._qt, err, params); } } @@ -91,7 +90,7 @@ class QueryTargetWrapper { try { return this._qt.delete(...params); } catch(err) { - throw new StorageError("delete failed", err); + throw new IDBRequestAttemptError("delete", this._qt, err, params); } } @@ -99,7 +98,7 @@ class QueryTargetWrapper { try { return this._qt.index(...params); } catch(err) { - throw new StorageError("index failed", err); + throw new IDBRequestAttemptError("index", this._qt, err, params); } } } diff --git a/src/matrix/storage/idb/error.js b/src/matrix/storage/idb/error.js new file mode 100644 index 00000000..cf545663 --- /dev/null +++ b/src/matrix/storage/idb/error.js @@ -0,0 +1,55 @@ +/* +Copyright 2020 Bruno Windels +Copyright 2020 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 { StorageError } from "../common.js"; + +export class IDBError extends StorageError { + constructor(message, source, cause) { + const storeName = source?.name || ""; + const databaseName = source?.transaction?.db?.name || ""; + let fullMessage = `${message} on ${databaseName}.${storeName}`; + if (cause) { + fullMessage += ": "; + if (typeof cause.name === "string") { + fullMessage += `(name: ${cause.name}) `; + } + if (typeof cause.code === "number") { + fullMessage += `(code: ${cause.code}) `; + } + } + if (cause) { + fullMessage += cause.message; + } + super(fullMessage, cause); + this.storeName = storeName; + this.databaseName = databaseName; + } +} + +export class IDBRequestError extends IDBError { + constructor(request) { + const source = request?.source; + const cause = request.error; + super(`IDBRequest failed`, source, cause); + } +} + +export class IDBRequestAttemptError extends IDBError { + constructor(method, source, cause, params) { + super(`${method}(${params.map(p => JSON.stringify(p)).join(", ")}) failed`, source, cause); + } +} diff --git a/src/matrix/storage/idb/utils.js b/src/matrix/storage/idb/utils.js index adecba52..1d347f46 100644 --- a/src/matrix/storage/idb/utils.js +++ b/src/matrix/storage/idb/utils.js @@ -15,6 +15,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { IDBRequestError } from "./error.js"; import { StorageError } from "../common.js"; let needsSyncPromise = false; @@ -47,17 +48,6 @@ export async function checkNeedsSyncPromise() { return needsSyncPromise; } -class IDBRequestError extends StorageError { - constructor(request) { - const source = request?.source; - const storeName = source?.name || ""; - const databaseName = source?.transaction?.db?.name || ""; - super(`Failed IDBRequest on ${databaseName}.${storeName}`, request.error); - this.storeName = storeName; - this.databaseName = databaseName; - } -} - // storage keys are defined to be unsigned 32bit numbers in WebPlatform.js, which is assumed by idb export function encodeUint32(n) { const hex = n.toString(16); From 4d23529b6840879e40b123f2eb27b938473c2654 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 09:42:43 +0200 Subject: [PATCH 05/28] set promise polyfill before others just in case --- src/legacy-polyfill.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/legacy-polyfill.js b/src/legacy-polyfill.js index 0ce493e0..c3c8263b 100644 --- a/src/legacy-polyfill.js +++ b/src/legacy-polyfill.js @@ -15,6 +15,14 @@ limitations under the License. */ // polyfills needed for IE11 +import Promise from "../lib/es6-promise/index.js"; +import {checkNeedsSyncPromise} from "./matrix/storage/idb/utils.js"; + +if (typeof window.Promise === "undefined") { + window.Promise = Promise; + // TODO: should be awaited before opening any session in the picker + checkNeedsSyncPromise(); +} import "core-js/stable"; import "regenerator-runtime/runtime"; import "mdn-polyfills/Element.prototype.closest"; @@ -24,14 +32,6 @@ import "mdn-polyfills/Element.prototype.closest"; // it will also include the file supporting *all* the encodings, // weighing a good extra 500kb :-( import "text-encoding"; -import {checkNeedsSyncPromise} from "./matrix/storage/idb/utils.js"; -import Promise from "../lib/es6-promise/index.js"; - -if (typeof window.Promise === "undefined") { - window.Promise = Promise; - // TODO: should be awaited before opening any session in the picker - checkNeedsSyncPromise(); -} // TODO: contribute this to mdn-polyfills if (!Element.prototype.remove) { From 07fcf7e75b6ccd4d14f5c2985af3ef0844708100 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 09:43:25 +0200 Subject: [PATCH 06/28] also do this in try catch --- src/matrix/storage/idb/Store.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index b26582bc..08123ccb 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -35,11 +35,11 @@ class QueryTargetWrapper { } openKeyCursor(...params) { - // not supported on Edge 15 - if (!this._qt.openKeyCursor) { - return this.openCursor(...params); - } try { + // not supported on Edge 15 + if (!this._qt.openKeyCursor) { + return this.openCursor(...params); + } return this._qt.openKeyCursor(...params); } catch(err) { throw new IDBRequestAttemptError("openKeyCursor", this._qt, err, params); From c1ecaffbae88a08a0a422f8bca37b95953b4d5a0 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 09:54:51 +0200 Subject: [PATCH 07/28] fix refactor typo --- src/matrix/storage/idb/Store.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index 08123ccb..e783d128 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -15,7 +15,7 @@ limitations under the License. */ import {QueryTarget} from "./QueryTarget.js"; -import {IDBRequestAttemptError} from "./utils.js"; +import {IDBRequestAttemptError} from "./error.js"; class QueryTargetWrapper { constructor(qt) { From c529df179bfafe9ec8f2a847b768fa954b83d6c0 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 09:56:46 +0200 Subject: [PATCH 08/28] also import this --- src/matrix/storage/idb/Store.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index e783d128..905ace1b 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -16,6 +16,7 @@ limitations under the License. import {QueryTarget} from "./QueryTarget.js"; import {IDBRequestAttemptError} from "./error.js"; +import {StorageError} from "../error.js"; class QueryTargetWrapper { constructor(qt) { From 919357b474607ccd707113abcf8cc3fc2b5d1faf Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 09:57:48 +0200 Subject: [PATCH 09/28] more broken imports after refactor --- src/matrix/storage/idb/Store.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index 905ace1b..9204999d 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -16,7 +16,8 @@ limitations under the License. import {QueryTarget} from "./QueryTarget.js"; import {IDBRequestAttemptError} from "./error.js"; -import {StorageError} from "../error.js"; +import {reqAsPromise} from "./utils.js"; +import {StorageError} from "../common.js"; class QueryTargetWrapper { constructor(qt) { From 163ca1285413fdf77238f887d03bf531042c01cc Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 10:52:52 +0200 Subject: [PATCH 10/28] ignore abort error --- src/matrix/Sync.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 2edc91f2..2db9ad54 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -196,7 +196,9 @@ export class Sync { // avoid corrupting state by only // storing the sync up till the point // the exception occurred - syncTxn.abort(); + try { + syncTxn.abort(); + } catch (abortErr) { /* ignore when we can't abort */ } throw err; } try { From 7627a2bda261b70e5ff572db1b8f40adcdc8cf96 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 10:53:02 +0200 Subject: [PATCH 11/28] add comment --- src/matrix/storage/idb/Store.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index 9204999d..dbe85b2b 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -129,6 +129,8 @@ export class Store extends QueryTarget { async add(value) { try { + // this will catch both the sync error already mapped + // in the QueryTargetWrapper above, and also the async request errors, which are still DOMException's return await reqAsPromise(this._idbStore.add(value)); } catch(err) { const originalErr = err.cause; From e5b1cbbcd3b05da1807d19fff78062b85f4333b2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 11:32:49 +0200 Subject: [PATCH 12/28] prevent endless loop when restoring messages that were already sent --- src/matrix/room/sending/SendQueue.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrix/room/sending/SendQueue.js b/src/matrix/room/sending/SendQueue.js index 52c2e7b8..b256b496 100644 --- a/src/matrix/room/sending/SendQueue.js +++ b/src/matrix/room/sending/SendQueue.js @@ -48,6 +48,7 @@ export class SendQueue { const pendingEvent = this._pendingEvents.get(this._amountSent); console.log("trying to send", pendingEvent.content.body); if (pendingEvent.remoteId) { + this._amountSent += 1; continue; } if (pendingEvent.needsEncryption) { From 43d430fc9803f60493235d754e05cf4b2bbdcde5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 11:47:49 +0200 Subject: [PATCH 13/28] remove unused storage modification functions --- src/matrix/storage/idb/stores/TimelineEventStore.js | 5 ----- src/matrix/storage/idb/utils.js | 11 ----------- src/matrix/storage/memory/stores/RoomTimelineStore.js | 7 ------- 3 files changed, 23 deletions(-) diff --git a/src/matrix/storage/idb/stores/TimelineEventStore.js b/src/matrix/storage/idb/stores/TimelineEventStore.js index a3a829f9..9a5fddfc 100644 --- a/src/matrix/storage/idb/stores/TimelineEventStore.js +++ b/src/matrix/storage/idb/stores/TimelineEventStore.js @@ -253,11 +253,6 @@ export class TimelineEventStore { get(roomId, eventKey) { return this._timelineStore.get(encodeKey(roomId, eventKey.fragmentId, eventKey.eventIndex)); } - // returns the entries as well!! (or not always needed? I guess not always needed, so extra method) - removeRange(roomId, range) { - // TODO: read the entries! - return this._timelineStore.delete(range.asIDBKeyRange(roomId)); - } getByEventId(roomId, eventId) { return this._timelineStore.index("byEventId").get(encodeEventIdKey(roomId, eventId)); diff --git a/src/matrix/storage/idb/utils.js b/src/matrix/storage/idb/utils.js index 1d347f46..ac9f3911 100644 --- a/src/matrix/storage/idb/utils.js +++ b/src/matrix/storage/idb/utils.js @@ -151,17 +151,6 @@ export async function select(db, storeName, toCursor, isDone) { return await fetchResults(cursor, isDone); } -export async function updateSingletonStore(db, storeName, value) { - const tx = db.transaction([storeName], "readwrite"); - const store = tx.objectStore(storeName); - const cursor = await reqAsPromise(store.openCursor()); - if (cursor) { - return reqAsPromise(cursor.update(storeName)); - } else { - return reqAsPromise(store.add(value)); - } -} - export async function findStoreValue(db, storeName, toCursor, matchesValue) { if (!matchesValue) { matchesValue = () => true; diff --git a/src/matrix/storage/memory/stores/RoomTimelineStore.js b/src/matrix/storage/memory/stores/RoomTimelineStore.js index 5cc31173..5be20eae 100644 --- a/src/matrix/storage/memory/stores/RoomTimelineStore.js +++ b/src/matrix/storage/memory/stores/RoomTimelineStore.js @@ -234,11 +234,4 @@ export class RoomTimelineStore extends Store { const event = count ? this._timeline[startIndex] : undefined; return Promise.resolve(event); } - - removeRange(roomId, range) { - this.assertWritable(); - const {startIndex, count} = range.project(roomId); - const removedEntries = this._timeline.splice(startIndex, count); - return Promise.resolve(removedEntries); - } } From 482b5f4d223a7a92d8c5afd0427ef4ce4540f458 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 11:50:10 +0200 Subject: [PATCH 14/28] allow passing message to IDBRequestError --- src/matrix/storage/idb/error.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/matrix/storage/idb/error.js b/src/matrix/storage/idb/error.js index cf545663..05390bea 100644 --- a/src/matrix/storage/idb/error.js +++ b/src/matrix/storage/idb/error.js @@ -41,10 +41,10 @@ export class IDBError extends StorageError { } export class IDBRequestError extends IDBError { - constructor(request) { + constructor(request, message = "IDBRequest failed") { const source = request?.source; const cause = request.error; - super(`IDBRequest failed`, source, cause); + super(message, source, cause); } } From 37690cffe32bbe64788987ef261765bbd50fc5cc Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 11:50:37 +0200 Subject: [PATCH 15/28] track storage write requests internally, as we never await their promise --- src/matrix/storage/idb/Store.js | 33 ++++++------------------- src/matrix/storage/idb/Transaction.js | 35 ++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index dbe85b2b..cffd03e8 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -106,8 +106,9 @@ class QueryTargetWrapper { } export class Store extends QueryTarget { - constructor(idbStore) { + constructor(idbStore, transaction) { super(new QueryTargetWrapper(idbStore)); + this._transaction = transaction; } get _idbStore() { @@ -118,33 +119,15 @@ export class Store extends QueryTarget { return new QueryTarget(new QueryTargetWrapper(this._idbStore.index(indexName))); } - async put(value) { - try { - return await reqAsPromise(this._idbStore.put(value)); - } catch(err) { - const originalErr = err.cause; - throw new StorageError(`put on ${err.databaseName}.${err.storeName} failed`, originalErr, value); - } + put(value) { + this._transaction._addWriteRequest(this._idbStore.put(value)); } - async add(value) { - try { - // this will catch both the sync error already mapped - // in the QueryTargetWrapper above, and also the async request errors, which are still DOMException's - return await reqAsPromise(this._idbStore.add(value)); - } catch(err) { - const originalErr = err.cause; - throw new StorageError(`add on ${err.databaseName}.${err.storeName} failed`, originalErr, value); - } + add(value) { + this._transaction._addWriteRequest(this._idbStore.add(value)); } - async delete(keyOrKeyRange) { - try { - return await reqAsPromise(this._idbStore.delete(keyOrKeyRange)); - } catch(err) { - const originalErr = err.cause; - throw new StorageError(`delete on ${err.databaseName}.${err.storeName} failed`, originalErr, keyOrKeyRange); - } - + delete(keyOrKeyRange) { + this._transaction._addWriteRequest(this._idbStore.delete(keyOrKeyRange)); } } diff --git a/src/matrix/storage/idb/Transaction.js b/src/matrix/storage/idb/Transaction.js index 08eacb34..1b98515a 100644 --- a/src/matrix/storage/idb/Transaction.js +++ b/src/matrix/storage/idb/Transaction.js @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import {IDBRequestError} from "./error.js"; import {txnAsPromise} from "./utils.js"; import {StorageError} from "../common.js"; import {Store} from "./Store.js"; @@ -38,6 +39,14 @@ export class Transaction { this._txn = txn; this._allowedStoreNames = allowedStoreNames; this._stores = {}; + this._writeRequests = null; + } + + _addWriteRequest(request) { + if (!this._writeRequests) { + this._writeRequests = []; + } + this._writeRequests.push(request); } _idbStore(name) { @@ -45,7 +54,7 @@ export class Transaction { // more specific error? this is a bug, so maybe not ... throw new StorageError(`Invalid store for transaction: ${name}, only ${this._allowedStoreNames.join(", ")} are allowed.`); } - return new Store(this._txn.objectStore(name)); + return new Store(this._txn.objectStore(name), this); } _store(name, mapStore) { @@ -116,8 +125,28 @@ export class Transaction { return this._store("accountData", idbStore => new AccountDataStore(idbStore)); } - complete() { - return txnAsPromise(this._txn); + async complete() { + // check the write requests if we haven't failed yet + if (this._writeRequests) { + for (const request of this._writeRequests) { + if (request.error) { + try { + this.abort(); + } catch (err) {/* ignore abort error, although it would be useful to know if the other stuff got committed or not... */} + return Promise.reject(new IDBRequestError(request, "Write request failed")); + } + } + } + const result = await txnAsPromise(this._txn); + // check the write requests if we haven't failed yet + if (this._writeRequests) { + for (const request of this._writeRequests) { + if (request.readyState !== "done") { + return Promise.reject(new IDBRequestError(request, "Request is still pending after transaction finished")); + } + } + } + return result; } abort() { From d5a52c32d651ee516caf5a77ffeae0baf47a12f5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 29 Sep 2020 11:51:14 +0200 Subject: [PATCH 16/28] these don't return a promise anymore --- src/matrix/storage/idb/stores/PendingEventStore.js | 4 ++-- src/matrix/storage/idb/stores/SessionStore.js | 4 ++-- src/matrix/storage/idb/stores/TimelineEventStore.js | 4 ++-- src/matrix/storage/idb/stores/TimelineFragmentStore.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/matrix/storage/idb/stores/PendingEventStore.js b/src/matrix/storage/idb/stores/PendingEventStore.js index 00898311..59b79a05 100644 --- a/src/matrix/storage/idb/stores/PendingEventStore.js +++ b/src/matrix/storage/idb/stores/PendingEventStore.js @@ -58,11 +58,11 @@ export class PendingEventStore { add(pendingEvent) { pendingEvent.key = encodeKey(pendingEvent.roomId, pendingEvent.queueIndex); - return this._eventStore.add(pendingEvent); + this._eventStore.add(pendingEvent); } update(pendingEvent) { - return this._eventStore.put(pendingEvent); + this._eventStore.put(pendingEvent); } getAll() { diff --git a/src/matrix/storage/idb/stores/SessionStore.js b/src/matrix/storage/idb/stores/SessionStore.js index c6486651..3be8e875 100644 --- a/src/matrix/storage/idb/stores/SessionStore.js +++ b/src/matrix/storage/idb/stores/SessionStore.js @@ -27,11 +27,11 @@ export class SessionStore { } set(key, value) { - return this._sessionStore.put({key, value}); + this._sessionStore.put({key, value}); } add(key, value) { - return this._sessionStore.add({key, value}); + this._sessionStore.add({key, value}); } remove(key) { diff --git a/src/matrix/storage/idb/stores/TimelineEventStore.js b/src/matrix/storage/idb/stores/TimelineEventStore.js index 9a5fddfc..b7712337 100644 --- a/src/matrix/storage/idb/stores/TimelineEventStore.js +++ b/src/matrix/storage/idb/stores/TimelineEventStore.js @@ -238,7 +238,7 @@ export class TimelineEventStore { entry.key = encodeKey(entry.roomId, entry.fragmentId, entry.eventIndex); entry.eventIdKey = encodeEventIdKey(entry.roomId, entry.event.event_id); // TODO: map error? or in idb/store? - return this._timelineStore.add(entry); + this._timelineStore.add(entry); } /** Updates the entry into the store with the given [roomId, eventKey] combination. @@ -247,7 +247,7 @@ export class TimelineEventStore { * @return {Promise<>} a promise resolving to undefined if the operation was successful, or a StorageError if not. */ update(entry) { - return this._timelineStore.put(entry); + this._timelineStore.put(entry); } get(roomId, eventKey) { diff --git a/src/matrix/storage/idb/stores/TimelineFragmentStore.js b/src/matrix/storage/idb/stores/TimelineFragmentStore.js index 2a28c7bc..849e5204 100644 --- a/src/matrix/storage/idb/stores/TimelineFragmentStore.js +++ b/src/matrix/storage/idb/stores/TimelineFragmentStore.js @@ -62,11 +62,11 @@ export class TimelineFragmentStore { // like give them meaning depending on range. not for now probably ... add(fragment) { fragment.key = encodeKey(fragment.roomId, fragment.id); - return this._store.add(fragment); + this._store.add(fragment); } update(fragment) { - return this._store.put(fragment); + this._store.put(fragment); } get(roomId, fragmentId) { From ddb14f48bf4f827febe8359c1b19c600aa5aed6c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:31:08 +0200 Subject: [PATCH 17/28] we actually don't need to track write requests as errors will bubble up to the txn --- src/matrix/storage/idb/Store.js | 20 ++++++++++++----- src/matrix/storage/idb/Transaction.js | 31 +-------------------------- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index cffd03e8..460d18f9 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -16,8 +16,6 @@ limitations under the License. import {QueryTarget} from "./QueryTarget.js"; import {IDBRequestAttemptError} from "./error.js"; -import {reqAsPromise} from "./utils.js"; -import {StorageError} from "../common.js"; class QueryTargetWrapper { constructor(qt) { @@ -120,14 +118,26 @@ export class Store extends QueryTarget { } put(value) { - this._transaction._addWriteRequest(this._idbStore.put(value)); + // 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. + // + // Perhaps at some later point, we will want to handle an error (like ConstraintError) for + // individual write requests. In that case, we should add a method that returns a promise (e.g. putAndObserve) + // and call preventDefault on the event to prevent it from aborting the transaction + // + // 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); } add(value) { - this._transaction._addWriteRequest(this._idbStore.add(value)); + // ok to not monitor result of request, see comment in `put`. + this._idbStore.add(value); } delete(keyOrKeyRange) { - this._transaction._addWriteRequest(this._idbStore.delete(keyOrKeyRange)); + // ok to not monitor result of request, see comment in `put`. + this._idbStore.delete(keyOrKeyRange); } } diff --git a/src/matrix/storage/idb/Transaction.js b/src/matrix/storage/idb/Transaction.js index 1b98515a..88e15186 100644 --- a/src/matrix/storage/idb/Transaction.js +++ b/src/matrix/storage/idb/Transaction.js @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {IDBRequestError} from "./error.js"; import {txnAsPromise} from "./utils.js"; import {StorageError} from "../common.js"; import {Store} from "./Store.js"; @@ -39,14 +38,6 @@ export class Transaction { this._txn = txn; this._allowedStoreNames = allowedStoreNames; this._stores = {}; - this._writeRequests = null; - } - - _addWriteRequest(request) { - if (!this._writeRequests) { - this._writeRequests = []; - } - this._writeRequests.push(request); } _idbStore(name) { @@ -126,27 +117,7 @@ export class Transaction { } async complete() { - // check the write requests if we haven't failed yet - if (this._writeRequests) { - for (const request of this._writeRequests) { - if (request.error) { - try { - this.abort(); - } catch (err) {/* ignore abort error, although it would be useful to know if the other stuff got committed or not... */} - return Promise.reject(new IDBRequestError(request, "Write request failed")); - } - } - } - const result = await txnAsPromise(this._txn); - // check the write requests if we haven't failed yet - if (this._writeRequests) { - for (const request of this._writeRequests) { - if (request.readyState !== "done") { - return Promise.reject(new IDBRequestError(request, "Request is still pending after transaction finished")); - } - } - } - return result; + return txnAsPromise(this._txn); } abort() { From 93a7f9959edfe9c11f09916ea59210b495c9b0f7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:31:38 +0200 Subject: [PATCH 18/28] Safari doesn't like the prepare txn still open when opening the sync txn Waiting for it to close magically solves the TransactionInactiveError we were seeing on some incremental sync request when reading from roomMembers. Still unsure what this is about, and if we should wait for all read txns to close or not. --- src/matrix/Sync.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 2db9ad54..ff41f9c1 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -235,6 +235,7 @@ export class Sync { await Promise.all(roomStates.map(async rs => { rs.preparation = await rs.room.prepareSync(rs.roomResponse, rs.membership, prepareTxn); })); + await prepareTxn.complete(); await Promise.all(roomStates.map(rs => rs.room.afterPrepareSync(rs.preparation))); } From 3c7125bb88cc61f91955152dc4ce85867bd10bdc Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:35:33 +0200 Subject: [PATCH 19/28] add (optional) logging for idb requests --- src/matrix/storage/idb/Store.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index 460d18f9..11112ba7 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -17,6 +17,14 @@ limitations under the License. import {QueryTarget} from "./QueryTarget.js"; import {IDBRequestAttemptError} from "./error.js"; +const LOG_REQUESTS = false; + +function logRequest(method, params, source) { + const storeName = source?.name; + const databaseName = source?.transaction?.db?.name; + console.info(`${databaseName}.${storeName}.${method}(${params.map(p => JSON.stringify(p)).join(", ")})`); +} + class QueryTargetWrapper { constructor(qt) { this._qt = qt; @@ -38,8 +46,10 @@ class QueryTargetWrapper { try { // not supported on Edge 15 if (!this._qt.openKeyCursor) { + LOG_REQUESTS && logRequest("openCursor", params, this._qt); return this.openCursor(...params); } + LOG_REQUESTS && logRequest("openKeyCursor", params, this._qt); return this._qt.openKeyCursor(...params); } catch(err) { throw new IDBRequestAttemptError("openKeyCursor", this._qt, err, params); @@ -48,6 +58,7 @@ class QueryTargetWrapper { openCursor(...params) { try { + LOG_REQUESTS && logRequest("openCursor", params, this._qt); return this._qt.openCursor(...params); } catch(err) { throw new IDBRequestAttemptError("openCursor", this._qt, err, params); @@ -56,6 +67,7 @@ class QueryTargetWrapper { put(...params) { try { + LOG_REQUESTS && logRequest("put", params, this._qt); return this._qt.put(...params); } catch(err) { throw new IDBRequestAttemptError("put", this._qt, err, params); @@ -64,6 +76,7 @@ class QueryTargetWrapper { add(...params) { try { + LOG_REQUESTS && logRequest("add", params, this._qt); return this._qt.add(...params); } catch(err) { throw new IDBRequestAttemptError("add", this._qt, err, params); @@ -72,6 +85,7 @@ class QueryTargetWrapper { get(...params) { try { + LOG_REQUESTS && logRequest("get", params, this._qt); return this._qt.get(...params); } catch(err) { throw new IDBRequestAttemptError("get", this._qt, err, params); @@ -80,6 +94,7 @@ class QueryTargetWrapper { getKey(...params) { try { + LOG_REQUESTS && logRequest("getKey", params, this._qt); return this._qt.getKey(...params); } catch(err) { throw new IDBRequestAttemptError("getKey", this._qt, err, params); @@ -88,6 +103,7 @@ class QueryTargetWrapper { delete(...params) { try { + LOG_REQUESTS && logRequest("delete", params, this._qt); return this._qt.delete(...params); } catch(err) { throw new IDBRequestAttemptError("delete", this._qt, err, params); From d5a6a4d350a568354c7ad70db1d935a0e5236ee0 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:35:46 +0200 Subject: [PATCH 20/28] todo comment --- src/matrix/storage/idb/Store.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index 11112ba7..22851af0 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -114,6 +114,7 @@ class QueryTargetWrapper { try { return this._qt.index(...params); } catch(err) { + // TODO: map to different error? this is not a request throw new IDBRequestAttemptError("index", this._qt, err, params); } } From 1117c77d05f00501a3560a72cf265c7b09be3732 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:36:00 +0200 Subject: [PATCH 21/28] note for future optimisation --- src/matrix/room/timeline/persistence/SyncWriter.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/matrix/room/timeline/persistence/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index 7f2275b1..231f7295 100644 --- a/src/matrix/room/timeline/persistence/SyncWriter.js +++ b/src/matrix/room/timeline/persistence/SyncWriter.js @@ -104,6 +104,8 @@ export class SyncWriter { const memberChange = new MemberChange(this._roomId, event); const {member} = memberChange; if (member) { + // TODO: can we avoid writing redundant members here by checking + // if this is not a limited sync and the state is not in the timeline? txn.roomMembers.set(member.serialize()); return memberChange; } From 300529b7c571d430a3903a10b71c820de0e29734 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:36:22 +0200 Subject: [PATCH 22/28] write sync token first in case we get a TransactionInactiveError, we have at least written the sync token and won't repeat the same sync request --- src/matrix/Session.js | 11 ++++++----- src/matrix/Sync.js | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index dbb95c24..3c2f0e8c 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -341,17 +341,18 @@ export class Session { deviceMessageDecryptionPending: false }; const syncToken = syncResponse.next_batch; - const deviceOneTimeKeysCount = syncResponse.device_one_time_keys_count; - - if (this._e2eeAccount && deviceOneTimeKeysCount) { - changes.e2eeAccountChanges = this._e2eeAccount.writeSync(deviceOneTimeKeysCount, txn); - } if (syncToken !== this.syncToken) { const syncInfo = {token: syncToken, filterId: syncFilterId}; // don't modify `this` because transaction might still fail txn.session.set("sync", syncInfo); changes.syncInfo = syncInfo; } + + const deviceOneTimeKeysCount = syncResponse.device_one_time_keys_count; + if (this._e2eeAccount && deviceOneTimeKeysCount) { + changes.e2eeAccountChanges = this._e2eeAccount.writeSync(deviceOneTimeKeysCount, txn); + } + if (this._deviceTracker) { const deviceLists = syncResponse.device_lists; if (deviceLists) { diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index ff41f9c1..889be6ad 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -186,12 +186,12 @@ export class Sync { let sessionChanges; const syncTxn = this._openSyncTxn(); try { + sessionChanges = await this._session.writeSync(response, syncFilterId, syncTxn); await Promise.all(roomStates.map(async rs => { console.log(` * applying sync response to room ${rs.room.id} ...`); rs.changes = await rs.room.writeSync( rs.roomResponse, isInitialSync, rs.preparation, syncTxn); })); - sessionChanges = await this._session.writeSync(response, syncFilterId, syncTxn); } catch(err) { // avoid corrupting state by only // storing the sync up till the point From f402e8c6c4c071eaa3c71d6956bf5c66c7e9073e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:39:23 +0200 Subject: [PATCH 23/28] typo/thinko in docs --- src/matrix/Sync.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 889be6ad..2d9780b1 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -253,7 +253,7 @@ export class Sync { storeNames.groupSessionDecryptions, storeNames.deviceIdentities, // to discard outbound session when somebody leaves a room - // and to create room key messages when somebody leaves + // and to create room key messages when somebody joins storeNames.outboundGroupSessions, storeNames.operations, storeNames.accountData, From b08b7e521a331b44283e3b5132d5e73cc17a5b45 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:41:31 +0200 Subject: [PATCH 24/28] (failed) attempt at a repro case for the safari TransactionInactiveError --- prototypes/idb-promises-safari.html | 169 ++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 prototypes/idb-promises-safari.html diff --git a/prototypes/idb-promises-safari.html b/prototypes/idb-promises-safari.html new file mode 100644 index 00000000..00f8174f --- /dev/null +++ b/prototypes/idb-promises-safari.html @@ -0,0 +1,169 @@ + + + + + + + + + + + From cb7da2ba4a77ac2103472948764296020467390d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:45:09 +0200 Subject: [PATCH 25/28] dont need this anymore --- src/matrix/storage/idb/Transaction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/storage/idb/Transaction.js b/src/matrix/storage/idb/Transaction.js index 88e15186..bfd50a5e 100644 --- a/src/matrix/storage/idb/Transaction.js +++ b/src/matrix/storage/idb/Transaction.js @@ -45,7 +45,7 @@ export class Transaction { // more specific error? this is a bug, so maybe not ... throw new StorageError(`Invalid store for transaction: ${name}, only ${this._allowedStoreNames.join(", ")} are allowed.`); } - return new Store(this._txn.objectStore(name), this); + return new Store(this._txn.objectStore(name)); } _store(name, mapStore) { From 9a4d47820d3903985233edd8b78a74efe15ffb07 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 14:46:30 +0200 Subject: [PATCH 26/28] change this back as well --- src/matrix/storage/idb/Transaction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/storage/idb/Transaction.js b/src/matrix/storage/idb/Transaction.js index bfd50a5e..08eacb34 100644 --- a/src/matrix/storage/idb/Transaction.js +++ b/src/matrix/storage/idb/Transaction.js @@ -116,7 +116,7 @@ export class Transaction { return this._store("accountData", idbStore => new AccountDataStore(idbStore)); } - async complete() { + complete() { return txnAsPromise(this._txn); } From c1df371a141a47dff15855549023a803015dfe01 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 16:14:06 +0200 Subject: [PATCH 27/28] add some documentation for our idb investigations --- doc/INDEXEDDB.md | 73 ++++++++++++++++++++++++++++++++++++++++++++++ src/matrix/Sync.js | 1 + 2 files changed, 74 insertions(+) create mode 100644 doc/INDEXEDDB.md diff --git a/doc/INDEXEDDB.md b/doc/INDEXEDDB.md new file mode 100644 index 00000000..ebfa6580 --- /dev/null +++ b/doc/INDEXEDDB.md @@ -0,0 +1,73 @@ +## Promises, async/await and indexedDB + +Doesn't indexedDB close your transaction if you don't queue more requests from an idb event handler? +So wouldn't that mean that you can't use promises and async/await when using idb? + +It used to be like this, and for IE11 on Win7 (not on Windows 10 strangely enough), it still is like this. +Here we manually flush the promise queue synchronously at the end of an idb event handler. + +In modern browsers, indexedDB transactions should only be closed after flushing the microtask queue of the event loop, +which is where promises run. + +Keep in mind that indexedDB events, just like any other DOM event, are fired as macro tasks. +Promises queue micro tasks, of which the queue is drained before proceeding to the next macro task. +This also means that if a transaction is completed, you will only receive the event once you are ready to process the next macro tasks. +That doesn't prevent any placed request from throwing TransactionInactiveError though. + +## TransactionInactiveError in Safari + +Safari doesn't fully follow the rules above, in that if you open a transaction, +you need to "use" (not sure if this means getting a store or actually placing a request) it straight away, +without waiting for any *micro*tasks. See comments about Safari at https://github.com/dfahlander/Dexie.js/issues/317#issue-178349994. + +Another failure mode perceived in Hydrogen on Safari is that when the (readonly) prepareTxn in sync wasn't awaited to be completed before opening and using the syncTxn. +I haven't found any documentation online about this at all. Awaiting prepareTxn.complete() fixed the issue below. It's strange though the put does not fail. + +What is happening below is: + - in the sync loop: + - we first open a readonly txn on inboundGroupSessions, which we don't use in the example below + - we then open a readwrite txn on session, ... (does not overlap with first txn) + - first the first incremental sync on a room (!YxKeAxtNcDZDrGgaMF:matrix.org) it seems to work well + - on a second incremental sync for that same room, the first get throws TransactionInactiveError for some reason. + - the put in the second incremental sync somehow did not throw. + +So it looks like safari doesn't like (some) transactions still being active while a second one is being openened, even with non-overlapping stores. +For now I haven't awaited every read txn in the app, as this was the only place it fails, but if this pops up again in safari, we might have to do that. + +Keep in mind that the `txn ... inactive` logs are only logged when the "complete" or "abort" events are processed, +which happens in a macro task, as opposed to all of our promises, which run in a micro task. +So the transaction is likely to have closed before it appears in the logs. + +``` +[Log] txn 4504181722375185 active on inboundGroupSessions +[Log] txn 861052256474256 active on session, roomSummary, roomState, roomMembers, timelineEvents, timelineFragments, pendingEvents, userIdentities, groupSessionDecryptions, deviceIdentities, outboundGroupSessions, operations, accountData +[Info] hydrogen_session_5286139994689036.session.put({"key":"sync","value":{"token":"s1572540047_757284957_7660701_602588550_435736037_1567300_101589125_347651623_132704","filterId":"2"}}) +[Info] hydrogen_session_5286139994689036.userIdentities.get("@bwindels:matrix.org") +[Log] txn 4504181722375185 inactive +[Log] * applying sync response to room !YxKeAxtNcDZDrGgaMF:matrix.org ... +[Info] hydrogen_session_5286139994689036.roomMembers.put({"roomId":"!YxKeAxtNcDZDrGgaMF:matrix.org","userId":"@bwindels:matrix.org","membership":"join","avatarUrl":"mxc://matrix.org/aerWVfICBMcyFcEyREcivLuI","displayName":"Bruno","key":"!YxKeAxtNcDZDrGgaMF:matrix.org|@bwindels:matrix.org"}) +[Info] hydrogen_session_5286139994689036.roomMembers.get("!YxKeAxtNcDZDrGgaMF:matrix.org|@bwindels:matrix.org") +[Info] hydrogen_session_5286139994689036.timelineEvents.add({"fragmentId":0,"eventIndex":2147483658,"roomId":"!YxKeAxtNcDZDrGgaMF:matrix.org","event":{"content":{"body":"haha","msgtype":"m.text"},"origin_server_ts":1601457573756,"sender":"@bwindels:matrix.org","type":"m.room.message","unsigned":{"age":8360},"event_id":"$eD9z73-lCpXBVby5_fKqzRZzMVHiPzKbE_RSZzqRKx0"},"displayName":"Bruno","avatarUrl":"mxc://matrix.org/aerWVfICBMcyFcEyREcivLuI","key":"!YxKeAxtNcDZDrGgaMF:matrix.org|00000000|8000000a","eventIdKey":"!YxKeAxtNcDZDrGgaMF:matrix.org|$eD9z73-lCpXBVby5_fKqzRZzMVHiPzKbE_RSZzqRKx0"}) +[Info] hydrogen_session_5286139994689036.roomSummary.put({"roomId":"!YxKeAxtNcDZDrGgaMF:matrix.org","name":"!!!test8!!!!!!","lastMessageBody":"haha","lastMessageTimestamp":1601457573756,"isUnread":true,"encryption":null,"lastDecryptedEventKey":null,"isDirectMessage":false,"membership":"join","inviteCount":0,"joinCount":2,"heroes":null,"hasFetchedMembers":false,"isTrackingMembers":false,"avatarUrl":null,"notificationCount":5,"highlightCount":0,"tags":{"m.lowpriority":{}}}) +[Log] txn 861052256474256 inactive +[Info] syncTxn committed!! + +... two more unrelated sync responses ... + +[Log] starting sync request with since s1572540191_757284957_7660742_602588567_435736063_1567300_101589126_347651632_132704 ... +[Log] txn 8104296957004707 active on inboundGroupSessions +[Log] txn 2233038992157489 active on session, roomSummary, roomState, roomMembers, timelineEvents, timelineFragments, pendingEvents, userIdentities, groupSessionDecryptions, deviceIdentities, outboundGroupSessions, operations, accountData +[Info] hydrogen_session_5286139994689036.session.put({"key":"sync","value":{"token":"s1572540223_757284957_7660782_602588579_435736078_1567300_101589130_347651633_132704","filterId":"2"}}) +[Log] * applying sync response to room !YxKeAxtNcDZDrGgaMF:matrix.org ... +[Info] hydrogen_session_5286139994689036.roomMembers.get("!YxKeAxtNcDZDrGgaMF:matrix.org|@bwindels:matrix.org") +[Warning] stopping sync because of error +[Error] StorageError: get("!YxKeAxtNcDZDrGgaMF:matrix.org|@bwindels:matrix.org") failed on txn with stores accountData, deviceIdentities, groupSessionDecryptions, operations, outboundGroupSessions, pendingEvents, roomMembers, roomState, roomSummary, session, timelineEvents, timelineFragments, userIdentities on hydrogen_session_5286139994689036.roomMembers: (name: TransactionInactiveError) (code: 0) Failed to execute 'get' on 'IDBObjectStore': The transaction is inactive or finished. + (anonymous function) + asyncFunctionResume + (anonymous function) + promiseReactionJobWithoutPromise + promiseReactionJob +[Log] newStatus – "SyncError" +[Log] txn 8104296957004707 inactive +[Log] txn 2233038992157489 inactive +``` diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 2d9780b1..81c9d649 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -235,6 +235,7 @@ export class Sync { await Promise.all(roomStates.map(async rs => { rs.preparation = await rs.room.prepareSync(rs.roomResponse, rs.membership, prepareTxn); })); + // This is needed for safari to not throw TransactionInactiveErrors on the syncTxn. See docs/INDEXEDDB.md await prepareTxn.complete(); await Promise.all(roomStates.map(rs => rs.room.afterPrepareSync(rs.preparation))); } From bebdaad7d48c3ec1ced3ac918bf8effac471db63 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 1 Oct 2020 16:23:15 +0200 Subject: [PATCH 28/28] log when we can't abort --- src/matrix/Sync.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index 81c9d649..3bdf7d9d 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -198,7 +198,9 @@ export class Sync { // the exception occurred try { syncTxn.abort(); - } catch (abortErr) { /* ignore when we can't abort */ } + } catch (abortErr) { + console.error("Could not abort sync transaction, the sync response was probably only partially written and may have put storage in a inconsistent state.", abortErr); + } throw err; } try {