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 130182 17e8172c812a985ea2e98a8039e799ce3f3a120b
parent 129950 34fabb5ccebfe170f84aaa84ee08fae96f40e068
child 130183 059a39ad762c6eb727698f587c16074a24d4dfbd
push idunknown
push userunknown
push dateunknown
reviewersrnewman
bugs866253
milestone23.0a1
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();
+});