Bug 1087636 - Better handling of server errors in GET /meta/global in Sync. r=markh draft
authorEdouard Oger <eoger@fastmail.com>
Fri, 13 May 2016 23:34:34 -0700
changeset 383168 4cc0db0a7ed7725bf8cc4064aecfe65db87b1e8a
parent 382798 82e1f1b9c0559f38a8460e2f2f3044de4c7712d6
child 524416 dda47a067f78ba9a469a3eb899084078f8e56069
push id21960
push userbmo:edouard.oger@gmail.com
push dateFri, 01 Jul 2016 17:45:08 +0000
reviewersmarkh
bugs1087636
milestone50.0a1
Bug 1087636 - Better handling of server errors in GET /meta/global in Sync. r=markh MozReview-Commit-ID: 7VNiSBpuwhP
services/sync/modules/service.js
services/sync/tests/unit/test_service_sync_remoteSetup.js
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -1080,32 +1080,36 @@ Sync11Service.prototype = {
       // If we got a 401, we do not want to create a new meta/global - we
       // should be able to get the existing meta after we get a new node.
       if (this.recordManager.response.status == 401) {
         this._log.debug("Fetching meta/global record on the server returned 401.");
         this.errorHandler.checkServerError(this.recordManager.response);
         return false;
       }
 
-      if (!this.recordManager.response.success || !newMeta) {
+      if (this.recordManager.response.status == 404) {
         this._log.debug("No meta/global record on the server. Creating one.");
         newMeta = new WBORecord("meta", "global");
         newMeta.payload.syncID = this.syncID;
         newMeta.payload.storageVersion = STORAGE_VERSION;
         newMeta.payload.declined = this.engineManager.getDeclined();
 
         newMeta.isNew = true;
 
         this.recordManager.set(this.metaURL, newMeta);
         let uploadRes = newMeta.upload(this.resource(this.metaURL));
         if (!uploadRes.success) {
           this._log.warn("Unable to upload new meta/global. Failing remote setup.");
           this.errorHandler.checkServerError(uploadRes);
           return false;
         }
+      } else if (!newMeta) {
+        this._log.warn("Unable to get meta/global. Failing remote setup.");
+        this.errorHandler.checkServerError(this.recordManager.response);
+        return false;
       } else {
         // If newMeta, then it stands to reason that meta != null.
         newMeta.isNew   = meta.isNew;
         newMeta.changed = meta.changed;
       }
 
       // Switch in the new meta object and record the new time.
       meta              = newMeta;
--- a/services/sync/tests/unit/test_service_sync_remoteSetup.js
+++ b/services/sync/tests/unit/test_service_sync_remoteSetup.js
@@ -48,25 +48,37 @@ function run_test() {
 
     let ts = new_timestamp();
     collectionsHelper.update_collection("crypto", ts);
     collectionsHelper.update_collection("clients", ts);
     collectionsHelper.update_collection("meta", ts);
     return_timestamp(request, response, ts);
   }
 
-  let server = httpd_setup({
+  const GLOBAL_PATH = "/1.1/johndoe/storage/meta/global";
+  const INFO_PATH = "/1.1/johndoe/info/collections";
+
+  let handlers = {
     "/1.1/johndoe/storage": storageHandler,
     "/1.1/johndoe/storage/crypto/keys": upd("crypto", keysWBO.handler()),
     "/1.1/johndoe/storage/crypto": upd("crypto", cryptoColl.handler()),
     "/1.1/johndoe/storage/clients": upd("clients", clients.handler()),
+    "/1.1/johndoe/storage/meta": upd("meta", wasCalledHandler(metaColl)),
     "/1.1/johndoe/storage/meta/global": upd("meta", wasCalledHandler(meta_global)),
-    "/1.1/johndoe/storage/meta": upd("meta", wasCalledHandler(metaColl)),
     "/1.1/johndoe/info/collections": collectionsHelper.handler
-  });
+  };
+
+  function mockHandler(path, mock) {
+    server.registerPathHandler(path, mock(handlers[path]));
+    return {
+      restore() { server.registerPathHandler(path, handlers[path]); }
+    }
+  }
+
+  let server = httpd_setup(handlers);
 
   try {
     _("Log in.");
     ensureLegacyIdentityManager();
     Service.serverURL = server.baseURI;
 
     _("Checking Status.sync with no credentials.");
     Service.verifyAndFetchSymmetricKeys();
@@ -84,16 +96,73 @@ function run_test() {
     Service.serverURL = server.baseURI;
     Service.login("johndoe", "ilovejane", syncKey);
     do_check_true(Service.isLoggedIn);
 
     _("Checking that remoteSetup returns true when credentials have changed.");
     Service.recordManager.get(Service.metaURL).payload.syncID = "foobar";
     do_check_true(Service._remoteSetup());
 
+    let returnStatusCode = (method, code) => (oldMethod) => (req, res) => {
+      if (req.method === method) {
+        res.setStatusLine(req.httpVersion, code, "");
+      } else {
+        oldMethod(req, res);
+      }
+    };
+
+    let mock = mockHandler(GLOBAL_PATH, returnStatusCode("GET", 401));
+    Service.recordManager.del(Service.metaURL);
+    _("Checking that remoteSetup returns false on 401 on first get /meta/global.");
+    do_check_false(Service._remoteSetup());
+    mock.restore();
+
+    Service.login("johndoe", "ilovejane", syncKey);
+    mock = mockHandler(GLOBAL_PATH, returnStatusCode("GET", 503));
+    Service.recordManager.del(Service.metaURL);
+    _("Checking that remoteSetup returns false on 503 on first get /meta/global.");
+    do_check_false(Service._remoteSetup());
+    do_check_eq(Service.status.sync, METARECORD_DOWNLOAD_FAIL);
+    mock.restore();
+
+    mock = mockHandler(GLOBAL_PATH, returnStatusCode("GET", 404));
+    Service.recordManager.del(Service.metaURL);
+    _("Checking that remoteSetup recovers on 404 on first get /meta/global.");
+    do_check_true(Service._remoteSetup());
+    mock.restore();
+
+    let makeOutdatedMeta = () => {
+      Service.metaModified = 0;
+      let infoResponse = Service._fetchInfo();
+      return {
+        status: infoResponse.status,
+        obj: {
+          crypto: infoResponse.obj.crypto,
+          clients: infoResponse.obj.clients,
+          meta: 1
+        }
+      };
+    }
+
+    _("Checking that remoteSetup recovers on 404 on get /meta/global after clear cached one.");
+    mock = mockHandler(GLOBAL_PATH, returnStatusCode("GET", 404));
+    Service.recordManager.set(Service.metaURL, { isNew: false });
+    do_check_true(Service._remoteSetup(makeOutdatedMeta()));
+    mock.restore();
+
+    _("Checking that remoteSetup returns false on 503 on get /meta/global after clear cached one.");
+    mock = mockHandler(GLOBAL_PATH, returnStatusCode("GET", 503));
+    Service.status.sync = "";
+    Service.recordManager.set(Service.metaURL, { isNew: false });
+    do_check_false(Service._remoteSetup(makeOutdatedMeta()));
+    do_check_eq(Service.status.sync, "");
+    mock.restore();
+
+    metaColl.delete({});
+
     _("Do an initial sync.");
     let beforeSync = Date.now()/1000;
     Service.sync();
 
     _("Checking that remoteSetup returns true.");
     do_check_true(Service._remoteSetup());
 
     _("Verify that the meta record was uploaded.");