From b31b71304de024e732992ce7ce5c71deff94f9cb Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Thu, 24 Jul 2025 01:10:52 -0400 Subject: [PATCH] ZFS: Don't retry download for canceled redirect with mtime/hash match This was a regression from the switch to `Zotero.HTTP.download()`. 302 wasn't a success code, so `HTTP.download()` would throw, and since the status wasn't set correctly on the `XMLHttpRequest` within `HTTP.UnexpectedStatusException`, it would think it was an interrupted S3 connection and trigger another download after a delay. --- chrome/content/zotero/xpcom/storage/zfs.js | 13 ++++++++----- test/tests/zfsTest.js | 5 +++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js index c63a3b912b..e683d1c25f 100644 --- a/chrome/content/zotero/xpcom/storage/zfs.js +++ b/chrome/content/zotero/xpcom/storage/zfs.js @@ -77,12 +77,13 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { var uri = this.apiClient.buildRequestURI(params); return new Promise(async (resolve, reject) => { + var resultOptions = {}; try { let req = await Zotero.HTTP.download( uri, destPath, { - successCodes: [200, 404], + successCodes: [200, 302, 404], headers: this.apiClient.getHeaders(), noCache: true, notificationCallbacks: { @@ -139,10 +140,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { } item.attachmentSyncState = "in_sync"; await item.saveTx({ skipAll: true }); - - resolve(new Zotero.Sync.Storage.Result({ - localChanges: true - })); + resultOptions.localChanges = true; callback.onRedirectVerifyCallback(Cr.NS_ERROR_ABORT); }, @@ -154,6 +152,11 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { } ); + if (req.status == 302) { + resolve(new Zotero.Sync.Storage.Result(resultOptions)); + return; + } + if (req.status == 404) { Zotero.debug("Remote file not found for item " + item.libraryKey); // Don't refresh item pane rows when nothing happened diff --git a/test/tests/zfsTest.js b/test/tests/zfsTest.js index 6802138c5f..5f2cbee90e 100644 --- a/test/tests/zfsTest.js +++ b/test/tests/zfsTest.js @@ -680,6 +680,8 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { var mtime = (Math.floor(new Date().getTime() / 1000) * 1000) + ""; var md5 = Zotero.Utilities.Internal.md5(file) + var processDownloadSpy = sinon.spy(Zotero.Sync.Storage.Local, "processDownload"); + var s3Path = `pretend-s3/${item.key}`; httpd.registerPathHandler( `/users/1/items/${item.key}/file`, @@ -709,6 +711,9 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { assert.isTrue(result.localChanges); assert.isFalse(result.remoteChanges); assert.isFalse(result.syncRequired); + assert.isTrue(processDownloadSpy.notCalled); + + processDownloadSpy.restore(); }) it("should update local info for file that already exists on the server", function* () {