Bug 1425152 - Update lastSync timestamp even if some sync engines failed r=markh
☠☠ backed out by 1f32df80a818 ☠ ☠
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Tue, 20 Feb 2018 18:56:35 -0500
changeset 461186 017d83ffe0a47a22d342ab61224ca09977a308a5
parent 461185 3aa4c86bbf163e222d9789f9f43912e7d24b3763
child 461187 b1ba6c67264bce7b38de085b41183a5029023127
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1425152
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1425152 - Update lastSync timestamp even if some sync engines failed r=markh MozReview-Commit-ID: HTWuZsp5Evb
services/sync/modules/stages/enginesync.js
services/sync/tests/unit/test_errorhandler_2.js
--- a/services/sync/modules/stages/enginesync.js
+++ b/services/sync/modules/stages/enginesync.js
@@ -181,18 +181,24 @@ EngineSynchronizer.prototype = {
       }
 
       if (!fastSync) {
         await Doctor.consult(enginesToValidate);
       }
 
       // If there were no sync engine failures
       if (this.service.status.service != SYNC_FAILED_PARTIAL) {
+        this.service.status.sync = SYNC_SUCCEEDED;
+      }
+
+      // Even if there were engine failures, bump lastSync even on partial since
+      // it's reflected in the UI (bug 1439777).
+      if (this.service.status.service == SYNC_FAILED_PARTIAL ||
+          this.service.status.service == STATUS_OK) {
         Svc.Prefs.set("lastSync", new Date().toString());
-        this.service.status.sync = SYNC_SUCCEEDED;
       }
     } finally {
       Svc.Prefs.reset("firstSync");
 
       let syncTime = ((Date.now() - startTime) / 1000).toFixed(2);
       let dateStr = Utils.formatTimestamp(new Date());
       this._log.info("Sync completed at " + dateStr
                      + " after " + syncTime + " secs.");
--- a/services/sync/tests/unit/test_errorhandler_2.js
+++ b/services/sync/tests/unit/test_errorhandler_2.js
@@ -126,45 +126,49 @@ add_task(async function test_crypto_keys
   Assert.equal(Status.login, SERVER_MAINTENANCE);
   Assert.ok(!errorHandler.didReportProlongedError);
 
   Svc.Obs.remove("weave:ui:login:error", onUIUpdate);
   await clean();
   await promiseStopServer(server);
 });
 
-add_task(async function test_sync_prolonged_server_maintenance_error() {
+add_task(async function test_lastSync_not_updated_on_complete_failure() {
   enableValidationPrefs();
 
-  // Test prolonged server maintenance errors are reported.
+  // Test info/collections prolonged server maintenance errors are reported.
   let server = await EHTestsCommon.sync_httpd_setup();
   await EHTestsCommon.setUp(server);
 
-  const BACKOFF = 42;
-  engine.enabled = true;
-  engine.exception = {status: 503,
-                      headers: {"retry-after": BACKOFF}};
+  await configureIdentity({username: "johndoe"}, server);
 
-  let promiseObserved = promiseOneObserver("weave:ui:sync:error");
+  // Do an initial sync that we expect to be successful.
+  await sync_and_validate_telem(false);
 
   Assert.equal(Status.service, STATUS_OK);
+  Assert.equal(Status.sync, SYNC_SUCCEEDED);
 
-  setLastSync(PROLONGED_ERROR_DURATION);
-  let ping = await sync_and_validate_telem(true);
-  deepEqual(ping.status.sync, PROLONGED_SYNC_FAILURE);
-  deepEqual(ping.engines.find(e => e.failureReason).failureReason,
-            { name: "httperror", code: 503 });
-  await promiseObserved;
+  let lastSync = Svc.Prefs.get("lastSync");
+
+  Assert.ok(lastSync);
+
+  // Report server maintenance on info/collections requests
+  server.registerPathHandler("/1.1/johndoe/info/collections",
+                             EHTestsCommon.service_unavailable);
 
+  await sync_and_validate_telem(true);
+
+  Assert.equal(Status.sync, SERVER_MAINTENANCE);
   Assert.equal(Status.service, SYNC_FAILED);
-  Assert.equal(Status.sync, PROLONGED_SYNC_FAILURE);
-  Assert.ok(errorHandler.didReportProlongedError);
 
+  // We shouldn't update lastSync on complete failure.
+  Assert.equal(lastSync, Svc.Prefs.get("lastSync"));
+
+  await clean();
   await promiseStopServer(server);
-  await clean();
 });
 
 add_task(async function test_info_collections_login_prolonged_server_maintenance_error() {
   enableValidationPrefs();
 
   // Test info/collections prolonged server maintenance errors are reported.
   let server = await EHTestsCommon.sync_httpd_setup();
   await EHTestsCommon.setUp(server);
@@ -790,17 +794,17 @@ add_task(async function test_sync_engine
 
   equal(getLogFiles().length, 0);
 
   let server = await EHTestsCommon.sync_httpd_setup();
   engine.enabled = true;
   engine.sync = async function sync() {
     Svc.Obs.notify("weave:engine:sync:error", ENGINE_UNKNOWN_FAIL, "catapult");
   };
-
+  let lastSync = Svc.Prefs.get("lastSync");
   let log = Log.repository.getLogger("Sync.ErrorHandler");
   Svc.Prefs.set("log.appender.file.logOnError", true);
 
   Assert.equal(Status.engines.catapult, undefined);
 
   let promiseObserved = new Promise(res => {
     Svc.Obs.add("weave:engine:sync:finish", function onEngineFinish() {
       Svc.Obs.remove("weave:engine:sync:finish", onEngineFinish);
@@ -819,16 +823,19 @@ add_task(async function test_sync_engine
   deepEqual(ping.engines.find(e => e.status).status, ENGINE_UNKNOWN_FAIL);
 
   await promiseObserved;
 
   _("Status.engines: " + JSON.stringify(Status.engines));
   Assert.equal(Status.engines.catapult, ENGINE_UNKNOWN_FAIL);
   Assert.equal(Status.service, SYNC_FAILED_PARTIAL);
 
+  // lastSync should update on partial failure.
+  Assert.notEqual(lastSync, Svc.Prefs.get("lastSync"));
+
   // Test Error log was written on SYNC_FAILED_PARTIAL.
   let logFiles = getLogFiles();
   equal(logFiles.length, 1);
   Assert.ok(logFiles[0].leafName.startsWith("error-sync-"), logFiles[0].leafName);
 
   await clean();
 
   let syncErrors = sumHistogram("WEAVE_ENGINE_SYNC_ERRORS", { key: "catapult" });