Bug 1529584 - Distinguish Remote Settings errors when reporting uptake r=glasserc
authorMathieu Leplatre <mathieu@mozilla.com>
Mon, 25 Feb 2019 20:22:16 +0000
changeset 518890 35606fc8f21a2fd01bf1f06e5ca2b4004188d052
parent 518889 5aa193f471ea49165243f10c49f6ef99e26f723f
child 518891 46ca7bb91c8e3315a7caaa0a73eb32a7c0b604a4
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglasserc
bugs1529584
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1529584 - Distinguish Remote Settings errors when reporting uptake r=glasserc Distinguish Remote Settings errors when reporting uptake Differential Revision: https://phabricator.services.mozilla.com/D20836
security/manager/ssl/tests/unit/test_cert_blocklist.js
services/settings/RemoteSettingsClient.jsm
services/settings/Utils.jsm
services/settings/remote-settings.js
services/settings/test/unit/test_remote_settings.js
services/settings/test/unit/test_remote_settings_poll.js
--- a/security/manager/ssl/tests/unit/test_cert_blocklist.js
+++ b/security/manager/ssl/tests/unit/test_cert_blocklist.js
@@ -133,16 +133,17 @@ const certBlocklistJSON = `{
       "subject": "MCIxIDAeBgNVBAMMF0Fub3RoZXIgVGVzdCBFbmQtZW50aXR5",
       "pubKeyHash": "VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8="
     }
   ]
 }`;
 
 function serveResponse(body) {
   return (req, response) => {
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.setStatusLine(null, 200, "OK");
     response.write(body);
   };
 }
 
 testserver.registerPathHandler("/v1/",
                                serveResponse(kintoHelloViewJSON));
 testserver.registerPathHandler("/v1/buckets/monitor/collections/changes/records",
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -327,19 +327,18 @@ class RemoteSettingsClient extends Event
       // Fetch changes from server.
       let syncResult;
       try {
         // Server changes have priority during synchronization.
         const strategy = Kinto.syncStrategy.SERVER_WINS;
         syncResult = await collection.sync({ remote: gServerURL, strategy, expectedTimestamp });
         const { ok } = syncResult;
         if (!ok) {
-          // Some synchronization conflicts occured.
-          reportStatus = UptakeTelemetry.STATUS.CONFLICT_ERROR;
-          throw new Error("Sync failed");
+          // With SERVER_WINS, there cannot be any conflicts, but don't silent it anyway.
+          throw new Error("Synced failed");
         }
       } catch (e) {
         if (e.message.includes(INVALID_SIGNATURE)) {
           // Signature verification failed during synchronization.
           reportStatus = UptakeTelemetry.STATUS.SIGNATURE_ERROR;
           // If sync fails with a signature error, it's likely that our
           // local data has been modified in some way.
           // We will attempt to fix this by retrieving the whole
@@ -352,18 +351,24 @@ class RemoteSettingsClient extends Event
             reportStatus = UptakeTelemetry.STATUS.SIGNATURE_RETRY_ERROR;
             throw e;
           }
         } else {
           // The sync has thrown, it can be related to metadata, network or a general error.
           if (e.message == MISSING_SIGNATURE) {
             // Collection metadata has no signature info, no need to retry.
             reportStatus = UptakeTelemetry.STATUS.SIGNATURE_ERROR;
+          } else if (/unparseable/.test(e.message)) {
+            reportStatus = UptakeTelemetry.STATUS.PARSE_ERROR;
           } else if (/NetworkError/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.NETWORK_ERROR;
+          } else if (/Timeout/.test(e.message)) {
+            reportStatus = UptakeTelemetry.STATUS.TIMEOUT_ERROR;
+          } else if (/HTTP 5??/.test(e.message)) {
+            reportStatus = UptakeTelemetry.STATUS.SERVER_ERROR;
           } else if (/Backoff/.test(e.message)) {
             reportStatus = UptakeTelemetry.STATUS.BACKOFF;
           } else {
             reportStatus = UptakeTelemetry.STATUS.SYNC_ERROR;
           }
           throw e;
         }
       }
@@ -374,16 +379,20 @@ class RemoteSettingsClient extends Event
         try {
           await this.emit("sync", { data: filteredSyncResult });
         } catch (e) {
           reportStatus = UptakeTelemetry.STATUS.APPLY_ERROR;
           throw e;
         }
       }
     } catch (e) {
+      // IndexedDB errors. See https://developer.mozilla.org/en-US/docs/Web/API/IDBRequest/error
+      if (/(AbortError|ConstraintError|QuotaExceededError|VersionError)/.test(e.message)) {
+        reportStatus = UptakeTelemetry.STATUS.CUSTOM_1_ERROR;
+      }
       // No specific error was tracked, mark it as unknown.
       if (reportStatus === null) {
         reportStatus = UptakeTelemetry.STATUS.UNKNOWN_ERROR;
       }
       throw e;
     } finally {
       // No error was reported, this is a success!
       if (reportStatus === null) {
--- a/services/settings/Utils.jsm
+++ b/services/settings/Utils.jsm
@@ -77,16 +77,20 @@ var Utils = {
     if (params) {
       url += "?" + Object.entries(params).map(([k, v]) => `${k}=${encodeURIComponent(v)}`).join("&");
     }
     const response = await fetch(url, { headers });
 
     let changes = [];
     // If no changes since last time, go on with empty list of changes.
     if (response.status != 304) {
+      const ct = response.headers.get("Content-Type");
+      if (!ct || !ct.includes("application/json")) {
+        throw new Error(`Unexpected content-type "${ct}"`);
+      }
       let payload;
       try {
         payload = await response.json();
       } catch (e) {
         payload = e.message;
       }
 
       if (!payload.hasOwnProperty("data")) {
--- a/services/settings/remote-settings.js
+++ b/services/settings/remote-settings.js
@@ -176,18 +176,24 @@ function remoteSettingsFunction() {
     const lastEtag = gPrefs.getCharPref(PREF_SETTINGS_LAST_ETAG, "");
 
     let pollResult;
     try {
       pollResult = await Utils.fetchLatestChanges(remoteSettings.pollingEndpoint, { expectedTimestamp, lastEtag });
     } catch (e) {
       // Report polling error to Uptake Telemetry.
       let reportStatus;
-      if (/Server/.test(e.message)) {
+      if (/JSON\.parse/.test(e.message)) {
+        reportStatus = UptakeTelemetry.STATUS.PARSE_ERROR;
+      } else if (/content-type/.test(e.message)) {
+        reportStatus = UptakeTelemetry.STATUS.CONTENT_ERROR;
+      } else if (/Server/.test(e.message)) {
         reportStatus = UptakeTelemetry.STATUS.SERVER_ERROR;
+      } else if (/Timeout/.test(e.message)) {
+        reportStatus = UptakeTelemetry.STATUS.TIMEOUT_ERROR;
       } else if (/NetworkError/.test(e.message)) {
         reportStatus = UptakeTelemetry.STATUS.NETWORK_ERROR;
       } else {
         reportStatus = UptakeTelemetry.STATUS.UNKNOWN_ERROR;
       }
       UptakeTelemetry.report(TELEMETRY_COMPONENT, reportStatus, { source: TELEMETRY_SOURCE });
       // No need to go further.
       throw new Error(`Polling for changes failed: ${e.message}.`);
--- a/services/settings/test/unit/test_remote_settings.js
+++ b/services/settings/test/unit/test_remote_settings.js
@@ -59,17 +59,19 @@ function run_test() {
                              sample.status.statusText);
       // send the headers
       for (let headerLine of sample.sampleHeaders) {
         let headerElements = headerLine.split(":");
         response.setHeader(headerElements[0], headerElements[1].trimLeft());
       }
       response.setHeader("Date", (new Date()).toUTCString());
 
-      response.write(JSON.stringify(sample.responseBody));
+      const body = typeof sample.responseBody == "string" ? sample.responseBody
+                                                          : JSON.stringify(sample.responseBody);
+      response.write(body);
       response.finish();
     } catch (e) {
       info(e);
     }
   }
   const configPath = "/v1/";
   const changesPath = "/v1/buckets/monitor/collections/changes/records";
   const recordsPath  = "/v1/buckets/main/collections/password-fields/records";
@@ -325,17 +327,49 @@ add_task(async function test_telemetry_r
 
   const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
 
   try {
     await client.maybeSync(10000);
   } catch (e) {}
 
   const endHistogram = getUptakeTelemetrySnapshot(client.identifier);
-  const expectedIncrements = {[UptakeTelemetry.STATUS.SYNC_ERROR]: 1};
+  const expectedIncrements = {[UptakeTelemetry.STATUS.SERVER_ERROR]: 1};
+  checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
+});
+add_task(clear_state);
+
+add_task(async function test_telemetry_reports_if_parsing_fails() {
+  const collection = await client.openCollection();
+  await collection.db.saveLastModified(10000);
+
+  const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
+
+  try {
+    await client.maybeSync(10001);
+  } catch (e) { }
+
+  const endHistogram = getUptakeTelemetrySnapshot(client.identifier);
+  const expectedIncrements = { [UptakeTelemetry.STATUS.PARSE_ERROR]: 1 };
+  checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
+});
+add_task(clear_state);
+
+add_task(async function test_telemetry_reports_if_fetching_signature_fails() {
+  const collection = await client.openCollection();
+  await collection.db.saveLastModified(11000);
+
+  const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
+
+  try {
+    await client.maybeSync(11001);
+  } catch (e) { }
+
+  const endHistogram = getUptakeTelemetrySnapshot(client.identifier);
+  const expectedIncrements = { [UptakeTelemetry.STATUS.SERVER_ERROR]: 1 };
   checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
 });
 add_task(clear_state);
 
 add_task(async function test_telemetry_reports_unknown_errors() {
   const backup = client.openCollection;
   client.openCollection = () => { throw new Error("Internal"); };
   const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
@@ -521,16 +555,58 @@ function getSampleResponse(req, port) {
       ],
       "status": {status: 503, statusText: "Service Unavailable"},
       "responseBody": {
         code: 503,
         errno: 999,
         error: "Service Unavailable",
       },
     },
+    "GET:/v1/buckets/main/collections/password-fields/records?_expected=10001&_sort=-last_modified&_since=10000": {
+      "sampleHeaders": [
+        "Access-Control-Allow-Origin: *",
+        "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff",
+        "Content-Type: application/json; charset=UTF-8",
+        "Server: waitress",
+        "Etag: \"10001\"",
+      ],
+      "status": { status: 200, statusText: "OK" },
+      "responseBody": "<invalid json",
+    },
+    "GET:/v1/buckets/main/collections/password-fields/records?_expected=11001&_sort=-last_modified&_since=11000": {
+      "sampleHeaders": [
+        "Access-Control-Allow-Origin: *",
+        "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff",
+        "Content-Type: application/json; charset=UTF-8",
+        "Server: waitress",
+      ],
+      "status": { status: 503, statusText: "Service Unavailable" },
+      "responseBody": {
+        "data": [{
+          "id": "c4f021e3-f68c-4269-ad2a-d4ba87762b35",
+          "last_modified": 4000,
+          "website": "https://www.eff.org",
+          "selector": "#pwd",
+        }],
+      },
+    },
+    "GET:/v1/buckets/main/collections/password-fields?_expected=11001": {
+      "sampleHeaders": [
+        "Access-Control-Allow-Origin: *",
+        "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff",
+        "Content-Type: application/json; charset=UTF-8",
+        "Server: waitress",
+      ],
+      "status": { status: 503, statusText: "Service Unavailable" },
+      "responseBody": {
+        code: 503,
+        errno: 999,
+        error: "Service Unavailable",
+      },
+    },
     "GET:/v1/buckets/monitor/collections/changes/records?collection=password-fields&bucket=main": {
       "sampleHeaders": [
         "Access-Control-Allow-Origin: *",
         "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff",
         "Content-Type: application/json; charset=UTF-8",
         "Server: waitress",
         `Date: ${new Date().toUTCString()}`,
         "Etag: \"1338\"",
--- a/services/settings/test/unit/test_remote_settings_poll.js
+++ b/services/settings/test/unit/test_remote_settings_poll.js
@@ -58,16 +58,17 @@ function run_test() {
 }
 
 add_task(clear_state);
 
 
 add_task(async function test_an_event_is_sent_on_start() {
   server.registerPathHandler(CHANGES_PATH, (request, response) => {
     response.write(JSON.stringify({ data: [] }));
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.setHeader("ETag", '"42"');
     response.setHeader("Date", (new Date()).toUTCString());
     response.setStatusLine(null, 200, "OK");
   });
   let notificationObserved = null;
   const observer = {
     observe(aSubject, aTopic, aData) {
       Services.obs.removeObserver(this, "remote-settings:changes-poll-start");
@@ -230,16 +231,17 @@ add_task(async function test_expected_ti
       bucket: "main",
       collection: "with-cache-busting",
     }];
     if (request.queryString == `_expected=${encodeURIComponent('"42"')}`) {
       response.write(JSON.stringify({
         data: entries,
       }));
     }
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.setHeader("ETag", '"1100"');
     response.setHeader("Date", (new Date()).toUTCString());
     response.setStatusLine(null, 200, "OK");
   }
   server.registerPathHandler(CHANGES_PATH, withCacheBust);
 
   const c = RemoteSettings("with-cache-busting");
   let maybeSyncCalled = false;
@@ -258,16 +260,17 @@ add_task(async function test_client_last
       data: [{
         id: "695c2407-de79-4408-91c7-70720dd59d78",
         last_modified: 1100,
         host: "localhost",
         bucket: "main",
         collection: "models-recipes",
       }],
     }));
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.setHeader("ETag", '"42"');
     response.setHeader("Date", (new Date()).toUTCString());
     response.setStatusLine(null, 200, "OK");
   });
 
   const c = RemoteSettings("models-recipes");
   c.maybeSync = () => {};
 
@@ -302,16 +305,17 @@ add_task(async function test_success_wit
       }));
       response.setHeader("ETag", '"43"');
     } else {
       response.write(JSON.stringify({
         data: entries,
       }));
       response.setHeader("ETag", '"42"');
     }
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.setHeader("Date", (new Date()).toUTCString());
     response.setStatusLine(null, 200, "OK");
   }
   server.registerPathHandler(CHANGES_PATH, partialList);
 
   const c = RemoteSettings("poll-test-collection");
   let maybeSyncCount = 0;
   c.maybeSync = () => { maybeSyncCount++; };
@@ -322,30 +326,65 @@ add_task(async function test_success_wit
   // On the second call, the server does not mention the poll-test-collection
   // and maybeSync() is not called.
   Assert.equal(maybeSyncCount, 1, "maybeSync should not be called twice");
 });
 add_task(clear_state);
 
 
 add_task(async function test_server_bad_json() {
+  const startHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
+
   function simulateBadJSON(request, response) {
     response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.write("<html></html>");
     response.setStatusLine(null, 200, "OK");
   }
   server.registerPathHandler(CHANGES_PATH, simulateBadJSON);
 
   let error;
   try {
     await RemoteSettings.pollChanges();
   } catch (e) {
     error = e;
   }
   Assert.ok(/JSON.parse: unexpected character/.test(error.message));
+
+  const endHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
+  const expectedIncrements = {
+    [UptakeTelemetry.STATUS.PARSE_ERROR]: 1,
+  };
+  checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
+});
+add_task(clear_state);
+
+
+add_task(async function test_server_bad_content_type() {
+  const startHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
+
+  function simulateBadContentType(request, response) {
+    response.setHeader("Content-Type", "text/html");
+    response.write("<html></html>");
+    response.setStatusLine(null, 200, "OK");
+  }
+  server.registerPathHandler(CHANGES_PATH, simulateBadContentType);
+
+  let error;
+  try {
+    await RemoteSettings.pollChanges();
+  } catch (e) {
+    error = e;
+  }
+  Assert.ok(/Unexpected content-type/.test(error.message));
+
+  const endHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY);
+  const expectedIncrements = {
+    [UptakeTelemetry.STATUS.CONTENT_ERROR]: 1,
+  };
+  checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
 });
 add_task(clear_state);
 
 
 add_task(async function test_server_404_response() {
   function simulateDummy404(request, response) {
     response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.write("<html></html>");
@@ -635,16 +674,17 @@ add_task(async function test_adding_clie
       response.setStatusLine(null, 304, "Not Modified");
     } else {
       response.write(JSON.stringify({
         data: entries,
       }));
       response.setHeader("ETag", '"42"');
       response.setStatusLine(null, 200, "OK");
     }
+    response.setHeader("Content-Type", "application/json; charset=UTF-8");
     response.setHeader("Date", (new Date()).toUTCString());
   }
   server.registerPathHandler(CHANGES_PATH, serve200or304);
 
   // Poll once, without any client for "a-collection"
   await RemoteSettings.pollChanges();
 
   // Register a new client.