зеркало из https://github.com/mozilla/gecko-dev.git
Bug 845431 - Send more errors in FHR payload; r=rnewman
This commit is contained in:
Родитель
79bcfdae5f
Коммит
65f43b0d79
|
@ -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();
|
||||
|
|
|
@ -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));
|
||||
}
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
|
|
|
@ -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();
|
||||
|
|
Загрузка…
Ссылка в новой задаче