Bug 1407082 - Handle more kinds of errors in SyncTelemetry.transformError. r?kitcambridge
MozReview-Commit-ID: CjwY7w7vUqs
--- a/services/sync/modules/telemetry.js
+++ b/services/sync/modules/telemetry.js
@@ -411,18 +411,19 @@ function cleanErrorMessage(error) {
error = error.replace(reProfileDir, "[profileDir]");
// MSG_INVALID_URL from /dom/bindings/Errors.msg -- no way to access this
// directly from JS.
if (error.endsWith("is not a valid URL.")) {
error = "<URL> is not a valid URL.";
}
// Try to filter things that look somewhat like a URL (in that they contain a
// colon in the middle of non-whitespace), in case anything else is including
- // these in error messages.
- error = error.replace(/\S+:\S+/g, "<URL>");
+ // these in error messages. Note that JSON.stringified stuff comes through
+ // here, so we explicitly ignore double-quotes as well.
+ error = error.replace(/[^\s"]+:[^\s"]+/g, "<URL>");
return error;
}
class SyncTelemetryImpl {
constructor(allowedEngines) {
log.level = Log.Level[Svc.Prefs.get("log.logger.telemetry", "Trace")];
// This is accessible so we can enable custom engines during tests.
this.allowedEngines = allowedEngines;
@@ -708,19 +709,31 @@ class SyncTelemetryImpl {
if (httpCode) {
return { name: "httperror", code: httpCode };
}
if (error.result) {
return { name: "nserror", code: error.result };
}
-
+ // It's probably an Error object, but it also could be some
+ // other object that may or may not override toString to do
+ // something useful.
+ let msg = String(error);
+ if (msg.startsWith("[object")) {
+ // Nothing useful in the default, check for a string "message" property.
+ if (typeof error.message == "string") {
+ msg = String(error.message);
+ } else {
+ // Hopefully it won't come to this...
+ msg = JSON.stringify(error);
+ }
+ }
return {
name: "unexpectederror",
- error: cleanErrorMessage(String(error))
+ error: cleanErrorMessage(msg)
};
}
}
/* global SyncTelemetry */
this.SyncTelemetry = new SyncTelemetryImpl(ENGINES);
--- a/services/sync/tests/unit/test_telemetry.js
+++ b/services/sync/tests/unit/test_telemetry.js
@@ -355,16 +355,46 @@ add_task(async function test_generic_eng
error: String(e)
});
} finally {
await cleanAndGo(engine, server);
Service.engineManager.unregister(engine);
}
});
+add_task(async function test_engine_fail_weird_errors() {
+ enableValidationPrefs();
+ await Service.engineManager.register(SteamEngine);
+ let engine = Service.engineManager.get("steam");
+ engine.enabled = true;
+ let server = await serverForFoo(engine);
+ await SyncTestingInfrastructure(server);
+ try {
+ let msg = "Bad things happened!"
+ engine._errToThrow = { message: msg };
+ let ping = await sync_and_validate_telem(true);
+ equal(ping.status.service, SYNC_FAILED_PARTIAL);
+ deepEqual(ping.engines.find(err => err.name === "steam").failureReason, {
+ name: "unexpectederror",
+ error: "Bad things happened!"
+ });
+ let e = { msg };
+ engine._errToThrow = e;
+ ping = await sync_and_validate_telem(true);
+ deepEqual(ping.engines.find(err => err.name === "steam").failureReason, {
+ name: "unexpectederror",
+ error: JSON.stringify(e)
+ });
+
+ } finally {
+ await cleanAndGo(engine, server);
+ Service.engineManager.unregister(engine);
+ }
+});
+
add_task(async function test_engine_fail_ioerror() {
enableValidationPrefs();
await Service.engineManager.register(SteamEngine);
let engine = Service.engineManager.get("steam");
engine.enabled = true;
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);