Shorten the default HTTP retry window
Some checks are pending
CI / Test () (push) Blocked by required conditions
CI / Test (macOS NFS) (push) Blocked by required conditions
CI / Test (Windows arm64) (push) Blocked by required conditions
CI / Test (Windows x64) (push) Blocked by required conditions
CI / Utilities Tests (push) Waiting to run
CI / Build, Upload (push) Waiting to run
CI / Detect changes (push) Waiting to run

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.
This commit is contained in:
Dan Stillman 2026-09-11 13:42:37 -04:00
parent cdf10bf62b
commit 9071805bdc
4 changed files with 23 additions and 7 deletions

View file

@ -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<Response>} - A promise for a fetch Response object
*/
this.download = async function (uri, path, options = {}) {

View file

@ -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);

View file

@ -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;

View file

@ -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);