Bug 1053315 - Catch more errors during upload; r=bsmedberg

If recording FHR data during uploading raised an exception, it could
potentially abort the upload. This would appear to Mozilla as clients
that suddenly stopped using Firefox.

This patch adds explicit exception trapping around event record to
ensure this doesn't happen.

--HG--
extra : rebase_source : 7cd207b08a4f62be55093c71cb56e28832fd39d8
extra : amend_source : 9144ecea16a013370fffa5c2e833af4ec528ef5b
This commit is contained in:
Gregory Szorc 2014-08-13 11:18:22 -07:00
parent 59ae59faa6
commit ec27c4e647
3 changed files with 69 additions and 12 deletions

View File

@ -494,7 +494,7 @@ AbstractHealthReporter.prototype = Object.freeze({
}.bind(this));
},
_initializeProviderManager: function () {
_initializeProviderManager: Task.async(function* _initializeProviderManager() {
if (this._collector) {
throw new Error("Provider manager has already been initialized.");
}
@ -511,7 +511,7 @@ AbstractHealthReporter.prototype = Object.freeze({
yield this._providerManager.registerProvidersFromCategoryManager(category);
}
}
},
}),
_onProviderManagerInitialized: function () {
TelemetryStopwatch.finish(this._initHistogram, this);
@ -1363,7 +1363,12 @@ this.HealthReporter.prototype = Object.freeze({
// The built-in provider may not be initialized if this instance failed
// to initialize fully.
if (hrProvider && !isDelete) {
hrProvider.recordEvent("uploadTransportFailure", date);
try {
hrProvider.recordEvent("uploadTransportFailure", date);
} catch (ex) {
this._log.error("Error recording upload transport failure: " +
CommonUtils.exceptionStr(ex));
}
}
request.onSubmissionFailureSoft("Network transport error.");
@ -1372,7 +1377,12 @@ this.HealthReporter.prototype = Object.freeze({
if (!result.serverSuccess) {
if (hrProvider && !isDelete) {
hrProvider.recordEvent("uploadServerFailure", date);
try {
hrProvider.recordEvent("uploadServerFailure", date);
} catch (ex) {
this._log.error("Error recording server failure: " +
CommonUtils.exceptionStr(ex));
}
}
request.onSubmissionFailureHard("Server failure.");
@ -1380,7 +1390,12 @@ this.HealthReporter.prototype = Object.freeze({
}
if (hrProvider && !isDelete) {
hrProvider.recordEvent("uploadSuccess", date);
try {
hrProvider.recordEvent("uploadSuccess", date);
} catch (ex) {
this._log.error("Error recording upload success: " +
CommonUtils.exceptionStr(ex));
}
}
if (isDelete) {
@ -1445,7 +1460,12 @@ this.HealthReporter.prototype = Object.freeze({
if (hrProvider) {
let event = lastID ? "continuationUploadAttempt"
: "firstDocumentUploadAttempt";
hrProvider.recordEvent(event, now);
try {
hrProvider.recordEvent(event, now);
} catch (ex) {
this._log.error("Error when recording upload attempt: " +
CommonUtils.exceptionStr(ex));
}
}
TelemetryStopwatch.start(TELEMETRY_UPLOAD, this);
@ -1461,7 +1481,12 @@ this.HealthReporter.prototype = Object.freeze({
} catch (ex) {
TelemetryStopwatch.cancel(TELEMETRY_UPLOAD, this);
if (hrProvider) {
hrProvider.recordEvent("uploadClientFailure", now);
try {
hrProvider.recordEvent("uploadClientFailure", now);
} catch (ex) {
this._log.error("Error when recording client failure: " +
CommonUtils.exceptionStr(ex));
}
}
throw ex;
}

View File

@ -19,6 +19,7 @@ Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/Promise.jsm");
Cu.import("resource://gre/modules/FileUtils.jsm");
Cu.import("resource://gre/modules/osfile.jsm");
Cu.import("resource://gre/modules/Task.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://gre/modules/services-common/utils.js");
Cu.import("resource://gre/modules/services/datareporting/policy.jsm");
@ -148,15 +149,13 @@ InspectedHealthReporter.prototype = {
return HealthReporter.prototype._onStorageCreated.call(this, storage);
},
_initializeProviderManager: function () {
for (let result of HealthReporter.prototype._initializeProviderManager.call(this)) {
yield result;
}
_initializeProviderManager: Task.async(function* () {
yield HealthReporter.prototype._initializeProviderManager.call(this);
if (this.onInitializeProviderManagerFinished) {
this.onInitializeProviderManagerFinished();
}
},
}),
_onProviderManagerInitialized: function () {
if (this.onProviderManagerInitialized) {

View File

@ -967,6 +967,39 @@ add_task(function test_upload_on_init_failure() {
yield shutdownServer(server);
});
// Simulate a SQLite write error during upload.
add_task(function* test_upload_with_provider_record_failure() {
let [reporter, server] = yield getReporterAndServer("upload_with_provider_record_failure");
try {
// Poison the FHR provider to simulate database error or some other
// catastrophic failure. We do this because this provider is written to
// during upload and failure to catch errors would result in upload not
// occurring.
let provider = reporter.getProvider("org.mozilla.healthreport");
let wrappedProto = {
__proto__: HealthReportProvider.prototype,
recordEvent: function (event, date=new Date()) {
this._log.warn("Simulating error during write");
throw new Error("Fake error during recordEvent.");
},
};
provider.__proto__ = wrappedProto;
let deferred = Promise.defer();
let now = new Date();
let request = new DataSubmissionRequest(deferred, now);
reporter._state.addRemoteID("foo");
reporter.requestDataUpload(request);
yield deferred.promise;
do_check_eq(request.state, request.SUBMISSION_SUCCESS);
} finally {
yield reporter._shutdown();
yield shutdownServer(server);
}
});
add_task(function test_state_prefs_conversion_simple() {
let reporter = getHealthReporter("state_prefs_conversion");
let prefs = reporter._prefs;