Bug 1257533 - Add safety check when server is failing draft
authorMathieu Leplatre <mathieu@mozilla.com>
Wed, 06 Apr 2016 12:47:04 +0200
changeset 350952 fda4a51492a1628dbeda991ff189ec410b76b298
parent 350951 124eeec50eb4153c1310308417614ad21ab4af6f
child 350953 726d33f7368e9500d4028f783b33e351ce307795
push id15453
push usermleplatre@mozilla.com
push dateThu, 14 Apr 2016 14:30:40 +0000
bugs1257533, 1259145
milestone48.0a1
Bug 1257533 - Add safety check when server is failing This also fixes Bug 1259145. MozReview-Commit-ID: 3UVZJ88E6Dk
services/common/kinto-updater.js
services/common/tests/unit/test_kinto_updater.js
--- a/services/common/kinto-updater.js
+++ b/services/common/kinto-updater.js
@@ -45,42 +45,50 @@ this.checkVersions = function() {
       const lastEtag = Services.prefs.getCharPref(PREF_KINTO_LAST_ETAG);
       if (lastEtag) {
         headers["If-None-Match"] = lastEtag;
       }
     }
 
     let response = yield fetch(changesEndpoint, {headers});
 
+    let versionInfo;
+    // No changes since last time. Go on with empty list of changes.
+    if (response.status == 304) {
+      versionInfo = {data: []};
+    }
+    else {
+      versionInfo = yield response.json();
+    }
+
+    // If the server is failing, the JSON response might not contain the
+    // expected data (e.g. error response - Bug 1259145)
+    if (!versionInfo.hasOwnProperty("data")) {
+      throw new Error("Polling for changes failed.");
+    }
+
     // Record new update time and the difference between local and server time
     let serverTimeMillis = Date.parse(response.headers.get("Date"));
     let clockDifference = Math.abs(Date.now() - serverTimeMillis) / 1000;
+    Services.prefs.setIntPref(PREF_KINTO_CLOCK_SKEW_SECONDS, clockDifference);
     Services.prefs.setIntPref(PREF_KINTO_LAST_UPDATE, serverTimeMillis / 1000);
-    Services.prefs.setIntPref(PREF_KINTO_CLOCK_SKEW_SECONDS, clockDifference);
-
-    // No changes since last time.
-    if (response.status == 304) {
-      return;
-    }
-
-    let versionInfo = yield response.json();
 
     let firstError;
     for (let collectionInfo of versionInfo.data) {
       // Skip changes that don't concern configured blocklist bucket.
       if (collectionInfo.bucket != blocklistsBucket) {
         continue;
       }
 
       let collection = collectionInfo.collection;
       let kintoClient = kintoClients[collection];
       if (kintoClient && kintoClient.maybeSync) {
         let lastModified = 0;
         if (collectionInfo.last_modified) {
-          lastModified = collectionInfo.last_modified
+          lastModified = collectionInfo.last_modified;
         }
         try {
           yield kintoClient.maybeSync(lastModified, serverTimeMillis);
         } catch (e) {
           if (!firstError) {
             firstError = e;
           }
         }
--- a/services/common/tests/unit/test_kinto_updater.js
+++ b/services/common/tests/unit/test_kinto_updater.js
@@ -27,34 +27,34 @@ add_task(function* test_check_maybeSync(
       response.setStatusLine(null, sampled.status.status,
                              sampled.status.statusText);
       // send the headers
       for (let headerLine of sampled.sampleHeaders) {
         let headerElements = headerLine.split(':');
         response.setHeader(headerElements[0], headerElements[1].trimLeft());
       }
 
-      // set the
+      // set the server date
       response.setHeader("Date", (new Date(2000)).toUTCString());
 
       response.write(sampled.responseBody);
     } catch (e) {
       dump(`${e}\n`);
     }
   }
 
   server.registerPathHandler(changesPath, handleResponse);
 
   // set up prefs so the kinto updater talks to the test server
   Services.prefs.setCharPref(PREF_KINTO_BASE,
     `http://localhost:${server.identity.primaryPort}/v1`);
 
   // set some initial values so we can check these are updated appropriately
-  Services.prefs.setIntPref("services.kinto.last_update", 0);
-  Services.prefs.setIntPref("services.kinto.clock_difference", 0);
+  Services.prefs.setIntPref(PREF_LAST_UPDATE, 0);
+  Services.prefs.setIntPref(PREF_CLOCK_SKEW_SECONDS, 0);
   Services.prefs.clearUserPref(PREF_LAST_ETAG);
 
 
   let startTime = Date.now();
 
   let updater = Cu.import("resource://services-common/kinto-updater.js");
 
   let syncPromise = new Promise(function(resolve, reject) {
@@ -77,25 +77,55 @@ add_task(function* test_check_maybeSync(
   do_check_eq(Services.prefs.getIntPref(PREF_LAST_UPDATE), 2);
 
   // How does the clock difference look?
   let endTime = Date.now();
   let clockDifference = Services.prefs.getIntPref(PREF_CLOCK_SKEW_SECONDS);
   // we previously set the serverTime to 2 (seconds past epoch)
   do_check_eq(clockDifference <= endTime / 1000
               && clockDifference >= Math.floor(startTime / 1000) - 2, true);
-
   // Last timestamp was saved. An ETag header value is a quoted string.
   let lastEtag = Services.prefs.getCharPref(PREF_LAST_ETAG);
   do_check_eq(lastEtag, "\"1100\"");
+
+
+  // Simulate a poll with up-to-date collection.
+  Services.prefs.setIntPref(PREF_LAST_UPDATE, 0);
   // If server has no change, a 304 is received, maybeSync() is not called.
   updater.addTestKintoClient("test-collection", {
     maybeSync: () => {throw new Error("Should not be called");}
   });
   yield updater.checkVersions();
+  // Last update is overwritten
+  do_check_eq(Services.prefs.getIntPref(PREF_LAST_UPDATE), 2);
+
+
+  // Simulate a server error.
+  function simulateErrorResponse (request, response) {
+    response.setHeader("Date", (new Date(3000)).toUTCString());
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
+    response.write(JSON.stringify({
+      code: 503,
+      errno: 999,
+      error: "Service Unavailable",
+    }));
+    response.setStatusLine(null, 503, "Service Unavailable");
+  }
+  server.registerPathHandler(changesPath, simulateErrorResponse);
+  // checkVersions() fails with adequate error.
+  let error;
+  try {
+    yield updater.checkVersions();
+  }
+  catch (e) {
+    error = e;
+  }
+  do_check_eq(error.message, "Polling for changes failed.");
+  // When an error occurs, last update was not overwritten (see Date header above).
+  do_check_eq(Services.prefs.getIntPref(PREF_LAST_UPDATE), 2);
 });
 
 function run_test() {
   // Set up an HTTP Server
   server = new HttpServer();
   server.start(-1);
 
   run_next_test();