Bug 866253 - Provider manager calls undefined function this._recordError; r=rnewman
authorGregory Szorc <gps@mozilla.com>
Fri, 26 Apr 2013 18:11:33 -0700
changeset 141129 17e8172c812a985ea2e98a8039e799ce3f3a120b
parent 140897 34fabb5ccebfe170f84aaa84ee08fae96f40e068
child 141130 059a39ad762c6eb727698f587c16074a24d4dfbd
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs866253
milestone23.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 866253 - Provider manager calls undefined function this._recordError; r=rnewman
services/metrics/modules-testing/mocks.jsm
services/metrics/providermanager.jsm
services/metrics/tests/xpcshell/test_metrics_provider_manager.js
--- a/services/metrics/modules-testing/mocks.jsm
+++ b/services/metrics/modules-testing/mocks.jsm
@@ -3,16 +3,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 this.EXPORTED_SYMBOLS = [
   "DummyMeasurement",
   "DummyProvider",
   "DummyConstantProvider",
+  "DummyPullOnlyThrowsOnInitProvider",
+  "DummyThrowOnInitProvider",
+  "DummyThrowOnShutdownProvider",
 ];
 
 const {utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/commonjs/sdk/core/promise.js");
 Cu.import("resource://gre/modules/Metrics.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
@@ -93,19 +96,59 @@ DummyProvider.prototype = {
     this.collectDailyCount++;
 
     return Promise.resolve();
   },
 };
 
 
 this.DummyConstantProvider = function () {
-  DummyProvider.call(this, "DummyConstantProvider");
+  DummyProvider.call(this, this.name);
 }
 
 DummyConstantProvider.prototype = {
   __proto__: DummyProvider.prototype,
 
   name: "DummyConstantProvider",
 
   pullOnly: true,
 };
 
+this.DummyThrowOnInitProvider = function () {
+  DummyProvider.call(this, "DummyThrowOnInitProvider");
+
+  throw new Error("Dummy Error");
+};
+
+this.DummyThrowOnInitProvider.prototype = {
+  __proto__: DummyProvider.prototype,
+
+  name: "DummyThrowOnInitProvider",
+};
+
+this.DummyPullOnlyThrowsOnInitProvider = function () {
+  DummyConstantProvider.call(this);
+
+  throw new Error("Dummy Error");
+};
+
+this.DummyPullOnlyThrowsOnInitProvider.prototype = {
+  __proto__: DummyConstantProvider.prototype,
+
+  name: "DummyPullOnlyThrowsOnInitProvider",
+};
+
+this.DummyThrowOnShutdownProvider = function () {
+  DummyProvider.call(this, "DummyThrowOnShutdownProvider");
+};
+
+this.DummyThrowOnShutdownProvider.prototype = {
+  __proto__: DummyProvider.prototype,
+
+  name: "DummyThrowOnShutdownProvider",
+
+  pullOnly: true,
+
+  onShutdown: function () {
+    throw new Error("Dummy shutdown error");
+  },
+};
+
--- a/services/metrics/providermanager.jsm
+++ b/services/metrics/providermanager.jsm
@@ -123,18 +123,19 @@ this.ProviderManager.prototype = Object.
         let ns = {};
         Cu.import(uri, ns);
 
         let promise = this.registerProviderFromType(ns[entry]);
         if (promise) {
           promises.push(promise);
         }
       } catch (ex) {
-        this._recordError("Error registering provider from category manager : " +
-                          entry + ": ", ex);
+        this._recordProviderError(entry,
+                                  "Error registering provider from category manager",
+                                  ex);
         continue;
       }
     }
 
     return Task.spawn(function wait() {
       for (let promise of promises) {
         yield promise;
       }
@@ -282,17 +283,19 @@ this.ProviderManager.prototype = Object.
         try {
           let provider = this._initProviderFromType(providerType);
 
           // This is a no-op if the provider is already registered. So, the
           // only overhead is constructing an instance. This should be cheap
           // and isn't worth optimizing.
           yield this.registerProvider(provider);
         } catch (ex) {
-          this._recordError("Error registering pull-only provider", ex);
+          this._recordProviderError(providerType.prototype.name,
+                                    "Error registering pull-only provider",
+                                    ex);
         }
       }
 
       // It's possible we changed state while registering. Only mark as
       // registered if we didn't change state.
       if (this._pullOnlyProvidersState == this.PULL_ONLY_REGISTERING) {
         this._pullOnlyProvidersState = this.PULL_ONLY_REGISTERED;
         this._pullOnlyProvidersCurrentPromise = null;
@@ -365,18 +368,19 @@ this.ProviderManager.prototype = Object.
         }
 
         this._log.info("Shutting down pull-only provider: " +
                        provider.name);
 
         try {
           yield provider.shutdown();
         } catch (ex) {
-          this._recordError("Error when shutting down provider: " +
-                            provider.name, ex);
+          this._recordProviderError(provider.name,
+                                    "Error when shutting down provider",
+                                    ex);
         } finally {
           this.unregisterProvider(provider.name);
         }
       }
 
       if (this._pullOnlyProvidersState == this.PULL_ONLY_UNREGISTERING) {
         this._pullOnlyProvidersState = this.PULL_ONLY_NOT_REGISTERED;
         this._pullOnlyProvidersCurrentPromise = null;
--- a/services/metrics/tests/xpcshell/test_metrics_provider_manager.js
+++ b/services/metrics/tests/xpcshell/test_metrics_provider_manager.js
@@ -1,15 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
+Cu.import("resource://gre/modules/commonjs/sdk/core/promise.js");
 Cu.import("resource://gre/modules/Metrics.jsm");
 Cu.import("resource://testing-common/services/metrics/mocks.jsm");
 
 const PULL_ONLY_TESTING_CATEGORY = "testing-only-pull-only-providers";
 
 function run_test() {
   let cm = Cc["@mozilla.org/categorymanager;1"]
              .getService(Ci.nsICategoryManager);
@@ -263,8 +264,93 @@ add_task(function test_pull_only_registe
 
   yield manager.ensurePullOnlyProvidersRegistered();
   manager.ensurePullOnlyProvidersUnregistered();
   yield manager.ensurePullOnlyProvidersRegistered();
   do_check_eq(manager.providers.length, 2);
 
   yield storage.close();
 });
+
+// Re-use database for perf reasons.
+const REGISTRATION_ERRORS_DB = "registration_errors";
+
+add_task(function test_category_manager_registration_error() {
+  let storage = yield Metrics.Storage(REGISTRATION_ERRORS_DB);
+  let manager = new Metrics.ProviderManager(storage);
+
+  let cm = Cc["@mozilla.org/categorymanager;1"]
+             .getService(Ci.nsICategoryManager);
+  cm.addCategoryEntry("registration-errors", "DummyThrowOnInitProvider",
+                      "resource://testing-common/services/metrics/mocks.jsm",
+                      false, true);
+
+  let deferred = Promise.defer();
+  let errorCount = 0;
+
+  manager.onProviderError = function (msg) {
+    errorCount++;
+    deferred.resolve(msg);
+  };
+
+  yield manager.registerProvidersFromCategoryManager("registration-errors");
+  do_check_eq(manager.providers.length, 0);
+  do_check_eq(errorCount, 1);
+
+  let msg = yield deferred.promise;
+  do_check_true(msg.contains("Provider error: DummyThrowOnInitProvider: " +
+                             "Error registering provider from category manager: Dummy Error"));
+
+  yield storage.close();
+});
+
+add_task(function test_pull_only_registration_error() {
+  let storage = yield Metrics.Storage(REGISTRATION_ERRORS_DB);
+  let manager = new Metrics.ProviderManager(storage);
+
+  let deferred = Promise.defer();
+  let errorCount = 0;
+
+  manager.onProviderError = function (msg) {
+    errorCount++;
+    deferred.resolve(msg);
+  };
+
+  yield manager.registerProviderFromType(DummyPullOnlyThrowsOnInitProvider);
+  do_check_eq(errorCount, 0);
+
+  yield manager.ensurePullOnlyProvidersRegistered();
+  do_check_eq(errorCount, 1);
+
+  let msg = yield deferred.promise;
+  do_check_true(msg.contains("Provider error: DummyPullOnlyThrowsOnInitProvider: " +
+                             "Error registering pull-only provider: Dummy Error"));
+
+  yield storage.close();
+});
+
+add_task(function test_error_during_shutdown() {
+  let storage = yield Metrics.Storage(REGISTRATION_ERRORS_DB);
+  let manager = new Metrics.ProviderManager(storage);
+
+  let deferred = Promise.defer();
+  let errorCount = 0;
+
+  manager.onProviderError = function (msg) {
+    errorCount++;
+    deferred.resolve(msg);
+  };
+
+  yield manager.registerProviderFromType(DummyThrowOnShutdownProvider);
+  yield manager.registerProviderFromType(DummyProvider);
+  do_check_eq(errorCount, 0);
+  do_check_eq(manager.providers.length, 1);
+
+  yield manager.ensurePullOnlyProvidersRegistered();
+  do_check_eq(errorCount, 0);
+  yield manager.ensurePullOnlyProvidersUnregistered();
+  do_check_eq(errorCount, 1);
+  let msg = yield deferred.promise;
+  do_check_true(msg.contains("Provider error: DummyThrowOnShutdownProvider: " +
+                             "Error when shutting down provider: Dummy shutdown error"));
+
+  yield storage.close();
+});