Bug 1416374 - Handle getting an abortincoming error in sync telemetry r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Fri, 10 Nov 2017 18:02:58 -0500
changeset 696690 124cdba100b5f31e8597a20835d064304467e150
parent 695096 26d7a3a91c8596ca6834effec4b77a2c13d5f622
child 739900 ef9637de0058bd137f3481ae89fddaa6ecb90419
push id88762
push userbmo:tchiovoloni@mozilla.com
push dateFri, 10 Nov 2017 23:05:03 +0000
reviewersmarkh
bugs1416374
milestone58.0a1
Bug 1416374 - Handle getting an abortincoming error in sync telemetry r?markh MozReview-Commit-ID: LFmbkGa4ypu
services/sync/modules/telemetry.js
services/sync/tests/unit/test_bookmark_engine.js
--- a/services/sync/modules/telemetry.js
+++ b/services/sync/modules/telemetry.js
@@ -673,16 +673,22 @@ class SyncTelemetryImpl {
     }
   }
 
   // Transform an exception into a standard description. Exposed here for when
   // this module isn't directly responsible for knowing the transform should
   // happen (for example, when including an error in the |extra| field of
   // event telemetry)
   transformError(error) {
+    // Certain parts of sync will use this pattern as a way to communicate to
+    // processIncoming to abort the processing. However, there's no guarantee
+    // this can only happen then.
+    if (typeof error == "object" && error.code && error.cause) {
+      error = error.cause;
+    }
     if (Async.isShutdownException(error)) {
       return { name: "shutdownerror" };
     }
 
     if (typeof error === "string") {
       if (error.startsWith("error.")) {
         // This is hacky, but I can't imagine that it's not also accurate.
         return { name: "othererror", error };
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -538,16 +538,29 @@ add_task(async function test_bookmark_gu
   err = undefined;
   try {
     await engine._processIncoming();
   } catch (ex) {
     err = ex;
   }
   do_check_eq(err, "Nooo");
 
+  _("Sync the engine and validate that we didn't put the error code in the wrong place");
+  let ping;
+  try {
+    // Clear processIncoming so that we initialize the guid map inside uploadOutgoing
+    engine._processIncoming = async function() {};
+    await sync_engine_and_validate_telem(engine, true, p => { ping = p; });
+  } catch (e) {}
+
+  deepEqual(ping.engines.find(e => e.name == "bookmarks").failureReason, {
+    name: "unexpectederror",
+    error: "Nooo"
+  });
+
   PlacesUtils.promiseBookmarksTree = pbt;
   await PlacesSyncUtils.bookmarks.reset();
   await promiseStopServer(server);
 });
 
 add_task(async function test_bookmark_tag_but_no_uri() {
   _("Ensure that a bookmark record with tags, but no URI, doesn't throw an exception.");