Bug 1432995 - Check for HTTP error codes before error strings in the sync ping. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Mon, 29 Jan 2018 15:01:04 -0500
changeset 748410 c634ab3d891373ed48595763a01bb0bff825d3f2
parent 747752 8275c46e2880469ebb154f6b0c49bb1a97630757
push id97162
push userbmo:tchiovoloni@mozilla.com
push dateMon, 29 Jan 2018 20:15:59 +0000
reviewersmarkh
bugs1432995
milestone60.0a1
Bug 1432995 - Check for HTTP error codes before error strings in the sync ping. r?markh MozReview-Commit-ID: KU15HVElChU
services/sync/modules/telemetry.js
services/sync/tests/unit/test_telemetry.js
--- a/services/sync/modules/telemetry.js
+++ b/services/sync/modules/telemetry.js
@@ -698,32 +698,32 @@ class SyncTelemetryImpl {
       if (error.startsWith("error.")) {
         // This is hacky, but I can't imagine that it's not also accurate.
         return { name: "othererror", error };
       }
       error = cleanErrorMessage(error);
       return { name: "unexpectederror", error };
     }
 
-    if (error.failureCode) {
-      return { name: "othererror", error: error.failureCode };
-    }
-
     if (error instanceof AuthenticationError) {
       return { name: "autherror", from: error.source };
     }
 
     let httpCode = error.status ||
       (error.response && error.response.status) ||
       error.code;
 
     if (httpCode) {
       return { name: "httperror", code: httpCode };
     }
 
+    if (error.failureCode) {
+      return { name: "othererror", error: error.failureCode };
+    }
+
     if (error.result) {
       return { name: "nserror", code: error.result };
     }
     // It's probably an Error object, but it also could be some
     // other object that may or may not override toString to do
     // something useful.
     let msg = String(error);
     if (msg.startsWith("[object")) {
--- a/services/sync/tests/unit/test_telemetry.js
+++ b/services/sync/tests/unit/test_telemetry.js
@@ -133,25 +133,25 @@ add_task(async function test_processInco
     } catch (ex) {
       error = ex;
     }
     ok(!!error);
     ok(!!pingPayload);
 
     equal(fullPing.uid, "f".repeat(32)); // as setup by SyncTestingInfrastructure
     deepEqual(pingPayload.failureReason, {
-      name: "othererror",
-      error: "error.engine.reason.record_download_fail"
+      name: "httperror",
+      code: 500,
     });
 
     equal(pingPayload.engines.length, 1);
     equal(pingPayload.engines[0].name, "bookmarks");
     deepEqual(pingPayload.engines[0].failureReason, {
-      name: "othererror",
-      error: "error.engine.reason.record_download_fail"
+      name: "httperror",
+      code: 500,
     });
 
   } finally {
     await store.wipe();
     await cleanAndGo(engine, server);
   }
 });
 
@@ -308,18 +308,18 @@ add_task(async function test_sync_partia
     _(`test_sync_partialUpload: Rotary tracker contents at second sync: ${
       JSON.stringify(engine._tracker.changedIDs)}`);
     try {
       // should throw
       await sync_engine_and_validate_telem(engine, true, errPing => ping = errPing);
     } catch (e) {}
     // It would be nice if we had a more descriptive error for this...
     let uploadFailureError = {
-      name: "othererror",
-      error: "error.engine.reason.record_upload_fail"
+      name: "httperror",
+      code: 500,
     };
 
     ok(!!ping);
     deepEqual(ping.failureReason, uploadFailureError);
     equal(ping.engines.length, 1);
     equal(ping.engines[0].name, "rotary");
     deepEqual(ping.engines[0].incoming, {
       failed: 1,