diff --git a/chrome/content/zotero/xpcom/http.js b/chrome/content/zotero/xpcom/http.js index 56baa55ced..55dd43e73c 100644 --- a/chrome/content/zotero/xpcom/http.js +++ b/chrome/content/zotero/xpcom/http.js @@ -3,13 +3,14 @@ * @namespace */ Zotero.HTTP = new function() { - this.lastGoogleScholarQueryTime = 0; - + var _errorDelayIntervals = [2500, 5000, 10000, 20000, 40000, 60000, 120000, 240000, 300000]; + var _errorDelayMax = 60 * 60 * 1000; // 1 hour + /** * Exception returned for unexpected status when promise* is used * @constructor */ - this.UnexpectedStatusException = function (xmlhttp, url, msg) { + this.UnexpectedStatusException = function (xmlhttp, url, msg, options = {}) { this.xmlhttp = xmlhttp; this.url = url; this.status = xmlhttp.status; @@ -19,6 +20,12 @@ Zotero.HTTP = new function() { this.message = msg; this.stack = new Error().stack; + if (options) { + for (let prop in options) { + this[prop] = options[prop]; + } + } + // Hide password from debug output // // Password also shows up in channel.name (nsIRequest.name), but that's @@ -73,6 +80,12 @@ Zotero.HTTP = new function() { }; this.BrowserOfflineException.prototype = Object.create(Error.prototype); + this.CancelledException = function () { + this.message = "Request cancelled"; + this.stack = new Error().stack; + }; + this.CancelledException.prototype = Object.create(Error.prototype); + this.TimeoutException = function(ms) { this.message = "XMLHttpRequest has timed out after " + ms + "ms"; this.stack = new Error().stack; @@ -115,19 +128,96 @@ Zotero.HTTP = new function() { * @param {Boolean} [options.foreground] - Make a foreground request, showing * certificate/authentication dialogs if necessary * @param {Number} [options.logBodyLength=1024] - Length of request body to log - * @param {Number} [options.timeout] - Request timeout specified in milliseconds * @param {Function} [options.requestObserver] - Callback to receive XMLHttpRequest after open() + * @param {Function} [options.cancellerReceiver] - Callback to receive a function to cancel + * the operation * @param {String} [options.responseType] - The type of the response. See XHR 2 documentation * for legal values * @param {String} [options.responseCharset] - The charset the response should be interpreted as * @param {Number[]|false} [options.successCodes] - HTTP status codes that are considered * successful, or FALSE to allow all * @param {Zotero.CookieSandbox} [options.cookieSandbox] - Cookie sandbox object + * @param {Number} [options.timeout = 30000] - Request timeout specified in milliseconds + * @param {Number[]} [options.errorDelayIntervals] - Array of milliseconds to wait before + * retrying after 5xx error; if unspecified, a default set is used + * @param {Number} [options.errorDelayMax = 3600000] - Milliseconds to wait before stopping + * 5xx retries; set to 0 to disable retrying * @return {Promise} - A promise resolved with the XMLHttpRequest object if the * request succeeds or rejected if the browser is offline or a non-2XX status response * code is received (or a code not in options.successCodes if provided). */ - this.request = Zotero.Promise.coroutine(function* (method, url, options = {}) { + this.request = async function (method, url, options = {}) { + var errorDelayGenerator; + + while (true) { + try { + let req = await this._requestInternal(...arguments); + return req; + } + catch (e) { + if (e instanceof this.UnexpectedStatusException) { + _checkConnection(e.xmlhttp, url); + + if (e.is5xx()) { + Zotero.logError(e); + // Check for Retry-After header on 503 and wait the specified amount of time + if (e.xmlhttp.status == 503 && await _checkRetry(e.xmlhttp)) { + continue; + } + // Automatically retry other 5xx errors by default + if (options.errorDelayMax !== 0) { + if (!errorDelayGenerator) { + // Keep trying for up to an hour + errorDelayGenerator = Zotero.Utilities.Internal.delayGenerator( + options.errorDelayIntervals || _errorDelayIntervals, + options.errorDelayMax !== undefined + ? options.errorDelayMax + : _errorDelayMax + ); + } + let delayPromise = errorDelayGenerator.next().value; + let keepGoing; + // Provide caller with a callback to cancel while waiting to retry + if (options.cancellerReceiver) { + let resolve; + let reject; + let cancelPromise = new Zotero.Promise((res, rej) => { + resolve = res; + reject = function () { + rej(new Zotero.HTTP.CancelledException); + }; + }); + options.cancellerReceiver(reject); + try { + keepGoing = await Promise.race([delayPromise, cancelPromise]); + } + catch (e) { + Zotero.debug("Request cancelled"); + throw e; + } + resolve(); + } + else { + keepGoing = await delayPromise; + } + if (!keepGoing) { + Zotero.logError("Failed too many times"); + throw e; + } + } + continue; + } + } + throw e; + } + } + }; + + + /** + * Most of the logic for request() is here, with request() handling automatic 5xx retries + */ + this._requestInternal = async function (method, url, options = {}) { if (url instanceof Components.interfaces.nsIURI) { // Extract username and password from URI and undo Mozilla's excessive percent-encoding options.username = url.username || null; @@ -264,7 +354,7 @@ Zotero.HTTP = new function() { if (options.compressBody && this.isWriteMethod(method)) { headers['Content-Encoding'] = 'gzip'; - compressedBody = yield Zotero.Utilities.Internal.gzip(options.body); + compressedBody = await Zotero.Utilities.Internal.gzip(options.body); let oldLen = options.body.length; let newLen = compressedBody.length; @@ -297,7 +387,19 @@ Zotero.HTTP = new function() { xmlhttp.ontimeout = function() { deferred.reject(new Zotero.HTTP.TimeoutException(options.timeout)); }; - + + // Provide caller with a callback to cancel a request in progress + if (options.cancellerReceiver) { + options.cancellerReceiver(() => { + if (xmlhttp.readyState == 4) { + Zotero.debug("Request already finished -- not cancelling"); + return; + } + deferred.reject(new Zotero.HTTP.CancelledException); + xmlhttp.abort(); + }); + } + xmlhttp.onloadend = async function() { var status = xmlhttp.status || redirectStatus; @@ -428,7 +530,7 @@ Zotero.HTTP = new function() { } return deferred.promise; - }); + }; /** * Send an HTTP GET request via XMLHTTPRequest @@ -1119,6 +1221,34 @@ Zotero.HTTP = new function() { } } + /** + * Check connection for interruption and throw an appropriate error + */ + function _checkConnection(xmlhttp, url) { + if (xmlhttp.status != 0) return; + + var msg = null; + var dialogButtonText = null; + var dialogButtonCallback = null; + + if (xmlhttp.status === 0) { + msg = Zotero.getString('sync.error.checkConnection'); + dialogButtonText = Zotero.getString('general.moreInformation'); + let supportURL = 'https://www.zotero.org/support/kb/connection_error'; + dialogButtonCallback = () => Zotero.launchURL(supportURL); + } + throw new Zotero.HTTP.UnexpectedStatusException( + xmlhttp, + url, + msg, + { + dialogButtonText, + dialogButtonCallback + } + ); + } + + this.checkSecurity = function (channel) { if (!channel) { return; @@ -1160,7 +1290,23 @@ Zotero.HTTP = new function() { } } } - + + + async function _checkRetry(req) { + var retryAfter = req.getResponseHeader("Retry-After"); + if (!retryAfter) { + return false; + } + if (parseInt(retryAfter) != retryAfter) { + Zotero.logError(`Invalid Retry-After delay ${retryAfter}`); + return false; + } + Zotero.debug(`Delaying ${retryAfter} seconds for Retry-After`); + await Zotero.Promise.delay(retryAfter * 1000); + return true; + } + + /** * Mimics the window.location/document.location interface, given an nsIURL * @param {nsIURL} url diff --git a/chrome/content/zotero/xpcom/sync/syncAPIClient.js b/chrome/content/zotero/xpcom/sync/syncAPIClient.js index 266514e276..5c223e6bd7 100644 --- a/chrome/content/zotero/xpcom/sync/syncAPIClient.js +++ b/chrome/content/zotero/xpcom/sync/syncAPIClient.js @@ -38,8 +38,6 @@ Zotero.Sync.APIClient = function (options) { this.caller = options.caller; this.debugUploadPolicy = Zotero.Prefs.get('sync.debugUploadPolicy'); - this.failureDelayIntervals = [2500, 5000, 10000, 20000, 40000, 60000, 120000, 240000, 300000]; - this.failureDelayMax = 60 * 60 * 1000; // 1 hour this.rateDelayIntervals = [30, 60, 300]; this.rateDelayPosition = 0; } @@ -610,7 +608,7 @@ Zotero.Sync.APIClient.prototype = { getHeaders: function (headers = {}) { let newHeaders = {}; newHeaders = Object.assign(newHeaders, headers); - newHeaders["Zotero-API-Version"] = this.apiVersion; + newHeaders["Zotero-API-Version"] = this.apiVersion.toString(); if (this.apiKey) { newHeaders["Zotero-API-Key"] = this.apiKey; } @@ -651,7 +649,6 @@ Zotero.Sync.APIClient.prototype = { } var tries = 0; - var failureDelayGenerator = null; while (true) { var result = yield this.caller.start(Zotero.Promise.coroutine(function* () { try { @@ -663,31 +660,10 @@ Zotero.Sync.APIClient.prototype = { catch (e) { tries++; if (e instanceof Zotero.HTTP.UnexpectedStatusException) { - this._checkConnection(e.xmlhttp, e.channel); if (this._check429(e.xmlhttp)) { // Return false to keep retrying request return false; } - - if (e.is5xx()) { - Zotero.logError(e); - if (e.xmlhttp.status == 503 && this._checkRetry(e.xmlhttp)) { - return false; - } - - if (!failureDelayGenerator) { - // Keep trying for up to an hour - failureDelayGenerator = Zotero.Utilities.Internal.delayGenerator( - this.failureDelayIntervals, this.failureDelayMax - ); - } - let keepGoing = yield failureDelayGenerator.next().value; - if (!keepGoing) { - Zotero.logError("Failed too many times"); - throw e; - } - return false; - } } else if (e instanceof Zotero.HTTP.BrowserOfflineException) { e.fatal = true; @@ -787,37 +763,6 @@ Zotero.Sync.APIClient.prototype = { }, - /** - * Check connection for interruptions and empty responses and throw an appropriate error - */ - _checkConnection: function (xmlhttp, channel) { - if (!xmlhttp.responseText && (xmlhttp.status == 0 || xmlhttp.status == 200)) { - let msg = null; - let dialogButtonText = null; - let dialogButtonCallback = null; - - if (xmlhttp.status === 0) { - msg = Zotero.getString('sync.error.checkConnection'); - dialogButtonText = Zotero.getString('general.moreInformation'); - let url = 'https://www.zotero.org/support/kb/connection_error'; - dialogButtonCallback = () => Zotero.launchURL(url); - } - else if (!msg) { - msg = Zotero.getString('sync.error.emptyResponseServer') - + Zotero.getString('general.tryAgainLater'); - } - throw new Zotero.Error( - msg, - 0, - { - dialogButtonText, - dialogButtonCallback - } - ); - } - }, - - _checkBackoff: function (xmlhttp) { var backoff = xmlhttp.getResponseHeader("Backoff"); if (backoff && parseInt(backoff) == backoff) { diff --git a/package-lock.json b/package-lock.json index d5422d1407..bcaab4ffbe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -734,15 +734,50 @@ } } }, - "@sinonjs/formatio": { - "version": "2.0.0", - "resolved": "http://registry.npmjs.org/@sinonjs/formatio/-/formatio-2.0.0.tgz", - "integrity": "sha512-ls6CAMA6/5gG+O/IdsBcblvnd8qcO/l1TYoNeAzp3wcISOxlPXQEus0mLcdwazEkWjaBdaJ3TaxmNgCLWwvWzg==", + "@sinonjs/commons": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.4.0.tgz", + "integrity": "sha512-9jHK3YF/8HtJ9wCAbG+j8cD0i0+ATS9A7gXFqS36TblLPNy6rEEc+SB0imo91eCboGaBYGV/MT1/br/J+EE7Tw==", "dev": true, "requires": { - "samsam": "1.3.0" + "type-detect": "4.0.8" + }, + "dependencies": { + "type-detect": { + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.0.8.tgz", + "integrity": "sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==", + "dev": true + } } }, + "@sinonjs/formatio": { + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-3.2.1.tgz", + "integrity": "sha512-tsHvOB24rvyvV2+zKMmPkZ7dXX6LSLKZ7aOtXY6Edklp0uRcgGpOsQTTGTcWViFyx4uhWc6GV8QdnALbIbIdeQ==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1", + "@sinonjs/samsam": "^3.1.0" + } + }, + "@sinonjs/samsam": { + "version": "3.3.1", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-3.3.1.tgz", + "integrity": "sha512-wRSfmyd81swH0hA1bxJZJ57xr22kC07a1N4zuIL47yTS04bDk6AoCkczcqHEjcRPmJ+FruGJ9WBQiJwMtIElFw==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.0.2", + "array-from": "^2.1.1", + "lodash": "^4.17.11" + } + }, + "@sinonjs/text-encoding": { + "version": "0.7.1", + "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz", + "integrity": "sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==", + "dev": true + }, "@zotero/eslint-config": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/@zotero/eslint-config/-/eslint-config-1.0.3.tgz", @@ -895,6 +930,12 @@ "integrity": "sha1-3wEKoSh+Fku9pvlyOwqWoexBh6E=", "dev": true }, + "array-from": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/array-from/-/array-from-2.1.1.tgz", + "integrity": "sha1-z+nYwmYoudxa7MYqn12PHzUsEZU=", + "dev": true + }, "array-includes": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/array-includes/-/array-includes-3.0.3.tgz", @@ -4340,9 +4381,9 @@ } }, "just-extend": { - "version": "1.1.27", - "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-1.1.27.tgz", - "integrity": "sha512-mJVp13Ix6gFo3SBAy9U/kL+oeZqzlYYYLQBwXVBlVzIsZwBqGREnOro24oC/8s8aox+rJhtZ2DiQof++IrkA+g==", + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.0.2.tgz", + "integrity": "sha512-FrLwOgm+iXrPV+5zDU6Jqu4gCRXbWEQg2O3SKONsWE4w7AXFRkryS53bpWdaL9cNol+AmR3AEYz6kn+o0fCPnw==", "dev": true }, "kind-of": { @@ -4462,12 +4503,6 @@ "lodash._isiterateecall": "^3.0.0" } }, - "lodash.get": { - "version": "4.4.2", - "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz", - "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=", - "dev": true - }, "lodash.isarguments": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/lodash.isarguments/-/lodash.isarguments-3.1.0.tgz", @@ -4498,9 +4533,9 @@ "dev": true }, "lolex": { - "version": "2.3.2", - "resolved": "https://registry.npmjs.org/lolex/-/lolex-2.3.2.tgz", - "integrity": "sha512-A5pN2tkFj7H0dGIAM6MFvHKMJcPnjZsOMvR7ujCjfgW5TbV6H9vb1PgxLtHvjqNZTHsUolz+6/WEO0N1xNx2ng==", + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/lolex/-/lolex-4.0.1.tgz", + "integrity": "sha512-UHuOBZ5jjsKuzbB/gRNNW8Vg8f00Emgskdq2kvZxgBJCS0aqquAuXai/SkWORlKeZEiNQWZjFZOqIUcH9LqKCw==", "dev": true }, "loose-envify": { @@ -4849,16 +4884,24 @@ "dev": true }, "nise": { - "version": "1.3.2", - "resolved": "https://registry.npmjs.org/nise/-/nise-1.3.2.tgz", - "integrity": "sha512-KPKb+wvETBiwb4eTwtR/OsA2+iijXP+VnlSFYJo3EHjm2yjek1NWxHOUQat3i7xNLm1Bm18UA5j5Wor0yO2GtA==", + "version": "1.4.10", + "resolved": "https://registry.npmjs.org/nise/-/nise-1.4.10.tgz", + "integrity": "sha512-sa0RRbj53dovjc7wombHmVli9ZihXbXCQ2uH3TNm03DyvOSIQbxg+pbqDKrk2oxMK1rtLGVlKxcB9rrc6X5YjA==", "dev": true, "requires": { - "@sinonjs/formatio": "^2.0.0", - "just-extend": "^1.1.27", + "@sinonjs/formatio": "^3.1.0", + "@sinonjs/text-encoding": "^0.7.1", + "just-extend": "^4.0.2", "lolex": "^2.3.2", - "path-to-regexp": "^1.7.0", - "text-encoding": "^0.6.4" + "path-to-regexp": "^1.7.0" + }, + "dependencies": { + "lolex": { + "version": "2.7.5", + "resolved": "https://registry.npmjs.org/lolex/-/lolex-2.7.5.tgz", + "integrity": "sha512-l9x0+1offnKKIzYVjyXU2SiwhXDLekRzKyhnbyldPHvC7BvLPVpdNUNR2KeMAiCN2D/kLNttZgQD5WjSxuBx3Q==", + "dev": true + } } }, "node-gyp": { @@ -5754,12 +5797,6 @@ "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==", "dev": true }, - "samsam": { - "version": "1.3.0", - "resolved": "https://registry.npmjs.org/samsam/-/samsam-1.3.0.tgz", - "integrity": "sha512-1HwIYD/8UlOtFS3QO3w7ey+SdSDFE4HRNLZoZRYVQefrOY3l17epswImeB1ijgJFQJodIaHcwkp3r/myBjFVbg==", - "dev": true - }, "sass-graph": { "version": "2.2.4", "resolved": "https://registry.npmjs.org/sass-graph/-/sass-graph-2.2.4.tgz", @@ -5891,20 +5928,26 @@ "dev": true }, "sinon": { - "version": "4.5.0", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-4.5.0.tgz", - "integrity": "sha512-trdx+mB0VBBgoYucy6a9L7/jfQOmvGeaKZT4OOJ+lPAtI8623xyGr8wLiE4eojzBS8G9yXbhx42GHUOVLr4X2w==", + "version": "7.3.2", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-7.3.2.tgz", + "integrity": "sha512-thErC1z64BeyGiPvF8aoSg0LEnptSaWE7YhdWWbWXgelOyThent7uKOnnEh9zBxDbKixtr5dEko+ws1sZMuFMA==", "dev": true, "requires": { - "@sinonjs/formatio": "^2.0.0", - "diff": "^3.1.0", - "lodash.get": "^4.4.2", - "lolex": "^2.2.0", - "nise": "^1.2.0", - "supports-color": "^5.1.0", - "type-detect": "^4.0.5" + "@sinonjs/commons": "^1.4.0", + "@sinonjs/formatio": "^3.2.1", + "@sinonjs/samsam": "^3.3.1", + "diff": "^3.5.0", + "lolex": "^4.0.1", + "nise": "^1.4.10", + "supports-color": "^5.5.0" }, "dependencies": { + "diff": { + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-3.5.0.tgz", + "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA==", + "dev": true + }, "has-flag": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", @@ -5912,19 +5955,13 @@ "dev": true }, "supports-color": { - "version": "5.3.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.3.0.tgz", - "integrity": "sha512-0aP01LLIskjKs3lq52EC0aGBAJhLq7B2Rd8HC/DR/PtNNpcLilNmHC12O+hu0usQpo7wtHNRqtrhBwtDb0+dNg==", + "version": "5.5.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.5.0.tgz", + "integrity": "sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==", "dev": true, "requires": { "has-flag": "^3.0.0" } - }, - "type-detect": { - "version": "4.0.8", - "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.0.8.tgz", - "integrity": "sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==", - "dev": true } } }, @@ -6387,12 +6424,6 @@ "inherits": "2" } }, - "text-encoding": { - "version": "0.6.4", - "resolved": "https://registry.npmjs.org/text-encoding/-/text-encoding-0.6.4.tgz", - "integrity": "sha1-45mpgiV6J22uQou5KEXLcb3CbRk=", - "dev": true - }, "text-table": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/text-table/-/text-table-0.2.0.tgz", diff --git a/package.json b/package.json index 4faf8096ae..bc82fa0a60 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "multimatch": "^2.1.0", "node-sass": "^4.12.0", "react-virtualized": "^9.21.0", - "sinon": "^4.5.0", + "sinon": "^7.3.2", "universalify": "^0.1.1" } } diff --git a/test/content/support.js b/test/content/support.js index 033fc301f4..5ce1685e99 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -499,8 +499,14 @@ var modifyDataObject = function (obj, params = {}, saveOptions) { /** * Return a promise for the error thrown by a promise, or false if none */ -function getPromiseError(promise) { - return promise.thenReturn(false).catch(e => e); +async function getPromiseError(promise) { + try { + await promise; + } + catch (e) { + return e; + } + return false; } /** diff --git a/test/tests/httpTest.js b/test/tests/httpTest.js index 7fee7963d1..18e56d6610 100644 --- a/test/tests/httpTest.js +++ b/test/tests/httpTest.js @@ -1,13 +1,19 @@ describe("Zotero.HTTP", function () { + var server; var httpd; var port = 16213; var baseURL = `http://127.0.0.1:${port}/` var testURL = baseURL + 'test.html'; var redirectLocation = baseURL + 'test2.html'; + function setResponse(response) { + setHTTPResponse(server, baseURL, response, {}); + } + + before(function* () { + // Real HTTP server Components.utils.import("resource://zotero-unit/httpd.js"); - httpd = new HttpServer(); httpd.start(port); httpd.registerPathHandler( @@ -41,15 +47,25 @@ describe("Zotero.HTTP", function () { ); }); + beforeEach(function () { + // Fake XHR, which can be disabled per test with `Zotero.HTTP.mock = null` + Zotero.HTTP.mock = sinon.FakeXMLHttpRequest; + server = sinon.fakeServer.create(); + server.autoRespond = true; + }); + after(function* () { var defer = new Zotero.Promise.defer(); httpd.stop(() => defer.resolve()); yield defer.promise; + + Zotero.HTTP.mock = null; }); describe("#request()", function () { it("should succeed with 3xx status if followRedirects is false", async function () { + Zotero.HTTP.mock = null; var req = await Zotero.HTTP.request( 'GET', baseURL + 'redirect', @@ -60,10 +76,168 @@ describe("Zotero.HTTP", function () { assert.equal(req.status, 301); assert.equal(req.getResponseHeader('Location'), redirectLocation); }); + + it("should catch an interrupted connection", async function () { + setResponse({ + method: "GET", + url: "empty", + status: 0, + text: "" + }) + var e = await getPromiseError(Zotero.HTTP.request("GET", baseURL + "empty")); + assert.ok(e); + assert.equal(e.message, Zotero.getString('sync.error.checkConnection')); + }); + + it("should provide cancellerReceiver with a callback to cancel a request", async function () { + var timeoutID; + server.autoRespond = false; + server.respondWith(function (req) { + if (req.method == "GET" && req.url == baseURL + "slow") { + req.respond( + 200, + {}, + "OK" + ); + } + }); + + setTimeout(function () { + cancel(); + }, 50); + var e = await getPromiseError(Zotero.HTTP.request( + "GET", + baseURL + "slow", + { + cancellerReceiver: function (f) { + cancel = f; + } + } + )); + + assert.instanceOf(e, Zotero.HTTP.CancelledException); + server.respond(); + }); + + describe("Retries", function () { + var spy; + var delayStub; + + beforeEach(function () { + delayStub = sinon.stub(Zotero.Promise, "delay").returns(Zotero.Promise.resolve()); + }); + + afterEach(function () { + if (spy) { + spy.restore(); + } + delayStub.restore(); + }); + + after(function () { + sinon.restore(); + }); + + + it("should retry on 500 error", async function () { + setResponse({ + method: "GET", + url: "error", + status: 500, + text: "" + }); + spy = sinon.spy(Zotero.HTTP, "_requestInternal"); + var e = await getPromiseError( + Zotero.HTTP.request( + "GET", + baseURL + "error", + { + errorDelayIntervals: [10, 20], + errorDelayMax: 25 + } + ) + ); + assert.instanceOf(e, Zotero.HTTP.UnexpectedStatusException); + assert.isTrue(spy.calledTwice); + }); + + it("should provide cancellerReceiver a callback to cancel while waiting to retry a 5xx error", async function () { + delayStub.restore(); + setResponse({ + method: "GET", + url: "error", + status: 500, + text: "" + }); + var cancel; + spy = sinon.spy(Zotero.HTTP, "_requestInternal"); + setTimeout(() => { + cancel(); + }, 50); + var e = await getPromiseError( + Zotero.HTTP.request( + "GET", + baseURL + "error", + { + errorDelayIntervals: [10, 10, 100], + cancellerReceiver: function () { + cancel = arguments[0]; + } + } + ) + ); + assert.instanceOf(e, Zotero.HTTP.CancelledException); + assert.isTrue(spy.calledTwice); + }); + + it("should obey Retry-After for 503", function* () { + var called = 0; + server.respond(function (req) { + if (req.method == "GET" && req.url == baseURL + "error") { + if (called < 1) { + req.respond( + 503, + { + "Retry-After": "5" + }, + "" + ); + } + else if (called < 2) { + req.respond( + 503, + { + "Retry-After": "10" + }, + "" + ); + } + else { + req.respond( + 200, + {}, + "" + ); + } + } + called++; + }); + spy = sinon.spy(Zotero.HTTP, "_requestInternal"); + yield Zotero.HTTP.request("GET", baseURL + "error"); + assert.isTrue(spy.calledThrice); + // DEBUG: Why are these slightly off? + assert.approximately(delayStub.args[0][0], 5 * 1000, 5); + assert.approximately(delayStub.args[1][0], 10 * 1000, 5); + }); + }); }); describe("#processDocuments()", function () { + beforeEach(function () { + Zotero.HTTP.mock = null; + }); + it("should provide a document object", function* () { var called = false; yield Zotero.HTTP.processDocuments( diff --git a/test/tests/syncAPIClientTest.js b/test/tests/syncAPIClientTest.js index f3a4e9d795..28921ae603 100644 --- a/test/tests/syncAPIClientTest.js +++ b/test/tests/syncAPIClientTest.js @@ -43,20 +43,6 @@ describe("Zotero.Sync.APIClient", function () { Zotero.HTTP.mock = null; }) - describe("#_checkConnection()", function () { - it("should catch an interrupted connection", function* () { - setResponse({ - method: "GET", - url: "empty", - status: 0, - text: "" - }) - var e = yield getPromiseError(client.makeRequest("GET", baseURL + "empty")); - assert.ok(e); - assert.equal(e.message, Zotero.getString('sync.error.checkConnection')); - }) - }) - describe("#getGroups()", function () { it("should automatically fetch multiple pages of results", function* () { function groupJSON(groupID) { @@ -135,8 +121,6 @@ describe("Zotero.Sync.APIClient", function () { }); beforeEach(function () { - client.failureDelayIntervals = [10, 20]; - client.failureDelayMax = 25; client.rateDelayIntervals = [15, 25]; }); @@ -148,95 +132,37 @@ describe("Zotero.Sync.APIClient", function () { }); after(function () { - delayStub.restore(); + sinon.restore(); }); - describe("#makeRequest()", function () { - it("should retry on 500 error", function* () { - setResponse({ - method: "GET", - url: "error", - status: 500, - text: "" - }); - spy = sinon.spy(Zotero.HTTP, "request"); - var e = yield getPromiseError(client.makeRequest("GET", baseURL + "error")); - assert.instanceOf(e, Zotero.HTTP.UnexpectedStatusException); - assert.isTrue(spy.calledTwice); - }); - - it("should obey Retry-After for 503", function* () { - var called = 0; - server.respond(function (req) { - if (req.method == "GET" && req.url == baseURL + "error") { - if (called < 1) { - req.respond( - 503, - { - "Retry-After": "5" - }, - "" - ); - } - else if (called < 2) { - req.respond( - 503, - { - "Retry-After": "10" - }, - "" - ); - } - else { - req.respond( - 200, - {}, - "" - ); - } + it("should retry on 429 error", function* () { + var called = 0; + server.respond(function (req) { + if (req.method == "GET" && req.url == baseURL + "error") { + if (called < 2) { + req.respond( + 429, + {}, + "" + ); } - called++; - }); - spy = sinon.spy(Zotero.HTTP, "request"); - yield client.makeRequest("GET", baseURL + "error"); - assert.isTrue(spy.calledThrice); - // DEBUG: Why are these slightly off? - assert.approximately(delayStub.args[0][0], 5 * 1000, 5); - assert.approximately(delayStub.args[1][0], 10 * 1000, 5); - }); - }); - - - describe("#_check429()", function () { - it("should retry on 429 error", function* () { - var called = 0; - server.respond(function (req) { - if (req.method == "GET" && req.url == baseURL + "error") { - if (called < 2) { - req.respond( - 429, - {}, - "" - ); - } - else { - req.respond( - 200, - {}, - "" - ); - } + else { + req.respond( + 200, + {}, + "" + ); } - called++; - }); - spy = sinon.spy(Zotero.HTTP, "request"); - yield client.makeRequest("GET", baseURL + "error"); - assert.isTrue(spy.calledThrice); - // DEBUG: Why are these slightly off? - assert.approximately(delayStub.args[0][0], 15 * 1000, 5); - assert.approximately(delayStub.args[1][0], 25 * 1000, 5); + } + called++; }); + spy = sinon.spy(Zotero.HTTP, "request"); + yield client.makeRequest("GET", baseURL + "error"); + assert.isTrue(spy.calledThrice); + // DEBUG: Why are these slightly off? + assert.approximately(delayStub.args[0][0], 15 * 1000, 5); + assert.approximately(delayStub.args[1][0], 25 * 1000, 5); }); }); })