--- a/toolkit/components/normandy/lib/PreferenceExperiments.jsm
+++ b/toolkit/components/normandy/lib/PreferenceExperiments.jsm
@@ -290,69 +290,75 @@ var PreferenceExperiments = {
preferenceBranchType,
preferenceType,
experimentType = "exp",
}) {
log.debug(`PreferenceExperiments.start(${name}, ${branch})`);
const store = await ensureStorage();
if (name in store.data) {
+ TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "name-conflict"});
throw new Error(`A preference experiment named "${name}" already exists.`);
}
const activeExperiments = Object.values(store.data).filter(e => !e.expired);
const hasConflictingExperiment = activeExperiments.some(
e => e.preferenceName === preferenceName
);
if (hasConflictingExperiment) {
+ TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "pref-conflict"});
throw new Error(
`Another preference experiment for the pref "${preferenceName}" is currently active.`
);
}
const preferences = PreferenceBranchType[preferenceBranchType];
if (!preferences) {
+ TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "invalid-branch"});
throw new Error(`Invalid value for preferenceBranchType: ${preferenceBranchType}`);
}
if (experimentType.length > MAX_EXPERIMENT_SUBTYPE_LENGTH) {
+ TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "experiment-type-too-long"})
throw new Error(
`experimentType must be less than ${MAX_EXPERIMENT_SUBTYPE_LENGTH} characters. ` +
`"${experimentType}" is ${experimentType.length} long.`
);
}
+ const prevPrefType = Services.prefs.getPrefType(preferenceName);
+ const givenPrefType = PREFERENCE_TYPE_MAP[preferenceType];
+
+ if (!preferenceType || !givenPrefType) {
+ TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "invalid-type"});
+ throw new Error(`Invalid preferenceType provided (given "${preferenceType}")`);
+ }
+
+ if (prevPrefType !== Services.prefs.PREF_INVALID && prevPrefType !== givenPrefType) {
+ TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "invalid-type"});
+ throw new Error(
+ `Previous preference value is of type "${prevPrefType}", but was given ` +
+ `"${givenPrefType}" (${preferenceType})`
+ );
+ }
+
/** @type {Experiment} */
const experiment = {
name,
branch,
expired: false,
lastSeen: new Date().toJSON(),
preferenceName,
preferenceValue,
preferenceType,
previousPreferenceValue: getPref(preferences, preferenceName, preferenceType),
preferenceBranchType,
experimentType,
};
- const prevPrefType = Services.prefs.getPrefType(preferenceName);
- const givenPrefType = PREFERENCE_TYPE_MAP[preferenceType];
-
- if (!preferenceType || !givenPrefType) {
- throw new Error(`Invalid preferenceType provided (given "${preferenceType}")`);
- }
-
- if (prevPrefType !== Services.prefs.PREF_INVALID && prevPrefType !== givenPrefType) {
- throw new Error(
- `Previous preference value is of type "${prevPrefType}", but was given ` +
- `"${givenPrefType}" (${preferenceType})`
- );
- }
-
setPref(preferences, preferenceName, preferenceType, preferenceValue);
PreferenceExperiments.startObserver(name, preferenceName, preferenceType, preferenceValue);
store.data[name] = experiment;
store.saveSoon();
TelemetryEnvironment.setExperimentActive(name, branch, {type: EXPERIMENT_TYPE_PREFIX + experimentType});
TelemetryEvents.sendEvent("enroll", "preference_study", name, {experimentType, branch});
await this.saveStartupPrefs();
@@ -468,21 +474,23 @@ var PreferenceExperiments = {
async stop(experimentName, {resetValue = true, reason = "unknown"} = {}) {
log.debug(`PreferenceExperiments.stop(${experimentName}, {resetValue: ${resetValue}, reason: ${reason}})`);
if (reason === "unknown") {
log.warn(`experiment ${experimentName} ending for unknown reason`);
}
const store = await ensureStorage();
if (!(experimentName in store.data)) {
+ TelemetryEvents.sendEvent("unenrollFailed", "preference_study", experimentName, {reason: "does-not-exist"});
throw new Error(`Could not find a preference experiment named "${experimentName}"`);
}
const experiment = store.data[experimentName];
if (experiment.expired) {
+ TelemetryEvents.sendEvent("unenrollFailed", "preference_study", experimentName, {reason: "already-unenrolled"});
throw new Error(
`Cannot stop preference experiment "${experimentName}" because it is already expired`
);
}
if (PreferenceExperiments.hasObserver(experimentName)) {
PreferenceExperiments.stopObserver(experimentName);
}
--- a/toolkit/components/normandy/lib/TelemetryEvents.jsm
+++ b/toolkit/components/normandy/lib/TelemetryEvents.jsm
@@ -16,17 +16,17 @@ const TelemetryEvents = {
methods: ["enroll"],
objects: ["preference_study", "addon_study", "preference_rollout"],
extra_keys: ["experimentType", "branch", "addonId", "addonVersion"],
record_on_release: true,
},
enroll_failed: {
methods: ["enrollFailed"],
- objects: ["addon_study", "preference_rollout"],
+ objects: ["addon_study", "preference_rollout", "preference_study"],
extra_keys: ["reason", "preference"],
record_on_release: true,
},
update: {
methods: ["update"],
objects: ["preference_rollout"],
extra_keys: ["previousState"],
@@ -37,17 +37,17 @@ const TelemetryEvents = {
methods: ["unenroll"],
objects: ["preference_study", "addon_study", "preference_rollback"],
extra_keys: ["reason", "didResetValue", "addonId", "addonVersion", "branch"],
record_on_release: true,
},
unenroll_failed: {
methods: ["unenrollFailed"],
- objects: ["preference_rollback"],
+ objects: ["preference_rollback", "preference_study"],
extra_keys: ["reason"],
record_on_release: true,
},
graduate: {
methods: ["graduate"],
objects: ["preference_rollout"],
extra_keys: [],
--- a/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
+++ b/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
@@ -38,66 +38,90 @@ decorate_task(
"clearAllExperimentStorage removed all stored experiments",
);
}
);
// start should throw if an experiment with the given name already exists
decorate_task(
withMockExperiments,
- async function(experiments) {
+ withSendEventStub,
+ async function(experiments, sendEventStub) {
experiments.test = experimentFactory({name: "test"});
await Assert.rejects(
PreferenceExperiments.start({
name: "test",
branch: "branch",
preferenceName: "fake.preference",
preferenceValue: "value",
preferenceType: "string",
preferenceBranchType: "default",
}),
+ /test.*already exists/,
"start threw an error due to a conflicting experiment name",
);
+
+ Assert.deepEqual(
+ sendEventStub.args,
+ [["enrollFailed", "preference_study", "test", {reason: "name-conflict"}]],
+ "event should be sent for failure",
+ );
}
);
// start should throw if an experiment for the given preference is active
decorate_task(
withMockExperiments,
- async function(experiments) {
+ withSendEventStub,
+ async function(experiments, sendEventStub) {
experiments.test = experimentFactory({name: "test", preferenceName: "fake.preference"});
await Assert.rejects(
PreferenceExperiments.start({
name: "different",
branch: "branch",
preferenceName: "fake.preference",
preferenceValue: "value",
preferenceType: "string",
preferenceBranchType: "default",
}),
+ /another.*is currently active/i,
"start threw an error due to an active experiment for the given preference",
);
+
+ Assert.deepEqual(
+ sendEventStub.args,
+ [["enrollFailed", "preference_study", "different", {reason: "pref-conflict"}]],
+ "event should be sent for failure",
+ );
}
);
// start should throw if an invalid preferenceBranchType is given
decorate_task(
withMockExperiments,
- async function() {
+ withSendEventStub,
+ async function(experiments, sendEventStub) {
await Assert.rejects(
PreferenceExperiments.start({
name: "test",
branch: "branch",
preferenceName: "fake.preference",
preferenceValue: "value",
preferenceType: "string",
preferenceBranchType: "invalid",
}),
+ /invalid value for preferenceBranchType: invalid/i,
"start threw an error due to an invalid preference branch type",
);
+
+ Assert.deepEqual(
+ sendEventStub.args,
+ [["enrollFailed", "preference_study", "test", {reason: "invalid-branch"}]],
+ "event should be sent for failure",
+ );
}
);
// start should save experiment data, modify the preference, and register a
// watcher.
decorate_task(
withMockExperiments,
withMockPreferences,
@@ -197,41 +221,50 @@ decorate_task(
);
is(Preferences.get("fake.preference"), "newvalue", "start modified the user preference");
}
);
// start should detect if a new preference value type matches the previous value type
decorate_task(
withMockPreferences,
- async function(mockPreferences) {
+ withSendEventStub,
+ async function(mockPreferences, sendEventStub) {
mockPreferences.set("fake.type_preference", "oldvalue");
await Assert.rejects(
PreferenceExperiments.start({
name: "test",
branch: "branch",
preferenceName: "fake.type_preference",
preferenceBranchType: "user",
preferenceValue: 12345,
preferenceType: "integer",
}),
+ /previous preference value is of type/i,
"start threw error for incompatible preference type"
);
+
+ Assert.deepEqual(
+ sendEventStub.args,
+ [["enrollFailed", "preference_study", "test", {reason: "invalid-type"}]],
+ "event should be sent for failure",
+ );
}
);
// startObserver should throw if an observer for the experiment is already
// active.
decorate_task(
withMockExperiments,
async function() {
PreferenceExperiments.startObserver("test", "fake.preference", "string", "newvalue");
Assert.throws(
() => PreferenceExperiments.startObserver("test", "another.fake", "string", "othervalue"),
+ /observer.*is already active/i,
"startObserver threw due to a conflicting active observer",
);
PreferenceExperiments.stopAllObservers();
}
);
// startObserver should register an observer that calls stop when a preference
// changes from its experimental value.
@@ -282,16 +315,17 @@ decorate_task(
);
// stopObserver should throw if there is no observer active for it to stop.
decorate_task(
withMockExperiments,
async function() {
Assert.throws(
() => PreferenceExperiments.stopObserver("neveractive", "another.fake", "othervalue"),
+ /no observer.*found/i,
"stopObserver threw because there was not matching active observer",
);
}
);
// stopObserver should cancel an active observer.
decorate_task(
withMockExperiments,
@@ -355,16 +389,17 @@ decorate_task(
);
// markLastSeen should throw if it can't find a matching experiment
decorate_task(
withMockExperiments,
async function() {
await Assert.rejects(
PreferenceExperiments.markLastSeen("neveractive"),
+ /could not find/i,
"markLastSeen threw because there was not a matching experiment",
);
}
);
// markLastSeen should update the lastSeen date
decorate_task(
withMockExperiments,
@@ -378,33 +413,49 @@ decorate_task(
"markLastSeen updated the experiment lastSeen date",
);
}
);
// stop should throw if an experiment with the given name doesn't exist
decorate_task(
withMockExperiments,
- async function() {
+ withSendEventStub,
+ async function(experiments, sendEventStub) {
await Assert.rejects(
PreferenceExperiments.stop("test"),
+ /could not find/i,
"stop threw an error because there are no experiments with the given name",
);
+
+ Assert.deepEqual(
+ sendEventStub.args,
+ [["unenrollFailed", "preference_study", "test", {reason: "does-not-exist"}]],
+ "event should be sent for failure",
+ );
}
);
// stop should throw if the experiment is already expired
decorate_task(
withMockExperiments,
- async function(experiments) {
+ withSendEventStub,
+ async function(experiments, sendEventStub) {
experiments.test = experimentFactory({name: "test", expired: true});
await Assert.rejects(
PreferenceExperiments.stop("test"),
+ /already expired/,
"stop threw an error because the experiment was already expired",
);
+
+ Assert.deepEqual(
+ sendEventStub.args,
+ [["unenrollFailed", "preference_study", "test", {reason: "already-unenrolled"}]],
+ "event should be sent for failure",
+ );
}
);
// stop should mark the experiment as expired, stop its observer, and revert the
// preference value.
decorate_task(
withMockExperiments,
withMockPreferences,
@@ -555,16 +606,17 @@ decorate_task(
);
// get should throw if no experiment exists with the given name
decorate_task(
withMockExperiments,
async function() {
await Assert.rejects(
PreferenceExperiments.get("neverexisted"),
+ /could not find/i,
"get rejects if no experiment with the given name is found",
);
}
);
// get
decorate_task(
withMockExperiments,
@@ -894,16 +946,17 @@ decorate_task(
async function testSaveStartupPrefsError(experiments) {
experiments.test = experimentFactory({
preferenceName: "fake.invalidValue",
preferenceValue: new Date(),
});
await Assert.rejects(
PreferenceExperiments.saveStartupPrefs(),
+ /invalid preference type/i,
"saveStartupPrefs throws if an experiment has an invalid preference value type",
);
},
);
// test that default branch prefs restore to the right value if the default pref changes
decorate_task(
withMockExperiments,