Bug 1087636 - Better handling of server errors in GET /meta/global in Sync. r=markh
MozReview-Commit-ID: 7VNiSBpuwhP
--- 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.");