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

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 63864f2f86
commit 00b24f85c9
2 changed files with 61 additions and 36 deletions

View file

@ -166,46 +166,48 @@ Zotero.HTTP = new function() {
if (e.xmlhttp.status == 503 && await _checkRetry(e.xmlhttp)) { if (e.xmlhttp.status == 503 && await _checkRetry(e.xmlhttp)) {
continue; continue;
} }
// Don't retry if errorDelayMax is 0
if (options.errorDelayMax === 0) {
throw e;
}
// Automatically retry other 5xx errors by default // Automatically retry other 5xx errors by default
if (options.errorDelayMax !== 0) { if (!errorDelayGenerator) {
if (!errorDelayGenerator) { // Keep trying for up to an hour
// Keep trying for up to an hour errorDelayGenerator = Zotero.Utilities.Internal.delayGenerator(
errorDelayGenerator = Zotero.Utilities.Internal.delayGenerator( options.errorDelayIntervals || _errorDelayIntervals,
options.errorDelayIntervals || _errorDelayIntervals, options.errorDelayMax !== undefined
options.errorDelayMax !== undefined ? options.errorDelayMax
? options.errorDelayMax : _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; catch (e) {
let keepGoing; Zotero.debug("Request cancelled");
// 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; throw e;
} }
resolve();
}
else {
keepGoing = await delayPromise;
}
if (!keepGoing) {
Zotero.logError("Failed too many times");
throw e;
} }
continue; continue;
} }

View file

@ -169,6 +169,29 @@ describe("Zotero.HTTP", function () {
assert.equal(delayStub.args[1][0], 20); 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 () { it("should provide cancellerReceiver a callback to cancel while waiting to retry a 5xx error", async function () {
delayStub.restore(); delayStub.restore();
setResponse({ setResponse({