diff --git a/doc/INDEXEDDB.md b/doc/INDEXEDDB.md new file mode 100644 index 0000000000..ebfa65800e --- /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/prototypes/idb-promises-es6.html b/prototypes/idb-promises-es6.html new file mode 100644 index 0000000000..ef7d357d15 --- /dev/null +++ b/prototypes/idb-promises-es6.html @@ -0,0 +1,112 @@ + + +
+ + + + + + + + + + diff --git a/prototypes/idb-promises-safari.html b/prototypes/idb-promises-safari.html new file mode 100644 index 0000000000..00f8174f7d --- /dev/null +++ b/prototypes/idb-promises-safari.html @@ -0,0 +1,169 @@ + + + + + + + + + + + diff --git a/src/legacy-polyfill.js b/src/legacy-polyfill.js index 0ce493e0c5..c3c8263b96 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) { diff --git a/src/matrix/DeviceMessageHandler.js b/src/matrix/DeviceMessageHandler.js index 854c901b08..e09041333a 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 ef535fc7b3..3c2f0e8c3b 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(); @@ -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 14655b8e80..3bdf7d9d70 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -184,19 +184,23 @@ 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 { + 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 // the exception occurred - syncTxn.abort(); + try { + syncTxn.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 { @@ -221,24 +225,26 @@ 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); })); + // 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))); } - async _openSyncTxn() { + _openSyncTxn() { const storeNames = this._storage.storeNames; - return await this._storage.readWriteTxn([ + return this._storage.readWriteTxn([ storeNames.session, storeNames.roomSummary, storeNames.roomState, @@ -250,7 +256,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, diff --git a/src/matrix/e2ee/Account.js b/src/matrix/e2ee/Account.js index c0cfd8de1c..7b4a5b0dd1 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 cc3a421517..bf6f1403b6 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 94af64d1bf..90b6fd23f7 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( @@ -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 { @@ -273,7 +274,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 +291,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 +330,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 +340,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 cb0dddf8fc..a0769ba18c 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 a193e0163a..7c4ef7e6b0 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 919acc4507..ff5f1a8ffa 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 369836ee53..f12da45b25 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 81a68b8feb..39b1d1b333 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 a8648eac75..aa14d2fbd2 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 52c2e7b84d..eba5fcf3e2 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) { @@ -129,7 +130,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 +152,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/SyncWriter.js b/src/matrix/room/timeline/persistence/SyncWriter.js index 7f2275b181..231f7295c0 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; } diff --git a/src/matrix/room/timeline/persistence/TimelineReader.js b/src/matrix/room/timeline/persistence/TimelineReader.js index 37b15574f0..24ad4127e2 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 286744fc20..d5bcfe9781 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/common.js b/src/matrix/storage/common.js index 88238a111d..d96dc3597c 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/Storage.js b/src/matrix/storage/idb/Storage.js index 9c79b92f05..03c2c8ef61 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"); diff --git a/src/matrix/storage/idb/Store.js b/src/matrix/storage/idb/Store.js index a0ca0b9686..22851af04d 100644 --- a/src/matrix/storage/idb/Store.js +++ b/src/matrix/storage/idb/Store.js @@ -15,8 +15,15 @@ limitations under the License. */ import {QueryTarget} from "./QueryTarget.js"; -import { reqAsPromise } from "./utils.js"; -import { StorageError } from "../common.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) { @@ -36,62 +43,70 @@ 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) { + 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 StorageError("openKeyCursor failed", err); + throw new IDBRequestAttemptError("openKeyCursor", this._qt, err, params); } } openCursor(...params) { try { + LOG_REQUESTS && logRequest("openCursor", params, this._qt); return this._qt.openCursor(...params); } catch(err) { - throw new StorageError("openCursor failed", err); + throw new IDBRequestAttemptError("openCursor", this._qt, err, params); } } put(...params) { try { + LOG_REQUESTS && logRequest("put", params, this._qt); return this._qt.put(...params); } catch(err) { - throw new StorageError("put failed", err); + throw new IDBRequestAttemptError("put", this._qt, err, params); } } add(...params) { try { + LOG_REQUESTS && logRequest("add", params, this._qt); return this._qt.add(...params); } catch(err) { - throw new StorageError("add failed", err); + throw new IDBRequestAttemptError("add", this._qt, err, params); } } get(...params) { try { + LOG_REQUESTS && logRequest("get", params, this._qt); return this._qt.get(...params); } catch(err) { - throw new StorageError("get failed", err); + throw new IDBRequestAttemptError("get", this._qt, err, params); } } getKey(...params) { try { + LOG_REQUESTS && logRequest("getKey", params, this._qt); return this._qt.getKey(...params); } catch(err) { - throw new StorageError("getKey failed", err); + throw new IDBRequestAttemptError("getKey", this._qt, err, params); } } delete(...params) { try { + LOG_REQUESTS && logRequest("delete", params, this._qt); return this._qt.delete(...params); } catch(err) { - throw new StorageError("delete failed", err); + throw new IDBRequestAttemptError("delete", this._qt, err, params); } } @@ -99,14 +114,16 @@ class QueryTargetWrapper { try { return this._qt.index(...params); } catch(err) { - throw new StorageError("index failed", err); + // TODO: map to different error? this is not a request + throw new IDBRequestAttemptError("index", this._qt, err, params); } } } export class Store extends QueryTarget { - constructor(idbStore) { + constructor(idbStore, transaction) { super(new QueryTargetWrapper(idbStore)); + this._transaction = transaction; } get _idbStore() { @@ -117,31 +134,27 @@ 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) { + // 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); } - async add(value) { - try { - 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) { + // ok to not monitor result of request, see comment in `put`. + 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) { + // ok to not monitor result of request, see comment in `put`. + this._idbStore.delete(keyOrKeyRange); } } diff --git a/src/matrix/storage/idb/error.js b/src/matrix/storage/idb/error.js new file mode 100644 index 0000000000..05390beab2 --- /dev/null +++ b/src/matrix/storage/idb/error.js @@ -0,0 +1,55 @@ +/* +Copyright 2020 Bruno Windels