scriptoriumadmin/zotero
1
0
Fork 0
mirror of https://github.com/zotero/zotero.git synced 2026-10-08 03:08:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Don't stall the Find Full Text queue on a failing download

Browse source 
A file URL returning a server error was retried for up to an hour inside
the download, and a 429 or Retry-After was waited out there too. Since
the queue processes one item at a time, that blocked every other
selected item. Throttling now goes to Find Full Text's own per-domain
handling, as it did before 10.0, which also needed to read Retry-After
from a fetch Response and parse HTTP-date values.

https://forums.zotero.org/discussion/133703/
This commit is contained in:
Dan Stillman 2026-09-11 11:27:04 -04:00
parent a6e814417f
commit cdf10bf62b
2 changed files with 190 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
chrome/content/zotero/xpcom/attachments.js
View file

@ -1170,6 +1170,8 @@ Zotero.Attachments = new function () {
* @param {String} [options.referrer]
* @param {Boolean} [options.enforceFileType] - Delete file if not one of SUPPORTED_FILE_TYPES
* @param {Boolean} [options.shouldDisplayCaptcha]
* @param {Number} [options.errorDelayMax] - Passed to Zotero.HTTP.download()
* @param {Boolean} [options.noRetryOnThrottle] - Passed to Zotero.HTTP.download()
*/
this.downloadFile = async function (url, path, options = {}) {
Zotero.debug(`Downloading file from ${url}`);
@ -1184,6 +1186,8 @@ Zotero.Attachments = new function () {
path,
{
headers,
errorDelayMax: options.errorDelayMax,
noRetryOnThrottle: options.noRetryOnThrottle,
}
);
// Check that the downloaded file is the expected type
@ -1733,7 +1737,10 @@ Zotero.Attachments = new function () {
// Retry-After
if (status == 429 || status == 503) {
let retryAfter = e.xmlhttp.getResponseHeader('Retry-After');
// xmlhttp is a fetch Response for downloads
let retryAfter = e.xmlhttp.headers?.get
? e.xmlhttp.headers.get('Retry-After')
: e.xmlhttp.getResponseHeader('Retry-After');
if (retryAfter) {
Zotero.debug("Got Retry-After: " + retryAfter);
if (parseInt(retryAfter) == retryAfter) {
@ -1745,7 +1752,7 @@ Zotero.Attachments = new function () {
return true;
}
else if (Zotero.Date.isHTTPDate(retryAfter)) {
let d = new Date(val);
let d = new Date(retryAfter);
if (d > Date.now() + maxDelay * 1000) {
Zotero.debug("Retry-After is too long -- skipping request");
return false;
@ -2086,7 +2093,14 @@ Zotero.Attachments = new function () {
while (tries-- > 0) {
try {
await beforeRequest(url);
await this.downloadFile(url, path, options);
// Waiting here blocks every other item in the queue, so
// keep it short and leave longer backoff and Retry-After
// to the loop below
await this.downloadFile(
url,
path,
{ ...options, errorDelayMax: 7500, noRetryOnThrottle: true }
);
afterRequest(url);
return { url, props: urlResolver };
}

173
test/tests/attachmentsTest.js
View file

@ -1524,6 +1524,179 @@ describe("Zotero.Attachments", function () {
assert.equal(requests, 1);
});
it("should give up on a file URL that keeps returning a server error", async function () {
var doi = doi4;
var item = createUnsavedDataObject('item', { itemType: 'journalArticle' });
item.setField('title', 'Test');
item.setField('DOI', doi);
await item.saveTx();
var requests = 0;
httpd.registerPathHandler(
"/failing-pdf",
{
handle: function (request, response) {
requests++;
response.setStatusLine(null, 502, "Bad Gateway");
}
}
);
httpd.registerPathHandler(
"/failing-pdf-page/" + doi,
{
handle: function (request, response) {
response.setStatusLine(null, 200, "OK");
response.write(
`<html><body><a id="pdf-link" href="${baseURL}failing-pdf">PDF</a>`
+ `</body></html>`
);
}
}
);
var resolvers = [{
name: 'Custom',
method: 'get',
url: baseURL + "failing-pdf-page/{doi}",
mode: 'html',
selector: '#pdf-link',
attribute: 'href'
}];
Zotero.Prefs.set('findPDFs.resolvers', JSON.stringify(resolvers));
var delayStub = sinon.stub(Zotero.Promise, "delay").returns(Promise.resolve());
try {
var attachment = await Zotero.Attachments.addAvailableFile(item);
}
finally {
delayStub.restore();
}
assert.isFalse(attachment);
// Initial request plus two retries within errorDelayMax
assert.equal(requests, 3);
});
it("should not wait on a Retry-After from a file URL", async function () {
var doi = doi4;
var item = createUnsavedDataObject('item', { itemType: 'journalArticle' });
item.setField('title', 'Test');
item.setField('DOI', doi);
await item.saveTx();
var requests = 0;
httpd.registerPathHandler(
"/throttled-pdf",
{
handle: function (request, response) {
requests++;
if (requests == 1) {
response.setStatusLine(null, 429, "Too Many Requests");
response.setHeader("Retry-After", "1", false);
}
else {
response.setStatusLine(null, 404, "Not Found");
}
}
}
);
httpd.registerPathHandler(
"/throttled-pdf-page/" + doi,
{
handle: function (request, response) {
response.setStatusLine(null, 200, "OK");
response.write(
`<html><body><a id="pdf-link" href="${baseURL}throttled-pdf">PDF</a>`
+ `</body></html>`
);
}
}
);
var resolvers = [{
name: 'Custom',
method: 'get',
url: baseURL + "throttled-pdf-page/{doi}",
mode: 'html',
selector: '#pdf-link',
attribute: 'href'
}];
Zotero.Prefs.set('findPDFs.resolvers', JSON.stringify(resolvers));
var attachment = await Zotero.Attachments.addAvailableFile(item);
assert.isFalse(attachment);
assert.equal(requests, 1);
});
async function testThrottledFileURL(retryAfter) {
var doi = doi4;
var item = createUnsavedDataObject('item', { itemType: 'journalArticle' });
item.setField('title', 'Test');
item.setField('DOI', doi);
await item.saveTx();
var requestTimes = [];
httpd.registerPathHandler(
"/throttled-pdf",
{
handle: function (request, response) {
requestTimes.push(Date.now());
if (requestTimes.length == 1) {
response.setStatusLine(null, 429, "Too Many Requests");
response.setHeader("Retry-After", retryAfter(), false);
}
else {
response.setStatusLine(null, 302, "Found");
response.setHeader("Location", pdfURL, false);
}
}
}
);
httpd.registerPathHandler(
"/throttled-pdf-page/" + doi,
{
handle: function (request, response) {
response.setStatusLine(null, 200, "OK");
response.write(
// Use an address that isn't exempt from per-domain delays
`<html><body><a id="pdf-link" href="http://127.0.0.1:${port}/throttled-pdf">PDF</a>`
+ `</body></html>`
);
}
}
);
var resolvers = [{
name: 'Custom',
method: 'get',
url: baseURL + "throttled-pdf-page/{doi}",
mode: 'html',
selector: '#pdf-link',
attribute: 'href'
}];
Zotero.Prefs.set('findPDFs.resolvers', JSON.stringify(resolvers));
await Zotero.Attachments.addAvailableFiles([item]);
assert.equal(item.numAttachments(), 1);
assert.lengthOf(requestTimes, 2);
return requestTimes[1] - requestTimes[0];
}
it("should retry a file URL after a Retry-After in seconds when finding files for multiple items", async function () {
var elapsed = await testThrottledFileURL(() => "1");
assert.isAtLeast(elapsed, 950);
});
it("should retry a file URL after a Retry-After date when finding files for multiple items", async function () {
var elapsed = await testThrottledFileURL(
() => new Date(Date.now() + 2000).toUTCString()
);
// HTTP dates have one-second resolution
assert.isAtLeast(elapsed, 950);
});
it("should not honor Retry-After from a custom resolver", async function () {
var doi = doi4;
var item = createUnsavedDataObject('item', { itemType: 'journalArticle' });