mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix HTTP.request() 5xx handling with errorDelayMax=0

Browse source 
This was causing delay-less infinite loops for OPTIONS requests to Box,
which are returning 502 errors.
This commit is contained in:
Dan Stillman 2022-08-29 23:35:06 +02:00
parent c255104ada
commit 91ae576074
2 changed files with 61 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

74
chrome/content/zotero/xpcom/http.js
View file

@ -166,46 +166,48 @@ Zotero.HTTP = new function() {
if (e.xmlhttp.status == 503 && await _checkRetry(e.xmlhttp)) {
continue;
}
// Don't retry if errorDelayMax is 0
if (options.errorDelayMax === 0) {
throw e;
}
// 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
);
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]);
}
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");
catch (e) {
Zotero.debug("Request cancelled");
throw e;
}
resolve();
}
else {
keepGoing = await delayPromise;
}
if (!keepGoing) {
Zotero.logError("Failed too many times");
throw e;
}
continue;
}

23
test/tests/httpTest.js
View file

@ -169,6 +169,29 @@ describe("Zotero.HTTP", function () {
assert.equal(delayStub.args[1][0], 20);
});
it("shouldn't retry on 500 error if errorDelayMax=0", 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, 100],
errorDelayMax: 0
}
)
);
assert.instanceOf(e, Zotero.HTTP.UnexpectedStatusException);
assert.isTrue(spy.calledOnce);
assert.isTrue(delayStub.notCalled);
});
it("should provide cancellerReceiver a callback to cancel while waiting to retry a 5xx error", async function () {
delayStub.restore();
setResponse({