From ef267ca3313eedd9fa3c7a42c9f265d3f3cd108a Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Sun, 5 Apr 2020 15:11:15 +0200 Subject: [PATCH] WIP2 --- doc/impl-thoughts/RECONNECTING.md | 16 ++-- src/domain/BrawlViewModel.js | 16 +++- src/main.js | 6 +- src/matrix/{Reconnecter.js => Reconnector.js} | 93 ++++++++----------- src/matrix/hs-api.js | 71 +++++++++----- src/matrix/session.js | 2 +- src/matrix/sync.js | 1 + src/utils/DOMClock.js | 7 +- 8 files changed, 118 insertions(+), 94 deletions(-) rename src/matrix/{Reconnecter.js => Reconnector.js} (56%) diff --git a/doc/impl-thoughts/RECONNECTING.md b/doc/impl-thoughts/RECONNECTING.md index f818b6fc..f648481f 100644 --- a/doc/impl-thoughts/RECONNECTING.md +++ b/doc/impl-thoughts/RECONNECTING.md @@ -1,22 +1,22 @@ # Reconnecting -`HomeServerApi` notifies `Reconnecter` of network call failure +`HomeServerApi` notifies `Reconnector` of network call failure -`Reconnecter` listens for online/offline event +`Reconnector` listens for online/offline event -`Reconnecter` polls `/versions` with a `RetryDelay` (implemented as ExponentialRetryDelay, also used by SendScheduler if no retry_after_ms is given) +`Reconnector` polls `/versions` with a `RetryDelay` (implemented as ExponentialRetryDelay, also used by SendScheduler if no retry_after_ms is given) -`Reconnecter` emits an event when sync and message sending should retry +`Reconnector` emits an event when sync and message sending should retry -`Sync` listen to `Reconnecter` +`Sync` listen to `Reconnector` `Sync` notifies when the catchup sync has happened -`Reconnecter` has state: +`Reconnector` has state: - disconnected (and retrying at x seconds from timestamp) - reconnecting (call /versions, and if successful /sync) - connected -`Reconnecter` has a method to try to connect now +`Reconnector` has a method to try to connect now `SessionStatus` can be: - disconnected (and retrying at x seconds from timestamp) @@ -31,4 +31,4 @@ rooms should report how many messages they have queued up, and each time they se `SendReporter` (passed from `Session` to `Room`, passed down to `SendQueue`), with: - setPendingEventCount(roomId, count) -`Session` listens to `Reconnecter` to update it's status, but perhaps we wait to send messages until catchup sync is done +`Session` listens to `Reconnector` to update it's status, but perhaps we wait to send messages until catchup sync is done diff --git a/src/domain/BrawlViewModel.js b/src/domain/BrawlViewModel.js index c9ebfa91..20bddd7f 100644 --- a/src/domain/BrawlViewModel.js +++ b/src/domain/BrawlViewModel.js @@ -126,7 +126,11 @@ export default class BrawlViewModel extends EventEmitter { try { this._loading = true; this._loadingText = "Loading your conversations…"; - const hsApi = this._createHsApi(sessionInfo.homeServer, sessionInfo.accessToken); + const reconnector = new Reconnector( + new ExponentialRetryDelay(2000, this._clock.createTimeout), + this._clock.createMeasure + ); + const hsApi = this._createHsApi(sessionInfo.homeServer, sessionInfo.accessToken, reconnector); const storage = await this._storageFactory.create(sessionInfo.id); // no need to pass access token to session const filteredSessionInfo = { @@ -136,10 +140,16 @@ export default class BrawlViewModel extends EventEmitter { }; const session = new Session({storage, sessionInfo: filteredSessionInfo, hsApi}); // show spinner now, with title loading stored data? - this.emit("change", "activeSection"); await session.load(); - const sync = new Sync({hsApi, storage, session}); + const sync = new Sync({hsApi, storage, session}); + + reconnector.on("state", state => { + if (state === ConnectionState.Online) { + sync.start(); + session.notifyNetworkAvailable(reconnector.lastVersionsResponse); + } + }); const needsInitialSync = !session.syncToken; if (!needsInitialSync) { diff --git a/src/main.js b/src/main.js index 9e973bb9..b8e4ab33 100644 --- a/src/main.js +++ b/src/main.js @@ -5,6 +5,7 @@ import StorageFactory from "./matrix/storage/idb/create.js"; import SessionsStore from "./matrix/sessions-store/localstorage/SessionsStore.js"; import BrawlViewModel from "./domain/BrawlViewModel.js"; import BrawlView from "./ui/web/BrawlView.js"; +import DOMClock from "./utils/DOMClock.js"; export default async function main(container) { try { @@ -17,14 +18,13 @@ export default async function main(container) { // const recorder = new RecordRequester(fetchRequest); // const request = recorder.request; // window.getBrawlFetchLog = () => recorder.log(); - // normal network: const request = fetchRequest; const vm = new BrawlViewModel({ storageFactory: new StorageFactory(), - createHsApi: (homeServer, accessToken = null) => new HomeServerApi({homeServer, accessToken, request}), + createHsApi: (homeServer, accessToken, reconnector) => new HomeServerApi({homeServer, accessToken, request, reconnector}), sessionStore: new SessionsStore("brawl_sessions_v1"), - clock: Date //just for `now` fn + clock: new DOMClock(), }); await vm.load(); const view = new BrawlView(vm); diff --git a/src/matrix/Reconnecter.js b/src/matrix/Reconnector.js similarity index 56% rename from src/matrix/Reconnecter.js rename to src/matrix/Reconnector.js index 863a263c..2125380e 100644 --- a/src/matrix/Reconnecter.js +++ b/src/matrix/Reconnector.js @@ -1,53 +1,49 @@ -class Clock { - // use cases - // StopWatch: not sure I like that name ... but measure time difference from start to current time - // Timeout: wait for a given number of ms, and be able to interrupt the wait - // Clock.timeout() -> creates a new timeout? - // Now: get current timestamp - // Clock.now(), or pass Clock.now so others can do now() - // - // should use subinterfaces so we can only pass part needed to other constructors - // -} - - // need to prevent memory leaks here! export class DomOnlineDetected { - constructor(reconnecter) { + constructor(reconnector) { // window.addEventListener('offline', () => appendOnlineStatus(false)); // window.addEventListener('online', () => appendOnlineStatus(true)); // appendOnlineStatus(navigator.onLine); - // on online, reconnecter.tryNow() + // on online, reconnector.tryNow() } } export class ExponentialRetryDelay { - constructor(start = 2000, delay) { + constructor(start = 2000, createTimeout) { this._start = start; this._current = start; - this._delay = delay; + this._createTimeout = createTimeout; this._max = 60 * 5 * 1000; //5 min - this._timer = null; + this._timeout = null; } async waitForRetry() { - this._timer = this._delay(this._current); + this._timeout = this._createTimeout(this._current); try { - await this._timer.timeout(); + await this._timeout.elapsed(); // only increase delay if we didn't get interrupted const seconds = this._current / 1000; const powerOfTwo = (seconds * seconds) * 1000; this._current = Math.max(this._max, powerOfTwo); + } catch(err) { + // swallow AbortError, means skipWaiting was called + if (!(err instanceof AbortError)) { + throw err; + } } finally { - this._timer = null; + this._timeout = null; + } + } + + skipWaiting() { + if (this._timeout) { + this._timeout.abort(); } } reset() { this._current = this._start; - if (this._timer) { - this._timer.abort(); - } + this.skipWaiting(); } get nextValue() { @@ -80,13 +76,12 @@ export const ConnectionState = createEnum( "Online" ); -export class Reconnecter { - constructor({hsApi, retryDelay, clock}) { +export class Reconnector { + constructor({retryDelay, createTimeMeasure}) { this._online this._retryDelay = retryDelay; this._currentDelay = null; - this._hsApi = hsApi; - this._clock = clock; + this._createTimeMeasure = createTimeMeasure; // assume online, and do our thing when something fails this._state = ConnectionState.Online; this._isReconnecting = false; @@ -102,25 +97,22 @@ export class Reconnecter { } get retryIn() { - return this._stateSince.measure(); + if (this._state === ConnectionState.Waiting) { + return this._retryDelay.nextValue - this._stateSince.measure(); + } + return 0; } - onRequestFailed() { + onRequestFailed(hsApi) { if (!this._isReconnecting) { this._setState(ConnectionState.Offline); - // do something with versions response of loop here? - // we might want to pass it to session to know what server supports? - // so emit it ... - this._reconnectLoop(); - // start loop + this._reconnectLoop(hsApi); } } - // don't throw from here tryNow() { - // skip waiting - if (this._currentDelay) { - this._currentDelay.abort(); + if (this._retryDelay) { + this._retryDelay.skipWaiting(); } } @@ -128,7 +120,7 @@ export class Reconnecter { if (state !== this._state) { this._state = state; if (this._state === ConnectionState.Waiting) { - this._stateSince = this._clock.stopwatch(); + this._stateSince = this._createTimeMeasure(); } else { this._stateSince = null; } @@ -136,30 +128,23 @@ export class Reconnecter { } } - async _reconnectLoop() { + async _reconnectLoop(hsApi) { this._isReconnecting = true; - this._retryDelay.reset(); this._versionsResponse = null; + this._retryDelay.reset(); while (!this._versionsResponse) { - // TODO: should we wait first or request first? - // as we've just failed a request? I guess no harm in trying immediately try { this._setState(ConnectionState.Reconnecting); - const versionsRequest = this._hsApi.versions(10000); + // use 10s timeout, because we don't want to be waiting for + // a stale connection when we just came online again + const versionsRequest = hsApi.versions({timeout: 10000}); this._versionsResponse = await versionsRequest.response(); this._setState(ConnectionState.Online); } catch (err) { + // NetworkError or AbortError from timeout this._setState(ConnectionState.Waiting); - this._currentDelay = this._retryDelay.next(); - try { - await this._currentDelay - } catch (err) { - // waiting interrupted, we should retry immediately, - // swallow error - } finally { - this._currentDelay = null; - } + await this._retryDelay.waitForRetry(); } } } diff --git a/src/matrix/hs-api.js b/src/matrix/hs-api.js index 4a2747d7..314a9324 100644 --- a/src/matrix/hs-api.js +++ b/src/matrix/hs-api.js @@ -1,5 +1,6 @@ import { HomeServerError, + NetworkError, } from "./error.js"; class RequestWrapper { @@ -28,19 +29,21 @@ class RequestWrapper { } export default class HomeServerApi { - constructor({homeServer, accessToken, request}) { + constructor({homeServer, accessToken, request, createTimeout, reconnector}) { // store these both in a closure somehow so it's harder to get at in case of XSS? // one could change the homeserver as well so the token gets sent there, so both must be protected from read/write this._homeserver = homeServer; this._accessToken = accessToken; this._requestFn = request; + this._createTimeout = createTimeout; + this._reconnector = reconnector; } _url(csPath) { return `${this._homeserver}/_matrix/client/r0${csPath}`; } - _request(method, url, queryParams = {}, body) { + _request(method, url, queryParams = {}, body, options) { const queryString = Object.entries(queryParams) .filter(([, value]) => value !== undefined) .map(([name, value]) => { @@ -66,51 +69,73 @@ export default class HomeServerApi { headers, body: bodyString, }); - return new RequestWrapper(method, url, requestResult); + + if (options.timeout) { + const timeout = this._createTimeout(options.timeout); + // abort request if timeout finishes first + timeout.elapsed().then( + () => requestResult.abort(), + () => {} // ignore AbortError + ); + // abort timeout if request finishes first + requestResult.response().then(() => timeout.abort()); + } + + + const wrapper = new RequestWrapper(method, url, requestResult); + + if (this._reconnector) { + wrapper.response().catch(err => { + if (err instanceof NetworkError) { + this._reconnector.onRequestFailed(this); + } + }); + } + + return wrapper; } - _post(csPath, queryParams, body) { - return this._request("POST", this._url(csPath), queryParams, body); + _post(csPath, queryParams, body, options) { + return this._request("POST", this._url(csPath), queryParams, body, options); } - _put(csPath, queryParams, body) { - return this._request("PUT", this._url(csPath), queryParams, body); + _put(csPath, queryParams, body, options) { + return this._request("PUT", this._url(csPath), queryParams, body, options); } - _get(csPath, queryParams, body) { - return this._request("GET", this._url(csPath), queryParams, body); + _get(csPath, queryParams, body, options) { + return this._request("GET", this._url(csPath), queryParams, body, options); } - sync(since, filter, timeout) { - return this._get("/sync", {since, timeout, filter}); + sync(since, filter, timeout, options = null) { + return this._get("/sync", {since, timeout, filter}, null, options); } // params is from, dir and optionally to, limit, filter. - messages(roomId, params) { - return this._get(`/rooms/${encodeURIComponent(roomId)}/messages`, params); + messages(roomId, params, options = null) { + return this._get(`/rooms/${encodeURIComponent(roomId)}/messages`, params, null, options); } - send(roomId, eventType, txnId, content) { - return this._put(`/rooms/${encodeURIComponent(roomId)}/send/${encodeURIComponent(eventType)}/${encodeURIComponent(txnId)}`, {}, content); + send(roomId, eventType, txnId, content, options = null) { + return this._put(`/rooms/${encodeURIComponent(roomId)}/send/${encodeURIComponent(eventType)}/${encodeURIComponent(txnId)}`, {}, content, options); } - passwordLogin(username, password) { - return this._post("/login", undefined, { + passwordLogin(username, password, options = null) { + return this._post("/login", null, { "type": "m.login.password", "identifier": { "type": "m.id.user", "user": username }, "password": password - }); + }, options); } - createFilter(userId, filter) { - return this._post(`/user/${encodeURIComponent(userId)}/filter`, undefined, filter); + createFilter(userId, filter, options = null) { + return this._post(`/user/${encodeURIComponent(userId)}/filter`, null, filter, options); } - versions(timeout) { - // TODO: implement timeout - return this._request("GET", `${this._homeserver}/_matrix/client/versions`); + versions(options = null) { + return this._request("GET", `${this._homeserver}/_matrix/client/versions`, null, options); } } diff --git a/src/matrix/session.js b/src/matrix/session.js index 33cd16aa..837cb51a 100644 --- a/src/matrix/session.js +++ b/src/matrix/session.js @@ -40,7 +40,7 @@ export default class Session { })); } - notifyNetworkAvailable() { + notifyNetworkAvailable(lastVersionResponse) { for (const [, room] of this._rooms) { room.resumeSending(); } diff --git a/src/matrix/sync.js b/src/matrix/sync.js index bc5f5797..56208198 100644 --- a/src/matrix/sync.js +++ b/src/matrix/sync.js @@ -33,6 +33,7 @@ export default class Sync extends EventEmitter { return this._isSyncing; } + // this should not throw? // returns when initial sync is done async start() { if (this._isSyncing) { diff --git a/src/utils/DOMClock.js b/src/utils/DOMClock.js index eeb5ebcc..c943b109 100644 --- a/src/utils/DOMClock.js +++ b/src/utils/DOMClock.js @@ -3,22 +3,25 @@ import {AbortError} from "./error.js"; class DOMTimeout { constructor(ms) { this._reject = null; + this._handle = null; this._promise = new Promise((resolve, reject) => { this._reject = reject; - setTimeout(() => { + this._handle = setTimeout(() => { this._reject = null; resolve(); }, ms); }); } - get elapsed() { + elapsed() { return this._promise; } abort() { if (this._reject) { this._reject(new AbortError()); + clearTimeout(this._handle); + this._handle = null; this._reject = null; } }