From f5b41aaf0fa9a5b5cec67c1c633814c4fc1aeb5e Mon Sep 17 00:00:00 2001 From: Kevin Fiol Date: Wed, 23 Dec 2020 05:42:03 -0500 Subject: [PATCH] Reject request on XHR timeout (#2646) Co-authored-by: Zachary Hamm --- request/request.js | 34 +++++++++++++++----- request/tests/test-request.js | 51 ++++++++++++++++++++++++++++++ test-utils/xhrMock.js | 58 +++++++++++++++++++++++++---------- 3 files changed, 120 insertions(+), 23 deletions(-) diff --git a/request/request.js b/request/request.js index e98cd9d1d..648bb1755 100644 --- a/request/request.js +++ b/request/request.js @@ -79,7 +79,7 @@ module.exports = function($window, Promise, oncompletion) { var assumeJSON = (args.serialize == null || args.serialize === JSON.serialize) && !(body instanceof $window.FormData) var responseType = args.responseType || (typeof args.extract === "function" ? "" : "json") - var xhr = new $window.XMLHttpRequest(), aborted = false + var xhr = new $window.XMLHttpRequest(), aborted = false, isTimeout = false var original = xhr, replacedAbort var abort = xhr.abort @@ -141,12 +141,25 @@ module.exports = function($window, Promise, oncompletion) { } if (success) resolve(response) else { - try { message = ev.target.responseText } - catch (e) { message = response } - var error = new Error(message) - error.code = ev.target.status - error.response = response - reject(error) + var completeErrorResponse = function() { + try { message = ev.target.responseText } + catch (e) { message = response } + var error = new Error(message) + error.code = ev.target.status + error.response = response + reject(error) + } + + if (xhr.status === 0) { + // Use setTimeout to push this code block onto the event queue + // This allows `xhr.ontimeout` to run in the case that there is a timeout + // Without this setTimeout, `xhr.ontimeout` doesn't have a chance to reject + // as `xhr.onreadystatechange` will run before it + setTimeout(function() { + if (isTimeout) return + completeErrorResponse() + }) + } else completeErrorResponse() } } catch (e) { @@ -155,6 +168,13 @@ module.exports = function($window, Promise, oncompletion) { } } + xhr.ontimeout = function (ev) { + isTimeout = true + var error = new Error("Request timed out") + error.code = ev.target.status + reject(error) + } + if (typeof args.config === "function") { xhr = args.config(xhr, args, url) || xhr diff --git a/request/tests/test-request.js b/request/tests/test-request.js index 51e8b0b36..7ff0f2fca 100644 --- a/request/tests/test-request.js +++ b/request/tests/test-request.js @@ -677,6 +677,57 @@ o.spec("request", function() { o(e instanceof Error).equals(true) }).then(done) }) + o("rejects on request timeout", function(done) { + var timeout = 50 + var timeToGetItem = timeout + 1 + + mock.$defineRoutes({ + "GET /item": function() { + return new Promise(function(resolve) { + setTimeout(function() { + resolve({status: 200}) + }, timeToGetItem) + }) + } + }) + + request({ + method: "GET", url: "/item", + timeout: timeout + }).catch(function(e) { + o(e instanceof Error).equals(true) + o(e.message).equals("Request timed out") + o(e.code).equals(0) + }).then(function() { + done() + }) + }) + o("does not reject when time to request resource does not exceed timeout", function(done) { + var timeout = 50 + var timeToGetItem = timeout - 1 + var isRequestRejected = false + + mock.$defineRoutes({ + "GET /item": function() { + return new Promise(function(resolve) { + setTimeout(function() { + resolve({status: 200}) + }, timeToGetItem) + }) + } + }) + + request({ + method: "GET", url: "/item", + timeout: timeout + }).catch(function(e) { + isRequestRejected = true + o(e.message).notEquals("Request timed out") + }).then(function() { + o(isRequestRejected).equals(false) + done() + }) + }) o("does not reject on status error code when extract provided", function(done) { mock.$defineRoutes({ "GET /item": function() { diff --git a/test-utils/xhrMock.js b/test-utils/xhrMock.js index 55c74606f..eefbf728e 100644 --- a/test-utils/xhrMock.js +++ b/test-utils/xhrMock.js @@ -46,6 +46,7 @@ module.exports = function() { } this.responseType = "" this.response = null + this.timeout = 0 Object.defineProperty(this, "responseText", {get: function() { if (this.responseType === "" || this.responseType === "text") { return this.response @@ -55,25 +56,50 @@ module.exports = function() { }}) this.send = function(body) { var self = this - if(!aborted) { - var handler = routes[args.method + " " + args.pathname] || serverErrorHandler.bind(null, args.pathname) - var data = handler({rawUrl: args.rawUrl, url: args.pathname, query: args.search || {}, body: body || null}) - self.status = data.status - // Match spec - if (self.responseType === "json") { - try { self.response = JSON.parse(data.responseText) } - catch (e) { /* ignore */ } + + var completeResponse = function (data) { + self._responseCompleted = true + if(!aborted) { + self.status = data.status + // Match spec + if (self.responseType === "json") { + try { self.response = JSON.parse(data.responseText) } + catch (e) { /* ignore */ } + } else { + self.response = data.responseText + } } else { - self.response = data.responseText + self.status = 0 + } + self.readyState = 4 + if (args.async === true) { + callAsync(function() { + if (typeof self.onreadystatechange === "function") self.onreadystatechange({target: self}) + }) } - } else { - self.status = 0 } - self.readyState = 4 - if (args.async === true) { - callAsync(function() { - if (typeof self.onreadystatechange === "function") self.onreadystatechange({target: self}) - }) + + var data + if (!aborted) { + var handler = routes[args.method + " " + args.pathname] || serverErrorHandler.bind(null, args.pathname) + data = handler({rawUrl: args.rawUrl, url: args.pathname, query: args.search || {}, body: body || null}) + } + + if (typeof self.timeout === "number" && self.timeout > 0) { + setTimeout(function () { + if (self._responseCompleted) { + return + } + + self.status = 0; + if (typeof self.ontimeout === "function") self.ontimeout({target: self, type:"timeout"}) + }, self.timeout) + } + + if (data instanceof Promise) { + data.then(completeResponse) + } else { + completeResponse(data) } } this.abort = function() {