Skip to content

Commit

Permalink
Use XHR onprogress to debounce http._request timeout (#392)
Browse files Browse the repository at this point in the history
Instead of just using a timeout to reject ongoing requests, reset the timeout when progress is observed (at least when requests are done from browsers).

This is to fix element-hq/element-web#2737
  • Loading branch information
lukebarnard1 authored and richvdh committed Mar 16, 2017
1 parent 8b4b0e0 commit 2f5da38
Showing 1 changed file with 30 additions and 12 deletions.
42 changes: 30 additions & 12 deletions src/http-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -631,19 +631,26 @@ module.exports.MatrixHttpApi.prototype = {
let timedOut = false;
let req;
const localTimeoutMs = opts.localTimeoutMs || this.opts.localTimeoutMs;
if (localTimeoutMs) {
timeoutId = callbacks.setTimeout(function() {
timedOut = true;
if (req && req.abort) {
req.abort();

const resetTimeout = () => {
if (localTimeoutMs) {
if (timeoutId) {
callbacks.clearTimeout(timeoutId);
}
defer.reject(new module.exports.MatrixError({
error: "Locally timed out waiting for a response",
errcode: "ORG.MATRIX.JSSDK_TIMEOUT",
timeout: localTimeoutMs,
}));
}, localTimeoutMs);
}
timeoutId = callbacks.setTimeout(function() {
timedOut = true;
if (req && req.abort) {
req.abort();
}
defer.reject(new module.exports.MatrixError({
error: "Locally timed out waiting for a response",
errcode: "ORG.MATRIX.JSSDK_TIMEOUT",
timeout: localTimeoutMs,
}));
}, localTimeoutMs);
}
};
resetTimeout();

const reqPromise = defer.promise;

Expand Down Expand Up @@ -679,6 +686,17 @@ module.exports.MatrixHttpApi.prototype = {
handlerFn(err, response, body);
},
);
// This will only work in a browser, where opts.request is the
// `browser-request` import. Currently `request` does not support progress
// updates - see https://github.com/request/request/pull/2346.
// `browser-request` returns an XHRHttpRequest which exposes `onprogress`
if ('onprogress' in req) {
req.onprogress = (e) => {
// Prevent the timeout from rejecting the deferred promise if progress is
// seen with the request
resetTimeout();
};
}
if (req && req.abort) {
// FIXME: This is EVIL, but I can't think of a better way to expose
// abort() operations on underlying HTTP requests :(
Expand Down

0 comments on commit 2f5da38

Please sign in to comment.