Backed out changeset 68e32f5b50e7 (bug 1444554) for failing its own test. r=backout

MozReview-Commit-ID: C6TooaguKcm

browser/modules/BrowserErrorReporter.jsm

@@ -24,13 +24,6 @@ const PREF_SAMPLE_RATE = "browser.chrome.errorReporter.sampleRate";
 const PREF_SUBMIT_URL = "browser.chrome.errorReporter.submitUrl";
 const SDK_NAME = "firefox-error-reporter";
 const SDK_VERSION = "1.0.0";
-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";
-
 
 // https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIScriptError#Categories
 const REPORTED_CATEGORIES = new Set([
@@ -52,13 +45,6 @@ const PLATFORM_NAMES = {
   android: "Android",
 };
 
-// Filename URI regexes that we are okay with reporting to Telemetry. URIs not
-// matching these patterns may contain local file paths.
-const TELEMETRY_REPORTED_PATTERNS = new Set([
-  /^resource:\/\/(?:\/|gre)/,
-  /^chrome:\/\/(?:global|browser|devtools)/,
-]);
-
 /**
  * Collects nsIScriptError messages logged to the browser console and reports
  * them to a remotely-hosted error collection service.
@@ -124,13 +110,6 @@ class BrowserErrorReporter {
       false,
       this.handleEnabledPrefChanged.bind(this),
     );
-    XPCOMUtils.defineLazyPreferenceGetter(
-      this,
-      "sampleRatePref",
-      PREF_SAMPLE_RATE,
-      "0.0",
-      this.handleSampleRatePrefChanged.bind(this),
-    );
   }
 
   /**
@@ -173,19 +152,6 @@ class BrowserErrorReporter {
     }
   }
 
-  handleSampleRatePrefChanged(prefName, previousValue, newValue) {
-    Services.telemetry.scalarSet(TELEMETRY_ERROR_SAMPLE_RATE, newValue);
-  }
-
-  shouldReportFilename(filename) {
-    for (const pattern of TELEMETRY_REPORTED_PATTERNS) {
-      if (filename.match(pattern)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   async observe(message) {
     try {
       message.QueryInterface(Ci.nsIScriptError);
@@ -199,21 +165,8 @@ class BrowserErrorReporter {
       return;
     }
 
-    // Record that we collected an error prior to applying the sample rate
-    Services.telemetry.scalarAdd(TELEMETRY_ERROR_COLLECTED, 1);
-    if (message.stack) {
-      Services.telemetry.scalarAdd(TELEMETRY_ERROR_COLLECTED_STACK, 1);
-    }
-    if (message.sourceName) {
-      let filename = "FILTERED";
-      if (this.shouldReportFilename(message.sourceName)) {
-        filename = message.sourceName;
-      }
-      Services.telemetry.keyedScalarAdd(TELEMETRY_ERROR_COLLECTED_FILENAME, filename.slice(0, 69), 1);
-    }
-
     // Sample the amount of errors we send out
-    const sampleRate = Number.parseFloat(this.sampleRatePref);
+    const sampleRate = Number.parseFloat(Services.prefs.getCharPref(PREF_SAMPLE_RATE));
     if (!Number.isFinite(sampleRate) || (Math.random() >= sampleRate)) {
       return;
     }
@@ -256,10 +209,8 @@ class BrowserErrorReporter {
         referrer: "https://fake.mozilla.org",
         body: JSON.stringify(requestBody)
       });
-      Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED, 1);
       this.logger.debug("Sent error successfully.");
     } catch (error) {
-      Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED_FAIL, 1);
       this.logger.warn(`Failed to send error: ${error}`);
     }
   }

browser/modules/test/browser/browser_BrowserErrorReporter.js

@@ -66,12 +66,6 @@ function fetchPassedError(fetchSpy, message) {
   return fetchCallForMessage(fetchSpy, message) !== null;
 }
 
-add_task(async function testSetup() {
-  const canRecordExtended = Services.telemetry.canRecordExtended;
-  Services.telemetry.canRecordExtended = true;
-  registerCleanupFunction(() => Services.telemetry.canRecordExtended = canRecordExtended);
-});
-
 add_task(async function testInitPrefDisabled() {
   let listening = false;
   const reporter = new BrowserErrorReporter({
@@ -455,138 +449,3 @@ add_task(async function testExtensionTag() {
   body = JSON.parse(call.args[1].body);
   is(body.tags.isExtensionError, undefined, "Normal errors do not have an isExtensionError tag.");
 });
-
-add_task(async function testScalars() {
-  const fetchStub = sinon.stub();
-  const reporter = new BrowserErrorReporter(fetchStub);
-  await SpecialPowers.pushPrefEnv({set: [
-    [PREF_ENABLED, true],
-    [PREF_SAMPLE_RATE, "1.0"],
-  ]});
-
-  Services.telemetry.clearScalars();
-
-  const messages = [
-    createScriptError({message: "No name"}),
-    createScriptError({message: "Also no name", sourceName: "resource://gre/modules/Foo.jsm"}),
-    createScriptError({message: "More no name", sourceName: "resource://gre/modules/Bar.jsm"}),
-    createScriptError({message: "Yeah sures", sourceName: "unsafe://gre/modules/Bar.jsm"}),
-    createScriptError({
-      message: "long",
-      sourceName: "resource://gre/modules/long/long/long/long/long/long/long/long/long/long/",
-    }),
-    {message: "Not a scripterror instance."},
-
-    // 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"]]});
-  await reporter.observe(createScriptError({message: "Additionally no name"}));
-
-  await SpecialPowers.pushPrefEnv({set: [[PREF_SAMPLE_RATE, "1.0"]]});
-  fetchStub.throws(new Error("Could not report"));
-  await reporter.observe(createScriptError({message: "Maybe name?"}));
-
-  const optin = Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN;
-  const scalars = Services.telemetry.snapshotScalars(optin, false).parent;
-  is(
-    scalars[TELEMETRY_ERROR_COLLECTED],
-    8,
-    `${TELEMETRY_ERROR_COLLECTED} is incremented when an error is collected.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_SAMPLE_RATE],
-    "1.0",
-    `${TELEMETRY_ERROR_SAMPLE_RATE} contains the last sample rate used.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_REPORTED],
-    6,
-    `${TELEMETRY_ERROR_REPORTED} is incremented when an error is reported.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_REPORTED_FAIL],
-    1,
-    `${TELEMETRY_ERROR_REPORTED_FAIL} is incremented when an error fails to be reported.`,
-  );
-  is(
-    scalars[TELEMETRY_ERROR_COLLECTED_STACK],
-    1,
-    `${TELEMETRY_ERROR_REPORTED_FAIL} is incremented when an error with a stack trace is collected.`,
-  );
-
-  const keyedScalars = Services.telemetry.snapshotKeyedScalars(optin, false).parent;
-  Assert.deepEqual(
-    keyedScalars[TELEMETRY_ERROR_COLLECTED_FILENAME],
-    {
-      "FILTERED": 1,
-      "resource://gre/modules/Foo.jsm": 1,
-      "resource://gre/modules/Bar.jsm": 1,
-      // Cut off at 70-character limit
-      "resource://gre/modules/long/long/long/long/long/long/long/long/long/l": 1,
-    },
-    `${TELEMETRY_ERROR_COLLECTED_FILENAME} is incremented when an error is collected.`,
-  );
-
-  resetConsole();
-});
-
-add_task(async function testCollectedFilenameScalar() {
-  const fetchStub = sinon.stub();
-  const reporter = new BrowserErrorReporter(fetchStub);
-  await SpecialPowers.pushPrefEnv({set: [
-    [PREF_ENABLED, true],
-    [PREF_SAMPLE_RATE, "1.0"],
-  ]});
-
-  const testCases = [
-    ["chrome://unknown/module.jsm", false],
-    ["resource://unknown/module.jsm", false],
-    ["unknown://unknown/module.jsm", false],
-
-    ["resource://gre/modules/Foo.jsm", true],
-    ["resource:///modules/Foo.jsm", true],
-    ["chrome://global/Foo.jsm", true],
-    ["chrome://browser/Foo.jsm", true],
-    ["chrome://devtools/Foo.jsm", true],
-  ];
-
-  for (const [filename, shouldMatch] of testCases) {
-    Services.telemetry.clearScalars();
-
-    // Use observe to avoid errors from other code messing up our counts.
-    await reporter.observe(createScriptError({
-      message: "Fine",
-      sourceName: filename,
-    }));
-
-    const keyedScalars = (
-      Services.telemetry.snapshotKeyedScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false).parent
-    );
-
-    let matched = null;
-    if (shouldMatch) {
-      matched = keyedScalars[TELEMETRY_ERROR_COLLECTED_FILENAME][filename] === 1;
-    } else {
-      matched = keyedScalars[TELEMETRY_ERROR_COLLECTED_FILENAME].FILTERED === 1;
-    }
-
-    ok(
-      matched,
-      shouldMatch
-        ? `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a key for ${filename}.`
-        : `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a FILTERED key for ${filename}.`,
-    );
-  }
-
-  resetConsole();
-});

toolkit/components/telemetry/Scalars.yaml

@@ -1491,94 +1491,6 @@ sw:
       - 'main'
       - 'content'
 
-# The following section contains the BrowserErrorReporter scalars.
-browser.errors:
-  collected_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that were collected locally.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  collected_with_stack_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of browser chrome JS errors that were collected locally and had
-      a usable stack trace.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  reported_success_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that were reported to the
-      remote collection service.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  reported_failure_count:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that we attempted to report to
-      the remote collection service, but failed to.
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  sample_rate:
-    bug_numbers:
-      - 1444554
-    description: >
-      The sample rate at which collected errors were reported.
-    expires: "64"
-    kind: string
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
-  collected_count_by_filename:
-    bug_numbers:
-      - 1444554
-    description: >
-      The count of all browser chrome JS errors that were collected locally,
-      keyed by the filename of the file in which the error occurred. Collected
-      filenames are limited to specific paths under the resource:// and
-      chrome:// protocols; non-matching filenames are reported as "FILTERED".
-      Long filenames are truncated to the first 70 characters.
-    keyed: true
-    expires: "64"
-    kind: uint
-    notification_emails:
-      - nightly-js-errors@mozilla.com
-      - mkelly@mozilla.com
-    record_in_processes:
-      - 'main'
-
 # The following section is for probes testing the Telemetry system. They will not be
 # submitted in pings and are only used for testing.
 telemetry.test: