From 9071805bdc35b3944aa23c0125d685306e9c54bd Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Sep 2026 13:42:37 -0400 Subject: [PATCH] Shorten the default HTTP retry window An hour of retries makes sense for syncing, which runs automatically and can just spin during server maintenance rather than showing errors that send people to the forums, but it was also inherited by foreground requests, where it stalled operations the user was waiting on. Sync and file syncing now ask for the long window explicitly. --- chrome/content/zotero/xpcom/http.js | 9 +++++---- chrome/content/zotero/xpcom/storage/zfs.js | 11 ++++++++++- chrome/content/zotero/xpcom/sync/syncAPIClient.js | 8 +++++++- test/tests/httpTest.js | 2 +- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/chrome/content/zotero/xpcom/http.js b/chrome/content/zotero/xpcom/http.js index e1ad399350..d00802e73c 100644 --- a/chrome/content/zotero/xpcom/http.js +++ b/chrome/content/zotero/xpcom/http.js @@ -5,7 +5,7 @@ Zotero.HTTP = new function () { this.disableErrorRetry = false; var _errorDelayIntervals = [2500, 5000, 10000, 20000, 40000, 60000, 120000, 240000, 300000]; - var _errorDelayMax = 60 * 60 * 1000; // 1 hour + var _errorDelayMax = 7500; var { SecurityInfo } = ChromeUtils.importESModule("resource://gre/modules/SecurityInfo.sys.mjs"); var { NetUtil } = ChromeUtils.importESModule("resource://gre/modules/NetUtil.sys.mjs"); @@ -208,8 +208,9 @@ Zotero.HTTP = new function () { * for no timeout * @param {Number[]} [options.errorDelayIntervals] - Array of milliseconds to wait before * retrying after 429/5xx error; if unspecified, a default set is used - * @param {Number} [options.errorDelayMax = 3600000] - Milliseconds to wait before stopping - * 429/5xx retries; set to 0 to disable retrying + * @param {Number} [options.errorDelayMax = 7500] - Milliseconds to wait before stopping + * 429/5xx retries; set to 0 to disable retrying. Background operations such as + * syncing set a much longer value. * @param {Boolean} [options.noRetryOnThrottle] - Don't retry on 429 or 503 with a * Retry-After header; instead, throw UnexpectedStatusException to the caller so it * can apply its own throttling (e.g., the sync API client, which pauses an entire @@ -630,7 +631,7 @@ Zotero.HTTP = new function () { * @param {Number} [options.timeout = 30000] - Timeout in milliseconds (connect and * inactivity); 0 to disable * @param {Number[]} [options.errorDelayIntervals] - Retry delay intervals for 5xx errors - * @param {Number} [options.errorDelayMax] - Max time to spend retrying 5xx errors + * @param {Number} [options.errorDelayMax = 7500] - Max time to spend retrying 5xx errors * @return {Promise} - A promise for a fetch Response object */ this.download = async function (uri, path, options = {}) { diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js index 970d3c579f..8d27d8301e 100644 --- a/chrome/content/zotero/xpcom/storage/zfs.js +++ b/chrome/content/zotero/xpcom/storage/zfs.js @@ -42,6 +42,10 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { name: "ZFS", verified: true, + // File syncing runs in the background along with data syncing, so keep + // retrying through server maintenance rather than showing an error + ERROR_DELAY_MAX: 60 * 60 * 1000, + /** * Begin download process for individual file @@ -90,6 +94,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { successCodes: [200, 302, 404], headers: this.apiClient.getHeaders(), noCache: true, + errorDelayMax: this.ERROR_DELAY_MAX, followRedirects: false, } ); @@ -164,6 +169,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { { displayURL, noCache: true, + errorDelayMax: this.ERROR_DELAY_MAX, onProgress(progress, progressMax) { request.onProgress(progress, progressMax); }, @@ -297,7 +303,9 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { var params = this._getRequestParams(libraryID, "removestoragefiles"); var uri = this.apiClient.buildRequestURI(params); - await Zotero.HTTP.request("POST", uri, ""); + await Zotero.HTTP.request("POST", uri, { + errorDelayMax: this.ERROR_DELAY_MAX + }); var sql = "DELETE FROM settings WHERE setting=? AND key=?"; await Zotero.DB.queryAsync(sql, ['storage', 'zfsPurge']); @@ -627,6 +635,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { headers: { "Content-Type": params.contentType }, + errorDelayMax: this.ERROR_DELAY_MAX, body: blob, requestObserver: function (req) { request.setChannel(req.channel); diff --git a/chrome/content/zotero/xpcom/sync/syncAPIClient.js b/chrome/content/zotero/xpcom/sync/syncAPIClient.js index 5931e41d90..2215113090 100644 --- a/chrome/content/zotero/xpcom/sync/syncAPIClient.js +++ b/chrome/content/zotero/xpcom/sync/syncAPIClient.js @@ -48,6 +48,10 @@ Zotero.Sync.APIClient.prototype = { MAX_OBJECTS_PER_REQUEST: 100, MIN_GZIP_SIZE: 1000, UPLOAD_TIMEOUT: 120000, + // Sync runs automatically in the background, so keep retrying through + // server maintenance instead of showing an error. If a server problem + // lasts longer than this, something is actually wrong. + ERROR_DELAY_MAX: 60 * 60 * 1000, getKeyInfo: async function (options={}) { @@ -881,7 +885,9 @@ Zotero.Sync.APIClient.prototype = { } } - let opts = {} + let opts = { + errorDelayMax: this.ERROR_DELAY_MAX + }; Object.assign(opts, options); opts.headers = this.getHeaders(options.headers); opts.noCache = !options.cache; diff --git a/test/tests/httpTest.js b/test/tests/httpTest.js index c0465e7b13..43bff76a34 100644 --- a/test/tests/httpTest.js +++ b/test/tests/httpTest.js @@ -428,7 +428,7 @@ describe("Zotero.HTTP", function () { called++; }); spy = sinon.spy(Zotero.HTTP, "_requestInternal"); - await Zotero.HTTP.request("GET", baseURL + "error"); + await Zotero.HTTP.request("GET", baseURL + "error", { errorDelayMax: 20000 }); assert.equal(3, spy.callCount); // DEBUG: Why are these slightly off? assert.approximately(delayStub.args[0][0], 5 * 1000, 5);