From e8e9740521341d7ef6999fb8eb46880590588100 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 5 Aug 2020 15:36:44 +0000 Subject: [PATCH] Move timeout to fetch, as XHR has native timeout support --- src/main.js | 8 ++-- src/matrix/net/HomeServerApi.js | 46 ++---------------- src/matrix/net/request/fetch.js | 85 ++++++++++++++++++--------------- src/matrix/net/timeout.js | 28 +++++++++++ 4 files changed, 83 insertions(+), 84 deletions(-) create mode 100644 src/matrix/net/timeout.js diff --git a/src/main.js b/src/main.js index 5a52b3a7..f5c13fc4 100644 --- a/src/main.js +++ b/src/main.js @@ -15,7 +15,7 @@ limitations under the License. */ // import {RecordRequester, ReplayRequester} from "./matrix/net/request/replay.js"; -import {fetchRequest} from "./matrix/net/request/fetch.js"; +import {createFetchRequest} from "./matrix/net/request/fetch.js"; import {SessionContainer} from "./matrix/SessionContainer.js"; import {StorageFactory} from "./matrix/storage/idb/StorageFactory.js"; import {SessionInfoStorage} from "./matrix/sessioninfo/localstorage/SessionInfoStorage.js"; @@ -32,13 +32,13 @@ export default async function main(container) { // const request = replay.request; // to record: - // const recorder = new RecordRequester(fetchRequest); + // const recorder = new RecordRequester(createFetchRequest(clock.createTimeout)); // const request = recorder.request; // window.getBrawlFetchLog = () => recorder.log(); // normal network: - const request = fetchRequest; - const sessionInfoStorage = new SessionInfoStorage("brawl_sessions_v1"); const clock = new Clock(); + const request = createFetchRequest(clock.createTimeout); + const sessionInfoStorage = new SessionInfoStorage("brawl_sessions_v1"); const storageFactory = new StorageFactory(); const vm = new BrawlViewModel({ diff --git a/src/matrix/net/HomeServerApi.js b/src/matrix/net/HomeServerApi.js index 19c0495e..c71ef0a5 100644 --- a/src/matrix/net/HomeServerApi.js +++ b/src/matrix/net/HomeServerApi.js @@ -21,9 +21,9 @@ import { } from "../error.js"; class RequestWrapper { - constructor(method, url, requestResult, responsePromise) { + constructor(method, url, requestResult) { this._requestResult = requestResult; - this._promise = responsePromise.then(response => { + this._promise = requestResult.response().then(response => { // ok? if (response.status >= 200 && response.status < 300) { return response.body; @@ -60,35 +60,6 @@ export class HomeServerApi { return `${this._homeserver}/_matrix/client/r0${csPath}`; } - _abortOnTimeout(timeoutAmount, requestResult, responsePromise) { - const timeout = this._createTimeout(timeoutAmount); - // abort request if timeout finishes first - let timedOut = false; - timeout.elapsed().then( - () => { - timedOut = true; - requestResult.abort(); - }, - () => {} // ignore AbortError - ); - // abort timeout if request finishes first - return responsePromise.then( - response => { - timeout.abort(); - return response; - }, - err => { - timeout.abort(); - // map error to TimeoutError - if (err instanceof AbortError && timedOut) { - throw new ConnectionError(`Request timed out after ${timeoutAmount}ms`, true); - } else { - throw err; - } - } - ); - } - _encodeQueryParams(queryParams) { return Object.entries(queryParams || {}) .filter(([, value]) => value !== undefined) @@ -118,19 +89,10 @@ export class HomeServerApi { method, headers, body: bodyString, + timeout: options && options.timeout }); - let responsePromise = requestResult.response(); - - if (options && options.timeout) { - responsePromise = this._abortOnTimeout( - options.timeout, - requestResult, - responsePromise - ); - } - - const wrapper = new RequestWrapper(method, url, requestResult, responsePromise); + const wrapper = new RequestWrapper(method, url, requestResult); if (this._reconnector) { wrapper.response().catch(err => { diff --git a/src/matrix/net/request/fetch.js b/src/matrix/net/request/fetch.js index 77c12dad..e7f33120 100644 --- a/src/matrix/net/request/fetch.js +++ b/src/matrix/net/request/fetch.js @@ -18,6 +18,7 @@ import { AbortError, ConnectionError } from "../../error.js"; +import {abortOnTimeout} from "../timeout.js"; class RequestResult { constructor(promise, controller) { @@ -31,9 +32,9 @@ class RequestResult { } }; }); - this._promise = Promise.race([promise, abortPromise]); + this.promise = Promise.race([promise, abortPromise]); } else { - this._promise = promise; + this.promise = promise; this._controller = controller; } } @@ -43,47 +44,55 @@ class RequestResult { } response() { - return this._promise; + return this.promise; } } -export function fetchRequest(url, options) { - const controller = typeof AbortController === "function" ? new AbortController() : null; - if (controller) { +export function createFetchRequest(createTimeout) { + return function fetchRequest(url, options) { + const controller = typeof AbortController === "function" ? new AbortController() : null; + if (controller) { + options = Object.assign(options, { + signal: controller.signal + }); + } options = Object.assign(options, { - signal: controller.signal + mode: "cors", + credentials: "omit", + referrer: "no-referrer", + cache: "no-cache", }); - } - options = Object.assign(options, { - mode: "cors", - credentials: "omit", - referrer: "no-referrer", - cache: "no-cache", - }); - if (options.headers) { - const headers = new Headers(); - for(const [name, value] of options.headers.entries()) { - headers.append(name, value); + if (options.headers) { + const headers = new Headers(); + for(const [name, value] of options.headers.entries()) { + headers.append(name, value); + } + options.headers = headers; } - options.headers = headers; - } - const promise = fetch(url, options).then(async response => { - const {status} = response; - const body = await response.json(); - return {status, body}; - }, err => { - if (err.name === "AbortError") { - throw new AbortError(); - } else if (err instanceof TypeError) { - // Network errors are reported as TypeErrors, see - // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#Checking_that_the_fetch_was_successful - // this can either mean user is offline, server is offline, or a CORS error (server misconfiguration). - // - // One could check navigator.onLine to rule out the first - // but the 2 latter ones are indistinguishable from javascript. - throw new ConnectionError(`${options.method} ${url}: ${err.message}`); + const promise = fetch(url, options).then(async response => { + const {status} = response; + const body = await response.json(); + return {status, body}; + }, err => { + if (err.name === "AbortError") { + throw new AbortError(); + } else if (err instanceof TypeError) { + // Network errors are reported as TypeErrors, see + // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#Checking_that_the_fetch_was_successful + // this can either mean user is offline, server is offline, or a CORS error (server misconfiguration). + // + // One could check navigator.onLine to rule out the first + // but the 2 latter ones are indistinguishable from javascript. + throw new ConnectionError(`${options.method} ${url}: ${err.message}`); + } + throw err; + }); + const result = new RequestResult(promise, controller); + + if (options.timeout) { + result.promise = abortOnTimeout(createTimeout, options.timeout, result, result.promise); } - throw err; - }); - return new RequestResult(promise, controller); + + return result; + } } diff --git a/src/matrix/net/timeout.js b/src/matrix/net/timeout.js new file mode 100644 index 00000000..463497a5 --- /dev/null +++ b/src/matrix/net/timeout.js @@ -0,0 +1,28 @@ +export function abortOnTimeout(createTimeout, timeoutAmount, requestResult, responsePromise) { + const timeout = createTimeout(timeoutAmount); + // abort request if timeout finishes first + let timedOut = false; + timeout.elapsed().then( + () => { + timedOut = true; + requestResult.abort(); + }, + () => {} // ignore AbortError when timeout is aborted + ); + // abort timeout if request finishes first + return responsePromise.then( + response => { + timeout.abort(); + return response; + }, + err => { + timeout.abort(); + // map error to TimeoutError + if (err instanceof AbortError && timedOut) { + throw new ConnectionError(`Request timed out after ${timeoutAmount}ms`, true); + } else { + throw err; + } + } + ); +} \ No newline at end of file