Bug 1425152 - Update lastSync timestamp even if some sync engines failed r=markh

MozReview-Commit-ID: HTWuZsp5Evb

--HG--
extra : rebase_source : 35240ec21bd902e46493d5f0d62a5cb136f7861e

services/sync/modules/stages/enginesync.js

@@ -186,9 +186,15 @@ EngineSynchronizer.prototype = {
 
       // If there were no sync engine failures
       if (this.service.status.service != SYNC_FAILED_PARTIAL) {
-        Svc.Prefs.set("lastSync", new Date().toString());
         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());
+      }
     } finally {
       Svc.Prefs.reset("firstSync");
 

services/sync/tests/unit/test_errorhandler_2.js

@@ -131,35 +131,39 @@ add_task(async function test_crypto_keys_login_server_maintenance_error() {
   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;
+  await configureIdentity({username: "johndoe"}, server);
-  engine.enabled = true;
-  engine.exception = {status: 503,
-                      headers: {"retry-after": BACKOFF}};
 
-  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 lastSync = Svc.Prefs.get("lastSync");
-  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;
 
+  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);
 
-  await promiseStopServer(server);
+  // We shouldn't update lastSync on complete failure.
+  Assert.equal(lastSync, Svc.Prefs.get("lastSync"));
+
   await clean();
+  await promiseStopServer(server);
 });
 
 add_task(async function test_info_collections_login_prolonged_server_maintenance_error() {
@@ -795,7 +799,7 @@ add_task(async function test_sync_engine_generic_fail() {
   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);
 
@@ -824,6 +828,9 @@ add_task(async function test_sync_engine_generic_fail() {
   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);