Bug 845431 - Send more errors in FHR payload; r=rnewman

This commit is contained in:
Gregory Szorc 2013-02-27 16:52:26 -08:00
parent 1330f8ee03
commit 6e10265a27
3 changed files with 86 additions and 54 deletions

View File

@ -88,6 +88,8 @@ function AbstractHealthReporter(branch, policy, sessionRecorder) {
this._shutdownComplete = false;
this._shutdownCompleteCallback = null;
this._errors = [];
this._pullOnlyProviders = {};
this._pullOnlyProvidersRegistered = false;
this._lastDailyDate = null;
@ -128,8 +130,7 @@ AbstractHealthReporter.prototype = Object.freeze({
delete this._initHistogram;
delete this._dbOpenHistogram;
this._log.error("Error during initialization: " +
CommonUtils.exceptionStr(error));
this._recordError("Error during initialization", error);
this._initializeHadError = true;
this._initiateShutdown();
this._initializedDeferred.reject(error);
@ -175,6 +176,7 @@ AbstractHealthReporter.prototype = Object.freeze({
this._log.info("Initializing collector.");
this._collector = new Metrics.Collector(this._storage);
this._collector.onProviderError = this._errors.push.bind(this._errors);
this._collectorInProgress = true;
let catString = this._prefs.get("service.providerCategories") || "";
@ -484,8 +486,8 @@ AbstractHealthReporter.prototype = Object.freeze({
promises.push(promise);
}
} catch (ex) {
this._log.warn("Error registering provider from category manager: " +
entry + "; " + CommonUtils.exceptionStr(ex));
this._recordError("Error registering provider from category manager : " +
entry + ": ", ex);
continue;
}
}
@ -525,8 +527,7 @@ AbstractHealthReporter.prototype = Object.freeze({
let provider = this.initProviderFromType(providerType);
yield this.registerProvider(provider);
} catch (ex) {
this._log.warn("Error registering pull-only provider: " +
CommonUtils.exceptionStr(ex));
this._recordError("Error registering pull-only provider", ex);
}
}
}.bind(this)).then(onFinished, onFinished);
@ -555,8 +556,8 @@ AbstractHealthReporter.prototype = Object.freeze({
try {
yield provider.shutdown();
} catch (ex) {
this._log.warn("Error when shutting down provider: " +
CommonUtils.exceptionStr(ex));
this._recordError("Error when shutting down provider: " +
provider.name, ex);
} finally {
this._collector.unregisterProvider(provider.name);
}
@ -564,6 +565,35 @@ AbstractHealthReporter.prototype = Object.freeze({
}.bind(this)).then(onFinished, onFinished);
},
/**
* Record an exception for reporting in the payload.
*
* A side effect is the exception is logged.
*
* Note that callers need to be extra sensitive about ensuring personal
* or otherwise private details do not leak into this. All of the user data
* on the stack in FHR code should be limited to data we were collecting with
* the intent to submit. So, it is covered under the user's consent to use
* the feature.
*
* @param message
* (string) Human readable message describing error.
* @param ex
* (Error) The error that should be captured.
*/
_recordError: function (message, ex) {
let recordMessage = message;
let logMessage = message;
if (ex) {
recordMessage += ": " + ex.message;
logMessage += ": " + CommonUtils.exceptionStr(ex);
}
this._log.warn(logMessage);
this._errors.push(recordMessage);
},
/**
* Collect all measurements for all registered providers.
*/
@ -628,8 +658,8 @@ AbstractHealthReporter.prototype = Object.freeze({
payload = yield this.getJSONPayload(asObject);
} catch (ex) {
error = ex;
this._log.warn("Error collecting and/or retrieving JSON payload: " +
CommonUtils.exceptionStr(ex));
this._collectException("Error collecting and/or retrieving JSON payload",
ex);
} finally {
yield this.ensurePullOnlyProvidersUnregistered();
@ -689,11 +719,6 @@ AbstractHealthReporter.prototype = Object.freeze({
let outputDataDays = o.data.days;
// We need to be careful that data in errors does not leak potentially
// private information.
// FUTURE ask Privacy if we can put exception stacks in here.
let errors = [];
// Guard here in case we don't track this (e.g., on Android).
let lastPingDate = this.lastPingDate;
if (lastPingDate && lastPingDate.getTime() > 0) {
@ -716,9 +741,8 @@ AbstractHealthReporter.prototype = Object.freeze({
// is aware of the measurement version.
serializer = measurement.serializer(measurement.SERIALIZE_JSON);
} catch (ex) {
this._log.warn("Error obtaining serializer for measurement: " + name +
": " + CommonUtils.exceptionStr(ex));
errors.push("Could not obtain serializer: " + name);
this._recordError("Error obtaining serializer for measurement: " +
name, ex);
continue;
}
@ -726,9 +750,8 @@ AbstractHealthReporter.prototype = Object.freeze({
try {
data = yield measurement.getValues();
} catch (ex) {
this._log.warn("Error obtaining data for measurement: " +
name + ": " + CommonUtils.exceptionStr(ex));
errors.push("Could not obtain data: " + name);
this._recordError("Error obtaining data for measurement: " + name,
ex);
continue;
}
@ -736,8 +759,8 @@ AbstractHealthReporter.prototype = Object.freeze({
try {
o.data.last[name] = serializer.singular(data.singular);
} catch (ex) {
this._log.warn("Error serializing data: " + CommonUtils.exceptionStr(ex));
errors.push("Error serializing singular: " + name);
this._recordError("Error serializing singular data: " + name,
ex);
continue;
}
}
@ -762,18 +785,15 @@ AbstractHealthReporter.prototype = Object.freeze({
outputDataDays[dateFormatted][name] = serialized;
} catch (ex) {
this._log.warn("Error populating data for day: " +
CommonUtils.exceptionStr(ex));
errors.push("Could not serialize day: " + name +
" ( " + dateFormatted + ")");
this._recordError("Error populating data for day: " + name, ex);
continue;
}
}
}
}
if (errors.length) {
o.errors = errors;
if (this._errors.length) {
o.errors = this._errors.slice(0, 20);
}
this._storage.compact();

View File

@ -32,7 +32,6 @@ this.Collector = function (storage) {
this._providerInitQueue = [];
this._providerInitializing = false;
this.providerErrors = new Map();
}
Collector.prototype = Object.freeze({
@ -124,12 +123,9 @@ Collector.prototype = Object.freeze({
constantsCollected: false,
});
this.providerErrors.set(provider.name, []);
deferred.resolve(result);
} catch (ex) {
this._log.warn("Provider failed to initialize: " + provider.name +
": " + CommonUtils.exceptionStr(ex));
this._recordProviderError(provider.name, "Failed to initialize", ex);
deferred.reject(ex);
} finally {
this._providerInitializing = false;
@ -184,15 +180,14 @@ Collector.prototype = Object.freeze({
try {
collectPromise = provider[fnProperty].call(provider);
} catch (ex) {
this._log.warn("Exception when calling " + provider.name + "." +
fnProperty + ": " + CommonUtils.exceptionStr(ex));
this.providerErrors.get(provider.name).push(ex);
this._recordProviderError(provider.name, "Exception when calling " +
"collect function: " + fnProperty, ex);
continue;
}
if (!collectPromise) {
this._log.warn("Provider does not return a promise from " +
fnProperty + "(): " + provider.name);
this._recordProviderError(provider.name, "Does not return a promise " +
"from " + fnProperty + "()");
continue;
}
@ -231,19 +226,32 @@ Collector.prototype = Object.freeze({
yield promise;
this._log.debug("Provider collected successfully: " + name);
} catch (ex) {
this._log.warn("Provider failed to collect: " + name + ": " +
CommonUtils.exceptionStr(ex));
try {
this.providerErrors.get(name).push(ex);
} catch (ex2) {
this._log.error("Error updating provider errors. This should " +
"never happen: " + CommonUtils.exceptionStr(ex2));
}
this._recordProviderError(name, "Failed to collect", ex);
}
}
throw new Task.Result(this);
}.bind(this));
},
/**
* Record an error that occurred operating on a provider.
*/
_recordProviderError: function (name, msg, ex) {
let msg = "Provider error: " + name + ": " + msg;
if (ex) {
msg += ": " + ex.message;
}
this._log.warn(msg);
if (this.onProviderError) {
try {
this.onProviderError(msg);
} catch (callError) {
this._log.warn("Exception when calling onProviderError callback: " +
CommonUtils.exceptionStr(callError));
}
}
},
});

View File

@ -29,7 +29,6 @@ add_task(function test_register_provider() {
do_check_eq(collector._providers.size, 1);
yield collector.registerProvider(dummy);
do_check_eq(collector._providers.size, 1);
do_check_eq(collector.providerErrors.size, 1);
do_check_eq(collector.getProvider(dummy.name), dummy);
let failed = false;
@ -52,7 +51,9 @@ add_task(function test_register_provider() {
add_task(function test_collect_constant_data() {
let storage = yield Metrics.Storage("collect_constant_data");
let errorCount = 0;
let collector = new Metrics.Collector(storage);
collector.onProviderError = function () { errorCount++; }
let provider = new DummyProvider();
yield collector.registerProvider(provider);
@ -62,23 +63,24 @@ add_task(function test_collect_constant_data() {
do_check_eq(provider.collectConstantCount, 1);
do_check_true(collector._providers.get("DummyProvider").constantsCollected);
do_check_eq(collector.providerErrors.get("DummyProvider").length, 0);
yield storage.close();
do_check_eq(errorCount, 0);
});
add_task(function test_collect_constant_throws() {
let storage = yield Metrics.Storage("collect_constant_throws");
let collector = new Metrics.Collector(storage);
let errors = [];
collector.onProviderError = function (error) { errors.push(error); };
let provider = new DummyProvider();
provider.throwDuringCollectConstantData = "Fake error during collect";
yield collector.registerProvider(provider);
yield collector.collectConstantData();
do_check_true(collector.providerErrors.has(provider.name));
let errors = collector.providerErrors.get(provider.name);
do_check_eq(errors.length, 1);
do_check_eq(errors[0].message, provider.throwDuringCollectConstantData);
do_check_true(errors[0].contains(provider.throwDuringCollectConstantData));
yield storage.close();
});
@ -86,15 +88,17 @@ add_task(function test_collect_constant_throws() {
add_task(function test_collect_constant_populate_throws() {
let storage = yield Metrics.Storage("collect_constant_populate_throws");
let collector = new Metrics.Collector(storage);
let errors = [];
collector.onProviderError = function (error) { errors.push(error); };
let provider = new DummyProvider();
provider.throwDuringConstantPopulate = "Fake error during constant populate";
yield collector.registerProvider(provider);
yield collector.collectConstantData();
let errors = collector.providerErrors.get(provider.name);
do_check_eq(errors.length, 1);
do_check_eq(errors[0].message, provider.throwDuringConstantPopulate);
do_check_true(errors[0].contains(provider.throwDuringConstantPopulate));
do_check_false(collector._providers.get(provider.name).constantsCollected);
yield storage.close();