Bug 1230011 - Remember when a wipeServer call fails and retry the wipe on later syncs. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Mon, 06 Mar 2017 13:59:28 -0500
changeset 494198 3c1df24f1eeb089ecf53dfc0ee327b6861e2804d
parent 494119 bfe78e7db8fd08053642798a33dcf54fba1580c9
child 548035 7754f74163c095302595b196ada8c08d5130ea41
push id47962
push userbmo:tchiovoloni@mozilla.com
push dateMon, 06 Mar 2017 20:19:41 +0000
reviewersmarkh
bugs1230011
milestone54.0a1
Bug 1230011 - Remember when a wipeServer call fails and retry the wipe on later syncs. r?markh MozReview-Commit-ID: AEx1F5ygCnT
services/sync/modules/service.js
services/sync/modules/stages/enginesync.js
services/sync/tests/unit/test_service_wipeServer.js
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -1192,75 +1192,105 @@ Sync11Service.prototype = {
     // Wipe everything we know about except meta because we just uploaded it
     // TODO: there's a bug here. We should be calling resetClient, no?
 
     // Generate, upload, and download new keys. Do this last so we don't wipe
     // them...
     this.generateNewSymmetricKeys();
   },
 
+  get pendingServerWipe() {
+    return JSON.parse(Svc.Prefs.get("pendingServerWipe", "[]"));
+  },
+
+  set pendingServerWipe(colls) {
+    return Svc.Prefs.set("pendingServerWipe", JSON.stringify(colls || []));
+  },
+
   /**
    * Wipe user data from the server.
    *
    * @param collections [optional]
    *        Array of collections to wipe. If not given, all collections are
    *        wiped by issuing a DELETE request for `storageURL`.
    *
    * @return the server's timestamp of the (last) DELETE.
    */
   wipeServer: function wipeServer(collections) {
+    dump("#### WIPE SERVER: " + JSON.stringify(collections) + "\n");
     let response;
     let histogram = Services.telemetry.getHistogramById("WEAVE_WIPE_SERVER_SUCCEEDED");
-    if (!collections) {
+    let pendingServerWipe = this.pendingServerWipe;
+    if (!collections || this.pendingServerWipe.includes("all")) {
       // Strip the trailing slash.
       let res = this.resource(this.storageURL.slice(0, -1));
       res.setHeader("X-Confirm-Delete", "1");
       try {
         response = res.delete();
       } catch (ex) {
-        this._log.debug("Failed to wipe server", ex);
+        this._log.error("Failed to wipe server", ex);
         histogram.add(false);
+        this.pendingServerWipe = ["all"];
         throw ex;
       }
       if (response.status != 200 && response.status != 404) {
         this._log.debug("Aborting wipeServer. Server responded with " +
                         response.status + " response for " + this.storageURL);
         histogram.add(false);
+        this.pendingServerWipe = ["all"];
         throw response;
       }
       histogram.add(true);
+      this.pendingServerWipe = [];
       return response.headers["x-weave-timestamp"];
     }
 
     let timestamp;
-    for (let name of collections) {
+    collections = Array.from(new Set(collections.concat(pendingServerWipe)));
+    for (let i = 0; i < collections.length; ++i) {
+      let name = collections[i];
       let url = this.storageURL + name;
       try {
         response = this.resource(url).delete();
       } catch (ex) {
-        this._log.debug("Failed to wipe '" + name + "' collection", ex);
+        this._log.warn("Failed to wipe '" + name + "' collection", ex);
         histogram.add(false);
+        this._log.info("Pending collections to wipe", collections.slice(i, collections.length));
+        this.pendingServerWipe = collections.slice(i, collections.length);
         throw ex;
       }
 
       if (response.status != 200 && response.status != 404) {
-        this._log.debug("Aborting wipeServer. Server responded with " +
+        this._log.warn("Aborting wipeServer. Server responded with " +
                         response.status + " response for " + url);
         histogram.add(false);
+        this._log.info("Pending collections to wipe", collections.slice(i, collections.length));
+        this.pendingServerWipe = collections.slice(i, collections.length);
         throw response;
       }
 
       if ("x-weave-timestamp" in response.headers) {
         timestamp = response.headers["x-weave-timestamp"];
       }
     }
     histogram.add(true);
+    this.pendingServerWipe = [];
     return timestamp;
   },
 
+  checkPendingServerWipe() {
+    let pendingWipe = this.pendingServerWipe;
+    if (pendingWipe && pendingWipe.length) {
+      if (pendingWipe.includes("all")) {
+        this.wipeServer();
+      } else {
+        this.wipeServer(pendingWipe);
+      }
+    }
+  },
   /**
    * Wipe all local user data.
    *
    * @param engines [optional]
    *        Array of engine names to wipe. If not given, all engines are used.
    */
   wipeClient: function wipeClient(engines) {
     // If we don't have any engines, reset the service and wipe all engines
@@ -1331,17 +1361,17 @@ Sync11Service.prototype = {
 
   /**
    * Reset the client by getting rid of any local server data and client data.
    *
    * @param engines [optional]
    *        Array of engine names to reset. If not given, all engines are used.
    */
   resetClient: function resetClient(engines) {
-    this._catch(function doResetClient() {
+    this._catch(function do_remoteSetupResetClient() {
       // If we don't have any engines, reset everything including the service
       if (!engines) {
         // Clear out any service data
         this.resetService();
 
         engines = [this.clientsEngine].concat(this.engineManager.getAll());
       } else {
         // Convert the array of names into engines
--- a/services/sync/modules/stages/enginesync.js
+++ b/services/sync/modules/stages/enginesync.js
@@ -86,16 +86,18 @@ EngineSynchronizer.prototype = {
       engine.lastModified = info.obj[engine.name] || 0;
     }
 
     if (!(this.service._remoteSetup(info))) {
       this.onComplete(new Error("Aborting sync, remote setup failed"));
       return;
     }
 
+    this.service.checkPendingServerWipe();
+
     // Make sure we have an up-to-date list of clients before sending commands
     this._log.debug("Refreshing client list.");
     if (!this._syncEngine(this.service.clientsEngine)) {
       // Clients is an engine like any other; it can fail with a 401,
       // and we can elect to abort the sync.
       this._log.warn("Client engine sync failed. Aborting.");
       this.onComplete(null);
       return;
--- a/services/sync/tests/unit/test_service_wipeServer.js
+++ b/services/sync/tests/unit/test_service_wipeServer.js
@@ -225,8 +225,131 @@ add_task(async function test_wipeServer_
     do_throw("Should have thrown!");
   } catch (ex) {
     do_check_eq(ex.result, Cr.NS_ERROR_CONNECTION_REFUSED);
   }
 
   Svc.Prefs.resetBranch("");
   await promiseStopServer(server);
 });
+
+function login_handling(handler) {
+  return function(request, response) {
+    if (has_hawk_header(request)) {
+      handler(request, response);
+    } else {
+      let body = "Unauthorized";
+      response.setStatusLine(request.httpVersion, 401, "Unauthorized");
+      response.bodyOutputStream.write(body, body.length);
+    }
+  };
+}
+
+add_task(async function test_wipeServer_retry_all() {
+  _("Service.wipeServer() retries next sync after first failure");
+  let shouldFail = true;
+  let deleted = false;
+  function storageHandler(request, response) {
+    do_check_eq("DELETE", request.method);
+    do_check_true(request.hasHeader("X-Confirm-Delete"));
+    if (shouldFail) {
+      response.setStatusLine(request.httpVersion, 503, "Service Unavailable");
+      let body = "Service Unavailable";
+      response.bodyOutputStream.write(body, body.length);
+    } else {
+      deleted = true;
+      serverTimestamp = return_timestamp(request, response);
+    }
+  }
+  let johnHelper = track_collections_helper();
+  let johnU = johnHelper.with_updated_collection;
+
+  let server = httpd_setup({
+    "/1.1/johndoe/storage": storageHandler,
+    "/1.1/johndoe/storage/crypto/keys": johnU("crypto", new ServerWBO("keys").handler()),
+    "/1.1/johndoe/storage/meta/global": johnU("meta", new ServerWBO("global").handler()),
+    "/1.1/johndoe/info/collections": login_handling(johnHelper.handler),
+  });
+
+  await setUpTestFixtures(server);
+
+  _("Try deletion.");
+  let error;
+  try {
+    await SyncTestingInfrastructure(server, "johndoe", "irrelevant");
+    Service.wipeServer();
+    do_throw("Should have thrown!");
+  } catch (ex) {
+    error = ex;
+  }
+  do_check_eq(error.status, 503);
+  shouldFail = false;
+  Service.sync();
+  ok(deleted);
+  await promiseStopServer(server);
+  Svc.Prefs.resetBranch("");
+});
+
+add_task(async function test_wipeServer_retry_list() {
+  _("Service.wipeServer() retries failed in provided list.");
+
+  let steam_coll = new FakeCollection();
+  let diesel_coll = new FakeCollection();
+  let petrol_coll = new FakeCollection();
+  let johnHelper = track_collections_helper();
+  let johnU = johnHelper.with_updated_collection;
+
+  let failPetrol = true;
+  let server = httpd_setup({
+    "/1.1/johndoe/storage/steam": steam_coll.handler(),
+    "/1.1/johndoe/storage/petrol": function(request, response) {
+      if (failPetrol) {
+        response.setStatusLine(request.httpVersion, 503, "Service Unavailable");
+        let body = "Service Unavailable";
+        response.bodyOutputStream.write(body, body.length);
+      } else {
+        return petrol_coll.handler()(request, response);
+      }
+    },
+    "/1.1/johndoe/storage/diesel": diesel_coll.handler(),
+    "/1.1/johndoe/storage/crypto/keys": johnU("crypto", new ServerWBO("keys").handler()),
+    "/1.1/johndoe/storage/meta/global": johnU("meta", new ServerWBO("global").handler()),
+    "/1.1/johndoe/info/collections": login_handling(johnHelper.handler),
+  });
+
+  try {
+    await setUpTestFixtures(server);
+    await SyncTestingInfrastructure(server, "johndoe", "irrelevant");
+
+    _("Confirm initial environment.");
+    do_check_false(steam_coll.deleted);
+    do_check_false(diesel_coll.deleted);
+
+    _("wipeServer() will happily ignore the non-existent collection, delete the 'steam' collection and abort after an receiving an error on the 'petrol' collection.");
+    let error;
+    try {
+      Service.wipeServer(["non-existent", "steam", "petrol", "diesel"]);
+      do_throw("Should have thrown!");
+    } catch (ex) {
+      error = ex;
+    }
+    _("wipeServer() threw this exception: " + error);
+    do_check_eq(error.status, 503);
+
+    _("wipeServer stopped deleting after encountering an error with the 'petrol' collection, thus only 'steam' has been deleted.");
+    do_check_true(steam_coll.deleted);
+    do_check_false(petrol_coll.deleted);
+    do_check_false(diesel_coll.deleted);
+    failPetrol = false;
+    deepEqual(["petrol", "diesel"], Service.pendingServerWipe);
+    // A full sync here fails for unrelated reasons like having garbage meta
+    // information.
+    Service.checkPendingServerWipe();
+
+    deepEqual([], Service.pendingServerWipe);
+    do_check_true(steam_coll.deleted);
+    do_check_true(petrol_coll.deleted);
+
+  } finally {
+    await promiseStopServer(server);
+    Svc.Prefs.resetBranch("");
+  }
+});
\ No newline at end of file