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

HTTP.request() improvements

Browse source 
- Move 5xx retries and connection checking out of the sync API client
  and into HTTP.request() so that they apply to all requests. 429 handling
  remains in the API client, since not all callers necessarily want to
  handle that the same way. Callers can still handle 5xx themselves by
  including the relevant 5xx status codes in `successCodes` or by passing
  `errorDelayMax: 0`.
- Add `cancellerReceiver` option, which is a callback that receives a
  function that will cancel the request, whether it's an active request
  or an automatic delay before a 5xx retry.

This also updates Sinon to 7.3.2.
This commit is contained in:
Dan Stillman 2019-04-23 19:49:48 -04:00
parent e25786e74c
commit dc60e5f840
7 changed files with 451 additions and 223 deletions
Download patch file Download diff file Expand all files Collapse all files

124
test/tests/syncAPIClientTest.js
View file

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