From 0a83bf11764fd42b7025f49f0a29bf4ba4e1c204 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 09:30:13 +0200 Subject: [PATCH 1/8] more notes for legacy css marker --- src/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.js b/src/main.js index 5ad97cda..2e7c80df 100644 --- a/src/main.js +++ b/src/main.js @@ -81,7 +81,7 @@ async function loadOlmWorker(paths) { // see https://github.com/rollup/plugins/tree/master/packages/multi-entry export async function main(container, paths, legacyExtras) { try { - // TODO: add .legacy to body in (legacy)platform.createAndMountRootView; and use body:not(.legacy) if needed for modern stuff + // TODO: add .legacy to .hydrogen (container) in (legacy)platform.createAndMountRootView; and use .hydrogen:not(.legacy) if needed for modern stuff const isIE11 = !!window.MSInputMethodContext && !!document.documentMode; if (isIE11) { document.body.className += " ie11"; @@ -104,7 +104,7 @@ export async function main(container, paths, legacyExtras) { } else { request = xhrRequest; } - const sessionInfoStorage = new SessionInfoStorage("brawl_sessions_v1"); + const sessionInfoStorage = new SessionInfoStorage("hydrogen_sessions_v1"); const storageFactory = new StorageFactory(); const olmPromise = loadOlm(paths.olm); From 6cd227b82dfc88d74d55ad21e7f8ab469d8c3148 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 09:30:25 +0200 Subject: [PATCH 2/8] only prompt after waiting 10s for sync UTD --- src/matrix/e2ee/RoomEncryption.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/matrix/e2ee/RoomEncryption.js b/src/matrix/e2ee/RoomEncryption.js index 87c64cbe..f0446f98 100644 --- a/src/matrix/e2ee/RoomEncryption.js +++ b/src/matrix/e2ee/RoomEncryption.js @@ -159,10 +159,6 @@ export class RoomEncryption { } async _requestMissingSessionFromBackup(senderKey, sessionId, source) { - if (!this._sessionBackup) { - this._notifyMissingMegolmSession(); - return; - } // if the message came from sync, wait 10s to see if the room key arrives, // and only after that proceed to request from backup if (source === DecryptionSource.Sync) { @@ -171,6 +167,11 @@ export class RoomEncryption { return; } } + // show prompt to enable secret storage + if (!this._sessionBackup) { + this._notifyMissingMegolmSession(); + return; + } try { const session = await this._sessionBackup.getSession(this._room.id, sessionId); From d7c25e3106ea655411828ad387981a76ab9a4a11 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 13:40:38 +0200 Subject: [PATCH 3/8] move MediaRepository out of HomeServerApi so HomeServerApi becomes easier to wrap, only having methods that return a RequestResult. --- src/matrix/Session.js | 6 ++-- src/matrix/SessionContainer.js | 14 ++++++-- src/matrix/net/HomeServerApi.js | 60 ++----------------------------- src/matrix/net/MediaRepository.js | 52 +++++++++++++++++++++++++++ src/matrix/net/common.js | 28 +++++++++++++++ src/matrix/room/Room.js | 5 +-- 6 files changed, 101 insertions(+), 64 deletions(-) create mode 100644 src/matrix/net/MediaRepository.js create mode 100644 src/matrix/net/common.js diff --git a/src/matrix/Session.js b/src/matrix/Session.js index 2bd93136..c2c2e20a 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -42,10 +42,11 @@ const PICKLE_KEY = "DEFAULT_KEY"; export class Session { // sessionInfo contains deviceId, userId and homeServer - constructor({clock, storage, hsApi, sessionInfo, olm, olmWorker, cryptoDriver}) { + constructor({clock, storage, hsApi, sessionInfo, olm, olmWorker, cryptoDriver, mediaRepository}) { this._clock = clock; this._storage = storage; this._hsApi = hsApi; + this._mediaRepository = mediaRepository; this._syncInfo = null; this._sessionInfo = sessionInfo; this._rooms = new ObservableMap(); @@ -332,7 +333,8 @@ export class Session { emitCollectionChange: this._roomUpdateCallback, hsApi: this._hsApi, sendScheduler: this._sendScheduler, - pendingEvents, +._hsApi, + mediaRepository: this._mediaRep pendingEvents, user: this._user, createRoomEncryption: this._createRoomEncryption, clock: this._clock diff --git a/src/matrix/SessionContainer.js b/src/matrix/SessionContainer.js index 0de70400..be2c0c72 100644 --- a/src/matrix/SessionContainer.js +++ b/src/matrix/SessionContainer.js @@ -19,6 +19,7 @@ import {ObservableValue} from "../observable/ObservableValue.js"; import {HomeServerApi} from "./net/HomeServerApi.js"; import {Reconnector, ConnectionStatus} from "./net/Reconnector.js"; import {ExponentialRetryDelay} from "./net/ExponentialRetryDelay.js"; +import {MediaRepository} from "./net/MediaRepository.js"; import {HomeServerError, ConnectionError, AbortError} from "./error.js"; import {Sync, SyncStatus} from "./Sync.js"; import {Session} from "./Session.js"; @@ -158,9 +159,16 @@ export class SessionContainer { if (this._workerPromise) { olmWorker = await this._workerPromise; } - this._session = new Session({storage: this._storage, - sessionInfo: filteredSessionInfo, hsApi, olm, - clock: this._clock, olmWorker, cryptoDriver: this._cryptoDriver}); + this._session = new Session({ + storage: this._storage, + sessionInfo: filteredSessionInfo, + hsApi, + olm, + clock: this._clock, + olmWorker, + cryptoDriver: this._cryptoDriver, + mediaRepository: new MediaRepository(sessionInfo.homeServer) + }); await this._session.load(); this._status.set(LoadStatus.SessionSetup); await this._session.beforeFirstSync(isNewLogin); diff --git a/src/matrix/net/HomeServerApi.js b/src/matrix/net/HomeServerApi.js index 955e9548..63d5c830 100644 --- a/src/matrix/net/HomeServerApi.js +++ b/src/matrix/net/HomeServerApi.js @@ -1,5 +1,6 @@ /* 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. @@ -14,11 +15,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { - HomeServerError, - ConnectionError, - AbortError -} from "../error.js"; +import {HomeServerError} from "../error.js"; +import {encodeQueryParams} from "./common.js"; class RequestWrapper { constructor(method, url, requestResult) { @@ -45,18 +43,6 @@ class RequestWrapper { } } -function encodeQueryParams(queryParams) { - return Object.entries(queryParams || {}) - .filter(([, value]) => value !== undefined) - .map(([name, value]) => { - if (typeof value === "object") { - value = JSON.stringify(value); - } - return `${encodeURIComponent(name)}=${encodeURIComponent(value)}`; - }) - .join("&"); -} - export class HomeServerApi { constructor({homeServer, accessToken, request, createTimeout, reconnector}) { // store these both in a closure somehow so it's harder to get at in case of XSS? @@ -66,7 +52,6 @@ export class HomeServerApi { this._requestFn = request; this._createTimeout = createTimeout; this._reconnector = reconnector; - this._mediaRepository = new MediaRepository(homeServer); } _url(csPath) { @@ -196,45 +181,6 @@ export class HomeServerApi { roomKeyForRoomAndSession(version, roomId, sessionId, options = null) { return this._get(`/room_keys/keys/${encodeURIComponent(roomId)}/${encodeURIComponent(sessionId)}`, {version}, null, options); } - - get mediaRepository() { - return this._mediaRepository; - } -} - -class MediaRepository { - constructor(homeserver) { - this._homeserver = homeserver; - } - - mxcUrlThumbnail(url, width, height, method) { - const parts = this._parseMxcUrl(url); - if (parts) { - const [serverName, mediaId] = parts; - const httpUrl = `${this._homeserver}/_matrix/media/r0/thumbnail/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`; - return httpUrl + "?" + encodeQueryParams({width, height, method}); - } - return null; - } - - mxcUrl(url) { - const parts = this._parseMxcUrl(url); - if (parts) { - const [serverName, mediaId] = parts; - return `${this._homeserver}/_matrix/media/r0/download/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`; - } else { - return null; - } - } - - _parseMxcUrl(url) { - const prefix = "mxc://"; - if (url.startsWith(prefix)) { - return url.substr(prefix.length).split("/", 2); - } else { - return null; - } - } } export function tests() { diff --git a/src/matrix/net/MediaRepository.js b/src/matrix/net/MediaRepository.js new file mode 100644 index 00000000..63fde496 --- /dev/null +++ b/src/matrix/net/MediaRepository.js @@ -0,0 +1,52 @@ +/* +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 {encodeQueryParams} from "./common.js"; + +export class MediaRepository { + constructor(homeserver) { + this._homeserver = homeserver; + } + + mxcUrlThumbnail(url, width, height, method) { + const parts = this._parseMxcUrl(url); + if (parts) { + const [serverName, mediaId] = parts; + const httpUrl = `${this._homeserver}/_matrix/media/r0/thumbnail/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`; + return httpUrl + "?" + encodeQueryParams({width, height, method}); + } + return null; + } + + mxcUrl(url) { + const parts = this._parseMxcUrl(url); + if (parts) { + const [serverName, mediaId] = parts; + return `${this._homeserver}/_matrix/media/r0/download/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`; + } else { + return null; + } + } + + _parseMxcUrl(url) { + const prefix = "mxc://"; + if (url.startsWith(prefix)) { + return url.substr(prefix.length).split("/", 2); + } else { + return null; + } + } +} diff --git a/src/matrix/net/common.js b/src/matrix/net/common.js new file mode 100644 index 00000000..c7a06351 --- /dev/null +++ b/src/matrix/net/common.js @@ -0,0 +1,28 @@ +/* +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. +*/ + +export function encodeQueryParams(queryParams) { + return Object.entries(queryParams || {}) + .filter(([, value]) => value !== undefined) + .map(([name, value]) => { + if (typeof value === "object") { + value = JSON.stringify(value); + } + return `${encodeURIComponent(name)}=${encodeURIComponent(value)}`; + }) + .join("&"); +} diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index db61aa14..18b5d18c 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -31,11 +31,12 @@ import {DecryptionSource} from "../e2ee/common.js"; const EVENT_ENCRYPTED_TYPE = "m.room.encrypted"; export class Room extends EventEmitter { - constructor({roomId, storage, hsApi, emitCollectionChange, sendScheduler, pendingEvents, user, createRoomEncryption, getSyncToken, clock}) { + constructor({roomId, storage, hsApi, mediaRepository, emitCollectionChange, sendScheduler, pendingEvents, user, createRoomEncryption, getSyncToken, clock}) { super(); this._roomId = roomId; this._storage = storage; this._hsApi = hsApi; + this._mediaRepository = mediaRepository; this._summary = new RoomSummary(roomId, user.id); this._fragmentIdComparer = new FragmentIdComparer([]); this._syncWriter = new SyncWriter({roomId, fragmentIdComparer: this._fragmentIdComparer}); @@ -517,7 +518,7 @@ export class Room extends EventEmitter { } get mediaRepository() { - return this._hsApi.mediaRepository; + return this._mediaRepository; } /** @package */ From 0a00d4c8652524b1c5119134d4da537d703b7437 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 13:43:18 +0200 Subject: [PATCH 4/8] use hsApi wrapper that handles rate-limiting instead of send scheduler --- src/matrix/SendScheduler.js | 181 ++++++++++++--------------- src/matrix/Session.js | 13 +- src/matrix/room/Room.js | 4 +- src/matrix/room/sending/SendQueue.js | 14 +-- 4 files changed, 95 insertions(+), 117 deletions(-) diff --git a/src/matrix/SendScheduler.js b/src/matrix/SendScheduler.js index e7627c81..4080e8ad 100644 --- a/src/matrix/SendScheduler.js +++ b/src/matrix/SendScheduler.js @@ -14,72 +14,70 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {Platform} from "../Platform.js"; -import {HomeServerError, ConnectionError} from "./error.js"; +import {AbortError} from "../utils/error.js"; +import {HomeServerError} from "./error.js"; +import {HomeServerApi} from "./net/HomeServerApi.js"; +import {ExponentialRetryDelay} from "./net/ExponentialRetryDelay.js"; -export class RateLimitingBackoff { - constructor() { - this._remainingRateLimitedRequest = 0; +class Request { + constructor(methodName, args) { + this._methodName = methodName; + this._args = args; + this._responsePromise = new Promise((resolve, reject) => { + this._resolve = resolve; + this._reject = reject; + }); + this._requestResult = null; } - async waitAfterLimitExceeded(retryAfterMs) { - // this._remainingRateLimitedRequest = 5; - // if (typeof retryAfterMs !== "number") { - // } else { - // } - if (!retryAfterMs) { - retryAfterMs = 5000; + abort() { + if (this._requestResult) { + this._requestResult.abort(); + } else { + this._reject(new AbortError()); } - await Platform.delay(retryAfterMs); } - // do we have to know about succeeding requests? - // we can just - - async waitForNextSend() { - // this._remainingRateLimitedRequest = Math.max(0, this._remainingRateLimitedRequest - 1); + response() { + return this._responsePromise; } } -/* -this represents a slot to do one rate limited api call. -because rate-limiting is handled here, it should only -try to do one call, so the SendScheduler can safely -retry if the call ends up being rate limited. -This is also why we have this abstraction it hsApi is not -passed straight to SendQueue when it is its turn to send. -e.g. we wouldn't want to repeat the callback in SendQueue that could -have other side-effects before the call to hsApi that we wouldn't want -repeated (setting up progress handlers for file uploads, -... a UI update to say it started sending? - ... updating storage would probably only happen once the call succeeded - ... doing multiple hsApi calls for e.g. a file upload before sending a image message (they should individually be retried) -) maybe it is a bit overengineering, but lets stick with it for now. -At least the above is a clear definition why we have this class -*/ -//class SendSlot -- obsolete +class HomeServerApiWrapper { + constructor(scheduler) { + this._scheduler = scheduler; + } +} + +// add request-wrapping methods to prototype +for (const methodName of Object.getOwnPropertyNames(HomeServerApi.prototype)) { + if (methodName !== "constructor" && !methodName.startsWith("_")) { + HomeServerApiWrapper.prototype[methodName] = function(...args) { + return this._scheduler._hsApiRequest(methodName, args); + }; + } +} export class SendScheduler { - constructor({hsApi, backoff}) { + constructor({hsApi, clock}) { this._hsApi = hsApi; - this._sendRequests = []; - this._sendScheduled = false; + this._clock = clock; + this._requests = new Set(); + this._isRateLimited = false; + this._isDrainingRateLimit = false; this._stopped = false; - this._waitTime = 0; - this._backoff = backoff; - /* - we should have some sort of flag here that we enable - after all the rooms have been notified that they can resume - sending, so that from session, we can say scheduler.enable(); - this way, when we have better scheduling, it won't be first come, - first serve, when there are a lot of events in different rooms to send, - but we can apply some priorization of who should go first - */ - // this._enabled; + } + + createHomeServerApiWrapper() { + return new HomeServerApiWrapper(this); } stop() { - // TODO: abort current requests and set offline + this._stopped = true; + for (const request of this._requests) { + request.abort(); + } + this._requests.clear(); } start() { @@ -90,60 +88,45 @@ export class SendScheduler { return !this._stopped; } - // this should really be per roomId to avoid head-of-line blocking - // - // takes a callback instead of returning a promise with the slot - // to make sure the scheduler doesn't get blocked by a slot that is not consumed - request(sendCallback) { - let request; - const promise = new Promise((resolve, reject) => request = {resolve, reject, sendCallback}); - this._sendRequests.push(request); - if (!this._sendScheduled && !this._stopped) { - this._sendLoop(); - } - return promise; + _hsApiRequest(name, args) { + const request = new Request(name, args); + this._doSend(request); + return request; } - async _sendLoop() { - while (this._sendRequests.length) { - const request = this._sendRequests.shift(); - let result; - try { - // this can throw! - result = await this._doSend(request.sendCallback); - } catch (err) { - if (err instanceof ConnectionError) { - // we're offline, everybody will have - // to re-request slots when we come back online - this._stopped = true; - for (const r of this._sendRequests) { - r.reject(err); + async _doSend(request) { + this._requests.add(request); + try { + let retryDelay; + while (!this._stopped) { + try { + const requestResult = this._hsApi[request._methodName].apply(this._hsApi, request._args); + // so the request can be aborted + request._requestResult = requestResult; + const response = await requestResult.response(); + request._resolve(response); + return; + } catch (err) { + if (err instanceof HomeServerError && err.errcode === "M_LIMIT_EXCEEDED") { + if (Number.isSafeInteger(err.retry_after_ms)) { + await this._clock.createTimeout(err.retry_after_ms).elapsed(); + } else { + if (!retryDelay) { + retryDelay = new ExponentialRetryDelay(this._clock.createTimeout); + } + await retryDelay.waitForRetry(); + } + } else { + request._reject(err); + return; } - this._sendRequests = []; - } - console.error("error for request", err); - request.reject(err); - break; - } - request.resolve(result); - } - // do next here instead of in _doSend - } - - async _doSend(sendCallback) { - this._sendScheduled = false; - await this._backoff.waitForNextSend(); - // loop is left by return or throw - while (true) { // eslint-disable-line no-constant-condition - try { - return await sendCallback(this._hsApi); - } catch (err) { - if (err instanceof HomeServerError && err.errcode === "M_LIMIT_EXCEEDED") { - await this._backoff.waitAfterLimitExceeded(err.retry_after_ms); - } else { - throw err; } } + if (this._stopped) { + request.abort(); + } + } finally { + this._requests.delete(request); } } } diff --git a/src/matrix/Session.js b/src/matrix/Session.js index c2c2e20a..32fe96b5 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -16,7 +16,7 @@ limitations under the License. import {Room} from "./room/Room.js"; import { ObservableMap } from "../observable/index.js"; -import { SendScheduler, RateLimitingBackoff } from "./SendScheduler.js"; +import {SendScheduler} from "./SendScheduler.js"; import {User} from "./User.js"; import {DeviceMessageHandler} from "./DeviceMessageHandler.js"; import {Account as E2EEAccount} from "./e2ee/Account.js"; @@ -42,15 +42,15 @@ const PICKLE_KEY = "DEFAULT_KEY"; export class Session { // sessionInfo contains deviceId, userId and homeServer - constructor({clock, storage, hsApi, sessionInfo, olm, olmWorker, cryptoDriver, mediaRepository}) { + constructor({clock, storage, unwrappedHsApi, sessionInfo, olm, olmWorker, cryptoDriver, mediaRepository}) { this._clock = clock; this._storage = storage; - this._hsApi = hsApi; + this._sendScheduler = new SendScheduler({hsApi: unwrappedHsApi, clock}); + this._hsApi = this._sendScheduler.createHomeServerApiWrapper(); this._mediaRepository = mediaRepository; this._syncInfo = null; this._sessionInfo = sessionInfo; this._rooms = new ObservableMap(); - this._sendScheduler = new SendScheduler({hsApi, backoff: new RateLimitingBackoff()}); this._roomUpdateCallback = (room, params) => this._rooms.update(room.id, params); this._user = new User(sessionInfo.userId); this._deviceMessageHandler = new DeviceMessageHandler({storage}); @@ -332,9 +332,8 @@ export class Session { storage: this._storage, emitCollectionChange: this._roomUpdateCallback, hsApi: this._hsApi, - sendScheduler: this._sendScheduler, -._hsApi, - mediaRepository: this._mediaRep pendingEvents, + mediaRepository: this._mediaRepository, + pendingEvents, user: this._user, createRoomEncryption: this._createRoomEncryption, clock: this._clock diff --git a/src/matrix/room/Room.js b/src/matrix/room/Room.js index 18b5d18c..99c2155e 100644 --- a/src/matrix/room/Room.js +++ b/src/matrix/room/Room.js @@ -31,7 +31,7 @@ import {DecryptionSource} from "../e2ee/common.js"; const EVENT_ENCRYPTED_TYPE = "m.room.encrypted"; export class Room extends EventEmitter { - constructor({roomId, storage, hsApi, mediaRepository, emitCollectionChange, sendScheduler, pendingEvents, user, createRoomEncryption, getSyncToken, clock}) { + constructor({roomId, storage, hsApi, mediaRepository, emitCollectionChange, pendingEvents, user, createRoomEncryption, getSyncToken, clock}) { super(); this._roomId = roomId; this._storage = storage; @@ -41,7 +41,7 @@ export class Room extends EventEmitter { this._fragmentIdComparer = new FragmentIdComparer([]); this._syncWriter = new SyncWriter({roomId, fragmentIdComparer: this._fragmentIdComparer}); this._emitCollectionChange = emitCollectionChange; - this._sendQueue = new SendQueue({roomId, storage, sendScheduler, pendingEvents}); + this._sendQueue = new SendQueue({roomId, storage, hsApi, pendingEvents}); this._timeline = null; this._user = user; this._changedMembersDuringSync = null; diff --git a/src/matrix/room/sending/SendQueue.js b/src/matrix/room/sending/SendQueue.js index fe7afe77..52c2e7b8 100644 --- a/src/matrix/room/sending/SendQueue.js +++ b/src/matrix/room/sending/SendQueue.js @@ -20,11 +20,11 @@ import {PendingEvent} from "./PendingEvent.js"; import {makeTxnId} from "../../common.js"; export class SendQueue { - constructor({roomId, storage, sendScheduler, pendingEvents}) { + constructor({roomId, storage, hsApi, pendingEvents}) { pendingEvents = pendingEvents || []; this._roomId = roomId; this._storage = storage; - this._sendScheduler = sendScheduler; + this._hsApi = hsApi; this._pendingEvents = new SortedArray((a, b) => a.queueIndex - b.queueIndex); if (pendingEvents.length) { console.info(`SendQueue for room ${roomId} has ${pendingEvents.length} pending events`, pendingEvents); @@ -51,22 +51,18 @@ export class SendQueue { continue; } if (pendingEvent.needsEncryption) { - const {type, content} = await this._sendScheduler.request(async hsApi => { - return await this._roomEncryption.encrypt(pendingEvent.eventType, pendingEvent.content, hsApi); - }); + const {type, content} = await this._roomEncryption.encrypt( + pendingEvent.eventType, pendingEvent.content, this._hsApi); pendingEvent.setEncrypted(type, content); await this._tryUpdateEvent(pendingEvent); } console.log("really sending now"); - const response = await this._sendScheduler.request(hsApi => { - console.log("got sendScheduler slot"); - return hsApi.send( + const response = await this._hsApi.send( pendingEvent.roomId, pendingEvent.eventType, pendingEvent.txnId, pendingEvent.content ).response(); - }); pendingEvent.remoteId = response.event_id; // console.log("writing remoteId now"); From 5660e0f4f0c8d481b87db6b2712c1a52f18e3f80 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 13:49:01 +0200 Subject: [PATCH 5/8] rename send scheduler to request scheduler --- src/matrix/Session.js | 12 ++++++------ .../{SendScheduler.js => net/RequestScheduler.js} | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) rename src/matrix/{SendScheduler.js => net/RequestScheduler.js} (93%) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index 32fe96b5..fdc9a83f 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -16,7 +16,7 @@ limitations under the License. import {Room} from "./room/Room.js"; import { ObservableMap } from "../observable/index.js"; -import {SendScheduler} from "./SendScheduler.js"; +import {RequestScheduler} from "./net/RequestScheduler.js"; import {User} from "./User.js"; import {DeviceMessageHandler} from "./DeviceMessageHandler.js"; import {Account as E2EEAccount} from "./e2ee/Account.js"; @@ -45,8 +45,8 @@ export class Session { constructor({clock, storage, unwrappedHsApi, sessionInfo, olm, olmWorker, cryptoDriver, mediaRepository}) { this._clock = clock; this._storage = storage; - this._sendScheduler = new SendScheduler({hsApi: unwrappedHsApi, clock}); - this._hsApi = this._sendScheduler.createHomeServerApiWrapper(); + this._requestScheduler = new RequestScheduler({hsApi: unwrappedHsApi, clock}); + this._hsApi = this._requestScheduler.createHomeServerApiWrapper(); this._mediaRepository = mediaRepository; this._syncInfo = null; this._sessionInfo = sessionInfo; @@ -268,12 +268,12 @@ export class Session { } get isStarted() { - return this._sendScheduler.isStarted; + return this._requestScheduler.isStarted; } dispose() { this._olmWorker?.dispose(); - this._sendScheduler.stop(); + this._requestScheduler.stop(); this._sessionBackup?.dispose(); for (const room of this._rooms.values()) { room.dispose(); @@ -297,7 +297,7 @@ export class Session { const operations = await opsTxn.operations.getAll(); const operationsByScope = groupBy(operations, o => o.scope); - this._sendScheduler.start(); + this._requestScheduler.start(); for (const [, room] of this._rooms) { let roomOperationsByType; const roomOperations = operationsByScope.get(room.id); diff --git a/src/matrix/SendScheduler.js b/src/matrix/net/RequestScheduler.js similarity index 93% rename from src/matrix/SendScheduler.js rename to src/matrix/net/RequestScheduler.js index 4080e8ad..0e75c9d7 100644 --- a/src/matrix/SendScheduler.js +++ b/src/matrix/net/RequestScheduler.js @@ -14,10 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {AbortError} from "../utils/error.js"; -import {HomeServerError} from "./error.js"; -import {HomeServerApi} from "./net/HomeServerApi.js"; -import {ExponentialRetryDelay} from "./net/ExponentialRetryDelay.js"; +import {AbortError} from "../../utils/error.js"; +import {HomeServerError} from "../error.js"; +import {HomeServerApi} from "./HomeServerApi.js"; +import {ExponentialRetryDelay} from "./ExponentialRetryDelay.js"; class Request { constructor(methodName, args) { @@ -58,7 +58,7 @@ for (const methodName of Object.getOwnPropertyNames(HomeServerApi.prototype)) { } } -export class SendScheduler { +export class RequestScheduler { constructor({hsApi, clock}) { this._hsApi = hsApi; this._clock = clock; From 85b451ffa14ec2fd5713acb0db461572f70e419c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 15:49:43 +0200 Subject: [PATCH 6/8] can't rename named params like this --- src/matrix/Session.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index fdc9a83f..db9f6933 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -42,10 +42,10 @@ const PICKLE_KEY = "DEFAULT_KEY"; export class Session { // sessionInfo contains deviceId, userId and homeServer - constructor({clock, storage, unwrappedHsApi, sessionInfo, olm, olmWorker, cryptoDriver, mediaRepository}) { + constructor({clock, storage, hsApi, sessionInfo, olm, olmWorker, cryptoDriver, mediaRepository}) { this._clock = clock; this._storage = storage; - this._requestScheduler = new RequestScheduler({hsApi: unwrappedHsApi, clock}); + this._requestScheduler = new RequestScheduler({hsApi, clock}); this._hsApi = this._requestScheduler.createHomeServerApiWrapper(); this._mediaRepository = mediaRepository; this._syncInfo = null; From 150f06b9bf88b8f4e9120c3707500adbc1ca8e25 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 16:39:04 +0200 Subject: [PATCH 7/8] also move to Stopped for aborts --- 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 dc169deb..515c096d 100644 --- a/src/matrix/Sync.js +++ b/src/matrix/Sync.js @@ -120,11 +120,11 @@ export class Sync { this._status.set(SyncStatus.Syncing); } } catch (err) { + this._status.set(SyncStatus.Stopped); if (!(err instanceof AbortError)) { console.warn("stopping sync because of error"); console.error(err); this._error = err; - this._status.set(SyncStatus.Stopped); } } if (this._status.get() !== SyncStatus.Stopped) { From 137f55b44d1076dfd875cbb3245358d338fa6685 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 22 Sep 2020 16:39:41 +0200 Subject: [PATCH 8/8] manage request scheduler in session container so we can start it before sync does its first request, which otherwise gets aborted because the scheduler hasn't started yet --- src/matrix/Session.js | 10 +--------- src/matrix/SessionContainer.js | 23 +++++++++++++++-------- src/matrix/net/RequestScheduler.js | 12 +++++------- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index db9f6933..2e470b58 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -16,7 +16,6 @@ limitations under the License. import {Room} from "./room/Room.js"; import { ObservableMap } from "../observable/index.js"; -import {RequestScheduler} from "./net/RequestScheduler.js"; import {User} from "./User.js"; import {DeviceMessageHandler} from "./DeviceMessageHandler.js"; import {Account as E2EEAccount} from "./e2ee/Account.js"; @@ -45,8 +44,7 @@ export class Session { constructor({clock, storage, hsApi, sessionInfo, olm, olmWorker, cryptoDriver, mediaRepository}) { this._clock = clock; this._storage = storage; - this._requestScheduler = new RequestScheduler({hsApi, clock}); - this._hsApi = this._requestScheduler.createHomeServerApiWrapper(); + this._hsApi = hsApi; this._mediaRepository = mediaRepository; this._syncInfo = null; this._sessionInfo = sessionInfo; @@ -267,13 +265,8 @@ export class Session { })); } - get isStarted() { - return this._requestScheduler.isStarted; - } - dispose() { this._olmWorker?.dispose(); - this._requestScheduler.stop(); this._sessionBackup?.dispose(); for (const room of this._rooms.values()) { room.dispose(); @@ -297,7 +290,6 @@ export class Session { const operations = await opsTxn.operations.getAll(); const operationsByScope = groupBy(operations, o => o.scope); - this._requestScheduler.start(); for (const [, room] of this._rooms) { let roomOperationsByType; const roomOperations = operationsByScope.get(room.id); diff --git a/src/matrix/SessionContainer.js b/src/matrix/SessionContainer.js index be2c0c72..dff6e38c 100644 --- a/src/matrix/SessionContainer.js +++ b/src/matrix/SessionContainer.js @@ -20,6 +20,7 @@ import {HomeServerApi} from "./net/HomeServerApi.js"; import {Reconnector, ConnectionStatus} from "./net/Reconnector.js"; import {ExponentialRetryDelay} from "./net/ExponentialRetryDelay.js"; import {MediaRepository} from "./net/MediaRepository.js"; +import {RequestScheduler} from "./net/RequestScheduler.js"; import {HomeServerError, ConnectionError, AbortError} from "./error.js"; import {Sync, SyncStatus} from "./Sync.js"; import {Session} from "./Session.js"; @@ -50,7 +51,7 @@ export class SessionContainer { this._request = request; this._storageFactory = storageFactory; this._sessionInfoStorage = sessionInfoStorage; - + this._sessionStartedByReconnector = false; this._status = new ObservableValue(LoadStatus.NotLoading); this._error = null; this._loginFailure = null; @@ -59,6 +60,7 @@ export class SessionContainer { this._sync = null; this._sessionId = null; this._storage = null; + this._requestScheduler = null; this._olmPromise = olmPromise; this._workerPromise = workerPromise; this._cryptoDriver = cryptoDriver; @@ -133,6 +135,7 @@ export class SessionContainer { } async _loadSessionInfo(sessionInfo, isNewLogin) { + this._sessionStartedByReconnector = false; this._status.set(LoadStatus.Loading); this._reconnector = new Reconnector({ onlineStatus: this._onlineStatus, @@ -159,10 +162,12 @@ export class SessionContainer { if (this._workerPromise) { olmWorker = await this._workerPromise; } + this._requestScheduler = new RequestScheduler({hsApi, clock: this._clock}); + this._requestScheduler.start(); this._session = new Session({ storage: this._storage, sessionInfo: filteredSessionInfo, - hsApi, + hsApi: this._requestScheduler.hsApi, olm, clock: this._clock, olmWorker, @@ -173,11 +178,14 @@ export class SessionContainer { this._status.set(LoadStatus.SessionSetup); await this._session.beforeFirstSync(isNewLogin); - this._sync = new Sync({hsApi, storage: this._storage, session: this._session}); + this._sync = new Sync({hsApi: this._requestScheduler.hsApi, storage: this._storage, session: this._session}); // notify sync and session when back online this._reconnectSubscription = this._reconnector.connectionStatus.subscribe(state => { if (state === ConnectionStatus.Online) { + // needs to happen before sync and session or it would abort all requests + this._requestScheduler.start(); this._sync.start(); + this._sessionStartedByReconnector = true; this._session.start(this._reconnector.lastVersionsResponse); } }); @@ -189,11 +197,7 @@ export class SessionContainer { // restored the connection, it would have already // started to session, so check first // to prevent an extra /versions request - - // TODO: this doesn't look logical, but works. Why? - // I think because isStarted is true by default. That's probably not what we intend. - // I think there is a bug here, in that even if the reconnector already started the session, we'd still do this. - if (this._session.isStarted) { + if (!this._sessionStartedByReconnector) { const lastVersionsResponse = await hsApi.versions({timeout: 10000}).response(); this._session.start(lastVersionsResponse); } @@ -259,6 +263,9 @@ export class SessionContainer { this._reconnectSubscription(); this._reconnectSubscription = null; } + if (this._requestScheduler) { + this._requestScheduler.stop(); + } if (this._sync) { this._sync.stop(); } diff --git a/src/matrix/net/RequestScheduler.js b/src/matrix/net/RequestScheduler.js index 0e75c9d7..53ab50ac 100644 --- a/src/matrix/net/RequestScheduler.js +++ b/src/matrix/net/RequestScheduler.js @@ -1,5 +1,6 @@ /* 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. @@ -65,11 +66,12 @@ export class RequestScheduler { this._requests = new Set(); this._isRateLimited = false; this._isDrainingRateLimit = false; - this._stopped = false; + this._stopped = true; + this._wrapper = new HomeServerApiWrapper(this); } - createHomeServerApiWrapper() { - return new HomeServerApiWrapper(this); + get hsApi() { + return this._wrapper; } stop() { @@ -84,10 +86,6 @@ export class RequestScheduler { this._stopped = false; } - get isStarted() { - return !this._stopped; - } - _hsApiRequest(name, args) { const request = new Request(name, args); this._doSend(request);