fix timeouts not working

and also not being handled in the Reconnector
This commit is contained in:
Bruno Windels 2020-05-06 19:38:33 +02:00
parent 328000059f
commit f8f13f54be
4 changed files with 60 additions and 23 deletions

View file

@ -6,14 +6,20 @@ export class HomeServerError extends Error {
this.statusCode = status; this.statusCode = status;
} }
get isFatal() { get name() {
switch (this.errcode) { return "HomeServerError";
}
} }
} }
export {AbortError} from "../utils/error.js"; export {AbortError} from "../utils/error.js";
export class ConnectionError extends Error { export class ConnectionError extends Error {
constructor(message, isTimeout) {
super(message || "ConnectionError");
this.isTimeout = isTimeout;
}
get name() {
return "ConnectionError";
}
} }

View file

@ -1,12 +1,13 @@
import { import {
HomeServerError, HomeServerError,
ConnectionError, ConnectionError,
AbortError
} from "../error.js"; } from "../error.js";
class RequestWrapper { class RequestWrapper {
constructor(method, url, requestResult) { constructor(method, url, requestResult, responsePromise) {
this._requestResult = requestResult; this._requestResult = requestResult;
this._promise = this._requestResult.response().then(response => { this._promise = responsePromise.then(response => {
// ok? // ok?
if (response.status >= 200 && response.status < 300) { if (response.status >= 200 && response.status < 300) {
return response.body; return response.body;
@ -43,6 +44,35 @@ export class HomeServerApi {
return `${this._homeserver}/_matrix/client/r0${csPath}`; 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;
}
}
);
}
_request(method, url, queryParams, body, options) { _request(method, url, queryParams, body, options) {
const queryString = Object.entries(queryParams || {}) const queryString = Object.entries(queryParams || {})
.filter(([, value]) => value !== undefined) .filter(([, value]) => value !== undefined)
@ -70,23 +100,21 @@ export class HomeServerApi {
body: bodyString, body: bodyString,
}); });
let responsePromise = requestResult.response();
if (options && options.timeout) { if (options && options.timeout) {
const timeout = this._createTimeout(options.timeout); responsePromise = this._abortOnTimeout(
// abort request if timeout finishes first options.timeout,
timeout.elapsed().then( requestResult,
() => requestResult.abort(), responsePromise
() => {} // ignore AbortError
); );
// abort timeout if request finishes first
const abort = () => timeout.abort();
requestResult.response().then(abort, abort);
} }
const wrapper = new RequestWrapper(method, url, requestResult); const wrapper = new RequestWrapper(method, url, requestResult, responsePromise);
if (this._reconnector) { if (this._reconnector) {
wrapper.response().catch(err => { wrapper.response().catch(err => {
if (err instanceof ConnectionError) { if (err.name === "ConnectionError") {
this._reconnector.onRequestFailed(this); this._reconnector.onRequestFailed(this);
} }
}); });

View file

@ -1,6 +1,4 @@
import {createEnum} from "../../utils/enum.js"; import {createEnum} from "../../utils/enum.js";
import {AbortError} from "../../utils/error.js";
import {ConnectionError} from "../error.js"
import {ObservableValue} from "../../observable/ObservableValue.js"; import {ObservableValue} from "../../observable/ObservableValue.js";
export const ConnectionStatus = createEnum( export const ConnectionStatus = createEnum(
@ -84,13 +82,14 @@ export class Reconnector {
while (!this._versionsResponse) { while (!this._versionsResponse) {
try { try {
this._setState(ConnectionStatus.Reconnecting); this._setState(ConnectionStatus.Reconnecting);
// use 10s timeout, because we don't want to be waiting for // use 30s timeout, as a tradeoff between not giving up
// too quickly on a slow server, and not waiting for
// a stale connection when we just came online again // a stale connection when we just came online again
const versionsRequest = hsApi.versions({timeout: 10000}); const versionsRequest = hsApi.versions({timeout: 30000});
this._versionsResponse = await versionsRequest.response(); this._versionsResponse = await versionsRequest.response();
this._setState(ConnectionStatus.Online); this._setState(ConnectionStatus.Online);
} catch (err) { } catch (err) {
if (err instanceof ConnectionError) { if (err.name === "ConnectionError") {
this._setState(ConnectionStatus.Waiting); this._setState(ConnectionStatus.Waiting);
await this._retryDelay.waitForRetry(); await this._retryDelay.waitForRetry();
} else { } else {
@ -104,6 +103,7 @@ export class Reconnector {
import {Clock as MockClock} from "../../mocks/Clock.js"; import {Clock as MockClock} from "../../mocks/Clock.js";
import {ExponentialRetryDelay} from "./ExponentialRetryDelay.js"; import {ExponentialRetryDelay} from "./ExponentialRetryDelay.js";
import {ConnectionError} from "../error.js"
export function tests() { export function tests() {
function createHsApiMock(remainingFailures) { function createHsApiMock(remainingFailures) {

View file

@ -1,2 +1,5 @@
export class AbortError extends Error { export class AbortError extends Error {
} get name() {
return "AbortError";
}
}