Backed out changeset 8ad87d070ec9 (bug 1451702) for failing on /test/browser/browser_BrowserErrorReporter.js
authorGurzau Raul <rgurzau@mozilla.com>
Fri, 18 May 2018 03:51:13 +0300
changeset 418791 94415f8067ec
parent 418790 4481e5d95c62
child 418792 7d3ae4f78bd1
push id34013
push userdluca@mozilla.com
push date2018-05-18 09:56 +0000
treeherdermozilla-central@11ee70f24ea5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1451702
milestone62.0a1
backs out8ad87d070ec9
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
Backed out changeset 8ad87d070ec9 (bug 1451702) for failing on /test/browser/browser_BrowserErrorReporter.js
browser/modules/BrowserErrorReporter.jsm
browser/modules/test/browser/browser_BrowserErrorReporter.js
--- a/browser/modules/BrowserErrorReporter.jsm
+++ b/browser/modules/BrowserErrorReporter.jsm
@@ -2,17 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/Timer.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
-ChromeUtils.defineModuleGetter(this, "FileUtils", "resource://gre/modules/FileUtils.jsm");
 ChromeUtils.defineModuleGetter(this, "Log", "resource://gre/modules/Log.jsm");
 ChromeUtils.defineModuleGetter(this, "UpdateUtils", "resource://gre/modules/UpdateUtils.jsm");
 
 Cu.importGlobalProperties(["fetch", "URL"]);
 
 var EXPORTED_SYMBOLS = ["BrowserErrorReporter"];
 
 const CONTEXT_LINES = 5;
@@ -244,17 +243,16 @@ class BrowserErrorReporter {
       },
     };
 
     const transforms = [
       addErrorMessage,
       addStacktrace,
       addModule,
       mangleExtensionUrls,
-      mangleFilePaths,
       tagExtensionErrors,
     ];
     for (const transform of transforms) {
       await transform(message, exceptionValue, requestBody);
     }
 
     const url = new URL(Services.prefs.getCharPref(PREF_SUBMIT_URL));
     url.searchParams.set("sentry_client", `${SDK_NAME}/${SDK_VERSION}`);
@@ -268,20 +266,20 @@ class BrowserErrorReporter {
           "Content-Type": "application/json",
           "Accept": "application/json",
         },
         // Sentry throws an auth error without a referrer specified.
         referrer: "https://fake.mozilla.org",
         body: JSON.stringify(requestBody)
       });
       Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED, 1);
-      this.logger.debug(`Sent error "${message.errorMessage}" successfully.`);
+      this.logger.debug("Sent error successfully.");
     } catch (error) {
       Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED_FAIL, 1);
-      this.logger.warn(`Failed to send error "${message.errorMessage}": ${error}`);
+      this.logger.warn(`Failed to send error: ${error}`);
     }
   }
 }
 
 function defaultFetch(...args) {
   // Do not make network requests while running in automation
   if (Cu.isInAutomation) {
     return null;
@@ -364,62 +362,28 @@ function mangleExtensionUrls(message, ex
 
   // Replaces any instances of moz-extension:// URLs with internal UUIDs to use
   // the add-on ID instead.
   function mangleExtURL(string, anchored = true) {
     if (!string) {
       return string;
     }
 
-    const re = new RegExp(`${anchored ? "^" : ""}moz-extension://([^/]+)/`, "g");
+    let re = new RegExp(`${anchored ? "^" : ""}moz-extension://([^/]+)/`, "g");
+
     return string.replace(re, (m0, m1) => {
-      const id = extensions.has(m1) ? extensions.get(m1).id : m1;
+      let id = extensions.has(m1) ? extensions.get(m1).id : m1;
       return `moz-extension://${id}/`;
     });
   }
 
   exceptionValue.value = mangleExtURL(exceptionValue.value, false);
   exceptionValue.module = mangleExtURL(exceptionValue.module);
   for (const frame of exceptionValue.stacktrace.frames) {
     frame.module = mangleExtURL(frame.module);
   }
 }
 
 function tagExtensionErrors(message, exceptionValue, requestBody) {
   requestBody.tags.isExtensionError = !!(
       exceptionValue.module && exceptionValue.module.startsWith("moz-extension://")
   );
 }
-
-/**
- * Alters file: and jar: paths to remove leading file paths that may contain
- * user-identifying or platform-specific paths.
- */
-function mangleFilePaths(message, exceptionValue) {
-  function transformFilePath(path) {
-    try {
-      const uri = Services.io.newURI(path);
-      if (uri.schemeIs("jar")) {
-        return uri.filePath;
-      }
-      if (uri.schemeIs("file")) {
-        const prefixes = {
-          greDir: FileUtils.getDir("GreD", []).path,
-          profileDir: FileUtils.getDir("ProfD", []).path,
-        };
-        for (const [name, prefix] of Object.entries(prefixes)) {
-          if (uri.filePath.startsWith(prefix)) {
-            return uri.filePath.replace(prefix, `[${name}]`);
-          }
-        }
-
-        return "[UNKNOWN_LOCAL_FILEPATH]";
-      }
-    } catch (err) {}
-
-    return path;
-  }
-
-  exceptionValue.module = transformFilePath(exceptionValue.module);
-  for (const frame of exceptionValue.stacktrace.frames) {
-    frame.module = transformFilePath(frame.module);
-  }
-}
--- a/browser/modules/test/browser/browser_BrowserErrorReporter.js
+++ b/browser/modules/test/browser/browser_BrowserErrorReporter.js
@@ -1,16 +1,15 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
+ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm", this);
 ChromeUtils.import("resource:///modules/BrowserErrorReporter.jsm", this);
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm", this);
-ChromeUtils.import("resource://gre/modules/FileUtils.jsm", this);
-ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm", this);
 
 /* global sinon */
 Services.scriptloader.loadSubScript("resource://testing-common/sinon-2.3.2.js");
 registerCleanupFunction(function() {
   delete window.sinon;
 });
 
 const PREF_ENABLED = "browser.chrome.errorReporter.enabled";
@@ -21,55 +20,29 @@ const PREF_SUBMIT_URL = "browser.chrome.
 const TELEMETRY_ERROR_COLLECTED = "browser.errors.collected_count";
 const TELEMETRY_ERROR_COLLECTED_FILENAME = "browser.errors.collected_count_by_filename";
 const TELEMETRY_ERROR_COLLECTED_STACK = "browser.errors.collected_with_stack_count";
 const TELEMETRY_ERROR_REPORTED = "browser.errors.reported_success_count";
 const TELEMETRY_ERROR_REPORTED_FAIL = "browser.errors.reported_failure_count";
 const TELEMETRY_ERROR_SAMPLE_RATE = "browser.errors.sample_rate";
 
 function createScriptError(options = {}) {
-  let scriptError = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError);
+  const scriptError = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError);
   scriptError.init(
     options.message || "",
     "sourceName" in options ? options.sourceName : null,
     options.sourceLine || null,
     options.lineNumber || null,
     options.columnNumber || null,
     options.flags || Ci.nsIScriptError.errorFlag,
     options.category || "chrome javascript",
   );
-
-  // You can't really set the stack of a scriptError in JS, so we shadow it instead.
-  if (options.stack) {
-    scriptError = Object.create(scriptError, {
-      stack: {
-        value: createStack(options.stack),
-      },
-    });
-  }
-
   return scriptError;
 }
 
-function createStack(frames) {
-  for (let k = 0; k < frames.length - 1; k++) {
-    frames[k].parent = frames[k + 1];
-  }
-  return frames[0];
-}
-
-function frame(options = {}) {
-  return Object.assign({
-    functionDisplayName: "fooFunction",
-    source: "resource://modules/BrowserErrorReporter.jsm",
-    line: 5,
-    column: 10,
-  }, options);
-}
-
 function noop() {
   // Do nothing
 }
 
 // Clears the console of any previous messages. Should be called at the end of
 // each test that logs to the console.
 function resetConsole() {
   Services.console.logStringMessage("");
@@ -529,17 +502,22 @@ add_task(async function testScalars() {
     createScriptError({message: "More no name", sourceName: "resource://gre/modules/Bar.jsm"}),
     createScriptError({message: "Yeah sures", sourceName: "unsafe://gre/modules/Bar.jsm"}),
     createScriptError({message: "Addon", sourceName: "moz-extension://foo/Bar.jsm"}),
     createScriptError({
       message: "long",
       sourceName: "resource://gre/modules/long/long/long/long/long/long/long/long/long/long/",
     }),
     {message: "Not a scripterror instance."},
-    createScriptError({message: "Whatever", stack: [frame()]}),
+
+    // No easy way to create an nsIScriptError with a stack, so let's pretend.
+    Object.create(
+      createScriptError({message: "Whatever"}),
+      {stack: {value: new Error().stack}},
+    ),
   ];
 
   // Use observe to avoid errors from other code messing up our counts.
   for (const message of messages) {
     await reporter.observe(message);
   }
 
   await SpecialPowers.pushPrefEnv({set: [[PREF_SAMPLE_RATE, "0.0"]]});
@@ -639,51 +617,8 @@ add_task(async function testCollectedFil
       shouldMatch
         ? `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a key for ${filename}.`
         : `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a FILTERED key for ${filename}.`,
     );
   }
 
   resetConsole();
 });
-
-add_task(async function testFilePathMangle() {
-  const fetchSpy = sinon.spy();
-  const reporter = new BrowserErrorReporter({fetch: fetchSpy});
-  await SpecialPowers.pushPrefEnv({set: [
-    [PREF_ENABLED, true],
-    [PREF_SAMPLE_RATE, "1.0"],
-  ]});
-
-  const greDir = FileUtils.getDir("GreD", []).path;
-  const profileDir = FileUtils.getDir("ProfD", []).path;
-
-  const message = createScriptError({
-    message: "Whatever",
-    sourceName: "file:///path/to/main.jsm",
-    stack: [
-      frame({source: "jar:file:///path/to/jar!/inside/jar.jsm"}),
-      frame({source: `file://${greDir}/defaults/prefs/channel-prefs.js`}),
-      frame({source: `file://${profileDir}/prefs.js`}),
-    ],
-  });
-  await reporter.observe(message);
-
-  const call = fetchCallForMessage(fetchSpy, "Whatever");
-  const body = JSON.parse(call.args[1].body);
-  const exception = body.exception.values[0];
-  is(exception.module, "[UNKNOWN_LOCAL_FILEPATH]", "Unrecognized local file paths are mangled");
-
-  // Stackframe order is reversed from what is in the message.
-  const stackFrames = exception.stacktrace.frames;
-  is(
-    stackFrames[0].module, "[profileDir]/prefs.js",
-    "Paths within the profile directory are preserved but mangled",
-  );
-  is(
-    stackFrames[1].module, "[greDir]/defaults/prefs/channel-prefs.js",
-    "Paths within the GRE directory are preserved but mangled",
-  );
-  is(
-    stackFrames[2].module, "/inside/jar.jsm",
-    "Paths within jarfiles are extracted from the full jar: URL",
-  );
-});