From c9ee5a5db27b513cf2c8d0e846107bf5cc4ac2a4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 21 Sep 2020 17:57:01 +0200 Subject: [PATCH] stay in catchup mode as long as there are device messages this implements https://github.com/vector-im/element-web/issues/2782 it also implements 0 timeout for catchup, getting rid of the catching up with your convo banner for 30s upon reconnection. --- src/matrix/Session.js | 14 ++++++++++---- src/matrix/Sync.js | 30 ++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index efdc5904..2bd93136 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -389,11 +389,17 @@ export class Session { } } - async afterSyncCompleted() { - const needsToUploadOTKs = await this._e2eeAccount.generateOTKsIfNeeded(this._storage); + async afterSyncCompleted(isCatchupSync) { const promises = [this._deviceMessageHandler.decryptPending(this.rooms)]; - if (needsToUploadOTKs) { - promises.push(this._e2eeAccount.uploadKeys(this._storage)); + // we don't start uploading one-time keys until we've caught up with + // to-device messages, to help us avoid throwing away one-time-keys that we + // are about to receive messages for + // (https://github.com/vector-im/riot-web/issues/2782). + if (!isCatchupSync) { + const needsToUploadOTKs = await this._e2eeAccount.generateOTKsIfNeeded(this._storage); + if (needsToUploadOTKs) { + promises.push(this._e2eeAccount.uploadKeys(this._storage)); + } } // run key upload and decryption in parallel await Promise.all(promises); diff --git a/src/matrix/Sync.js b/src/matrix/Sync.js index cffa8682..dc169deb 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -98,11 +98,27 @@ export class Sync { let roomStates; try { console.log(`starting sync request with since ${syncToken} ...`); - const timeout = syncToken ? INCREMENTAL_TIMEOUT : undefined; + // unless we are happily syncing already, we want the server to return + // as quickly as possible, even if there are no events queued. This + // serves two purposes: + // + // * When the connection dies, we want to know asap when it comes back, + // so that we can hide the error from the user. (We don't want to + // have to wait for an event or a timeout). + // + // * We want to know if the server has any to_device messages queued up + // for us. We do that by calling it with a zero timeout until it + // doesn't give us any more to_device messages. + const timeout = this._status.get() === SyncStatus.Syncing ? INCREMENTAL_TIMEOUT : 0; const syncResult = await this._syncRequest(syncToken, timeout); syncToken = syncResult.syncToken; roomStates = syncResult.roomStates; - this._status.set(SyncStatus.Syncing); + // initial sync or catchup sync + if (this._status.get() !== SyncStatus.Syncing && syncResult.hadToDeviceMessages) { + this._status.set(SyncStatus.CatchupSync); + } else { + this._status.set(SyncStatus.Syncing); + } } catch (err) { if (!(err instanceof AbortError)) { console.warn("stopping sync because of error"); @@ -118,9 +134,10 @@ export class Sync { } async _runAfterSyncCompleted(roomStates) { + const isCatchupSync = this._status.get() === SyncStatus.CatchupSync; const sessionPromise = (async () => { try { - await this._session.afterSyncCompleted(); + await this._session.afterSyncCompleted(isCatchupSync); } catch (err) { console.error("error during session afterSyncCompleted, continuing", err.stack); } @@ -186,7 +203,12 @@ export class Sync { rs.room.afterSync(rs.changes); } - return {syncToken, roomStates}; + const toDeviceEvents = response.to_device?.events; + return { + syncToken, + roomStates, + hadToDeviceMessages: Array.isArray(toDeviceEvents) && toDeviceEvents.length > 0, + }; } async _openPrepareSyncTxn() {