Bug 1416374 - Handle getting an abortincoming error in sync telemetry r=markh
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Fri, 10 Nov 2017 18:02:58 -0500
changeset 391574 a279f3cd1b8b581d886dff12739ef1f61ec0f023
parent 391573 24e91eb85a31fedd6a53f4e6e5925e967f8a961a
child 391575 b3883358e0182cd158341f62e87f31c3b69f3c5d
push id97297
push userncsoregi@mozilla.com
push dateMon, 13 Nov 2017 23:01:19 +0000
treeherdermozilla-inbound@1f7a23820597 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1416374
milestone59.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 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
@@ -542,16 +542,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.");