commit a0c7cf9beed887ec25d10e1e4370d3dc294e439e
parent 4cc6408105bdc2d3aad6cf9779bde6810f38c065
Author: Dan Stillman <dstillman@zotero.org>
Date: Mon, 2 May 2016 01:33:58 -0400
Remove redundant error handling in Z.Sync.Data.Engine::_uploadObjects()
makeRequest() now retries 5xx errors automatically, so it's not
necessary higher up.
Diffstat:
1 file changed, 99 insertions(+), 124 deletions(-)
diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -835,141 +835,116 @@ Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(func
let results;
let numSuccessful = 0;
- try {
- ({ libraryVersion, results } = yield this.apiClient.uploadObjects(
- this.library.libraryType,
- this.libraryTypeID,
- "POST",
- libraryVersion,
- objectType,
- batch
- ));
-
- Zotero.debug("===");
- Zotero.debug(results);
-
- // Mark successful and unchanged objects as synced with new version,
- // and save uploaded JSON to cache
- let ids = [];
- let toSave = [];
- let toCache = [];
- for (let state of ['successful', 'unchanged']) {
- for (let index in results[state]) {
- let current = results[state][index];
- // 'successful' includes objects, not keys
- let key = state == 'successful' ? current.key : current;
-
- if (key != batch[index].key) {
- throw new Error("Key mismatch (" + key + " != " + batch[index].key + ")");
- }
-
- let obj = yield objectsClass.getByLibraryAndKeyAsync(
- this.libraryID, key, { noCache: true }
- )
- ids.push(obj.id);
-
- if (state == 'successful') {
- // Update local object with saved data if necessary
- yield obj.loadAllData();
- obj.fromJSON(current.data);
- toSave.push(obj);
- toCache.push(current);
- }
- else {
- // This won't reflect the actual version of the item on the server, but
- // it will guarantee that the item won't be redownloaded unnecessarily
- // in the case of a full sync, because the version will be higher than
- // whatever version is on the server.
- batch[index].version = libraryVersion
- toCache.push(batch[index]);
- }
-
- numSuccessful++;
- // Remove from batch to mark as successful
- delete batch[index];
- }
- }
- yield Zotero.Sync.Data.Local.saveCacheObjects(
- objectType, this.libraryID, toCache
- );
- yield Zotero.DB.executeTransaction(function* () {
- for (let i = 0; i < toSave.length; i++) {
- yield toSave[i].save();
- }
- this.library.libraryVersion = libraryVersion;
- yield this.library.save();
- objectsClass.updateVersion(ids, libraryVersion);
- objectsClass.updateSynced(ids, true);
- }.bind(this));
-
- // Handle failed objects
- for (let index in results.failed) {
- let { code, message, data } = results.failed[index];
- let e = new Error(message);
- e.name = "ZoteroObjectUploadError";
- e.code = code;
- if (data) {
- e.data = data;
- }
- Zotero.logError("Error for " + objectType + " " + batch[index].key + " in "
- + this.library.name + ":\n\n" + e);
+ ({ libraryVersion, results } = yield this.apiClient.uploadObjects(
+ this.library.libraryType,
+ this.libraryTypeID,
+ "POST",
+ libraryVersion,
+ objectType,
+ batch
+ ));
+
+ Zotero.debug("===");
+ Zotero.debug(results);
+
+ // Mark successful and unchanged objects as synced with new version,
+ // and save uploaded JSON to cache
+ let ids = [];
+ let toSave = [];
+ let toCache = [];
+ for (let state of ['successful', 'unchanged']) {
+ for (let index in results[state]) {
+ let current = results[state][index];
+ // 'successful' includes objects, not keys
+ let key = state == 'successful' ? current.key : current;
- // This shouldn't happen, because the upload request includes a library
- // version and should prevent an outdated upload before the object version is
- // checked. If it does, we need to do a full sync.
- if (e.code == 412) {
- return this.UPLOAD_RESULT_OBJECT_CONFLICT;
+ if (key != batch[index].key) {
+ throw new Error("Key mismatch (" + key + " != " + batch[index].key + ")");
}
- if (this.onError) {
- this.onError(e);
- }
- if (this.stopOnError) {
- throw new Error(e);
+ let obj = yield objectsClass.getByLibraryAndKeyAsync(
+ this.libraryID, key, { noCache: true }
+ )
+ ids.push(obj.id);
+
+ if (state == 'successful') {
+ // Update local object with saved data if necessary
+ yield obj.loadAllData();
+ obj.fromJSON(current.data);
+ toSave.push(obj);
+ toCache.push(current);
}
- batch[index].tries++;
- // Mark 400 errors as permanently failed
- if (e.code >= 400 && e.code < 500) {
- batch[index].failed = true;
+ else {
+ // This won't reflect the actual version of the item on the server, but
+ // it will guarantee that the item won't be redownloaded unnecessarily
+ // in the case of a full sync, because the version will be higher than
+ // whatever version is on the server.
+ batch[index].version = libraryVersion
+ toCache.push(batch[index]);
}
- // 500 errors should stay in queue and be retried
+
+ numSuccessful++;
+ // Remove from batch to mark as successful
+ delete batch[index];
}
+ }
+ yield Zotero.Sync.Data.Local.saveCacheObjects(
+ objectType, this.libraryID, toCache
+ );
+ yield Zotero.DB.executeTransaction(function* () {
+ for (let i = 0; i < toSave.length; i++) {
+ yield toSave[i].save();
+ }
+ this.library.libraryVersion = libraryVersion;
+ yield this.library.save();
+ objectsClass.updateVersion(ids, libraryVersion);
+ objectsClass.updateSynced(ids, true);
+ }.bind(this));
+
+ // Handle failed objects
+ for (let index in results.failed) {
+ let { code, message, data } = results.failed[index];
+ let e = new Error(message);
+ e.name = "ZoteroObjectUploadError";
+ e.code = code;
+ if (data) {
+ e.data = data;
+ }
+ Zotero.logError("Error for " + objectType + " " + batch[index].key + " in "
+ + this.library.name + ":\n\n" + e);
- // Add failed objects back to end of queue
- var numFailed = 0;
- for (let o of batch) {
- if (o !== undefined) {
- queue.push(o);
- // TODO: Clear JSON?
- numFailed++;
- }
+ // This shouldn't happen, because the upload request includes a library
+ // version and should prevent an outdated upload before the object version is
+ // checked. If it does, we need to do a full sync.
+ if (e.code == 412) {
+ return this.UPLOAD_RESULT_OBJECT_CONFLICT;
+ }
+
+ if (this.onError) {
+ this.onError(e);
+ }
+ if (this.stopOnError) {
+ throw new Error(e);
}
- Zotero.debug("Failed: " + numFailed, 2);
+ batch[index].tries++;
+ // Mark 400 errors as permanently failed
+ if (e.code >= 400 && e.code < 500) {
+ batch[index].failed = true;
+ }
+ // 500 errors should stay in queue and be retried
}
- catch (e) {
- if (e instanceof Zotero.HTTP.UnexpectedStatusException) {
- if (e.status == 412) {
- throw e;
- }
-
- // On 5xx, delay and retry
- if (e.status >= 500 && e.status <= 600) {
- if (!failureDelayGenerator) {
- // Keep trying for up to an hour
- failureDelayGenerator = Zotero.Utilities.Internal.delayGenerator(
- Zotero.Sync.Data.failureDelayIntervals, 60 * 60 * 1000
- );
- }
- let keepGoing = yield failureDelayGenerator.next();
- if (!keepGoing) {
- Zotero.logError("Failed too many times");
- throw e;
- }
- continue;
- }
+
+ // Add failed objects back to end of queue
+ var numFailed = 0;
+ for (let o of batch) {
+ if (o !== undefined) {
+ queue.push(o);
+ // TODO: Clear JSON?
+ numFailed++;
}
- throw e;
}
+ Zotero.debug("Failed: " + numFailed, 2);
+
// If we didn't make any progress, bail
if (!numSuccessful) {
throw new Error("Made no progress during upload -- stopping");
