ExperimentManager.unenroll() is now effectively async when running in a
browser test (because we are now executing SQL queries during
unenrollment) so all cleanup functions that trigger unenrollment are
async as well.
Differential Revision: https://phabricator.services.mozilla.com/D250511
If ASRouter._recordReachEvent throws an error, right now we don't handle it, so we don't send exposure, and we don't route the message either. This patch makes that method catch all errors that occur inside it, much like the Glean record method.
The test we include forces an exception by replacing Glean's reach message with a stub that throws, even though it doesn't seem particularly likely that this is what's happening in the field. What's happening in the field seems to happen on the FxMS messaging IDs that are configured incorrectly AND that have some other as-yet-unknown property. There are a couple of live experiments that have (have had) issues but ONLY AFTER A CERTAIN POINT (maybe related to 138 hitting release).
The fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1965869 has repaired the configuration on the recent misconfigured feature ids, which will likely fix many/most problems. This adds another bandaid, where if there's some issue inside _recordReachEvent, we'll handle that too.
More could be done to bulletproof sendTriggerMessage, where the relevant code all lives (we could consider putting the entire thing inside a try/catch block, and ideally even send telemetry if the catch block gets hit). I'm open to thoughts about that...
Differential Revision: https://phabricator.services.mozilla.com/D250302
Instead of calling this function on the `RemoteSettingsExperimentLoader`
from `RemoteSettingsExperimentLoader.sys.mjs`, we instead provide the
public API on the `ExperimentAPI`, which will delegate to its current
`RemoteSettingsExperimentLoader`.
Differential Revision: https://phabricator.services.mozilla.com/D248073
Instead of calling this function on the `RemoteSettingsExperimentLoader`
from `RemoteSettingsExperimentLoader.sys.mjs`, we instead provide the
public API on the `ExperimentAPI`, which will delegate to its current
`RemoteSettingsExperimentLoader`.
Differential Revision: https://phabricator.services.mozilla.com/D248073
As part of this bug, the global ExperimentManager will no longer be
exported from ExperimentManager.sys.mjs and will only be available as a
property on the ExperimentAPI.
Differential Revision: https://phabricator.services.mozilla.com/D249108
All the (relevant) methods of ExperimentFakes and ExperimentTestUtils
are now defined on a new object called NimbusTestUtils. For convenience,
the old aliases still work, but they will be removed in a future bug.
There are also some new goodies, like
`NimbusTestUtils.assert.storeIsEmpty`, which is a port of a method from
our head.js files, and `NimbusTestUtils.removeStore`. This does require
you to give NimbusTestUtils access to the test scope (via
`NimbusTestUtils.init(this)`), but it lets us do actual assertions
instead of just throwing errors.
Nimbus' browser and xpcshell tests have been updated to use these new
helpers by replacing the internals of `assertEmptyStore` in each head.js
file with calls to them. They will be cleaned up to remove the wrapper
in a follow-up.
Additionally, `NimbusTestUtils.factories.recipe` will now return a
recipe that will *always* enroll. `ExperimentFakes.recipe` returns a
recipe that would only enroll 10% of the time and most tests that were
using `ExperimentFakes.recipe` were replacing that `bucketConfig` with
one that guaranteed enrollment. Existing callers do not have to change
until we migrate everything to `NimbusTestUtils`.
Differential Revision: https://phabricator.services.mozilla.com/D244960
Previously we were recording enrollment status telemetry everywhere we
were calling into `enroll` and `_unenroll` from onRecipe and
`updateEnrollment`. Now we record the enrollment status telemetry from
inside `NimbusTelemetry.recordEnrollment` and
`NimbusTelemetry.recordUnenrollment`, so that the enrollment status
telemetry is recorded from *every* enrollment event, not just during
`RemoteSettingsExperimentLoader.updateRecipes()`.
To help with this, a new helper has been added, `UnenrollmentCause`,
which carries all the metadata required to unenroll from an experiment
(similar how `CheckRecipeResult` carries all the metadata for enrollment
and updating enrollment) and to submit the corresponding telemetry. Now
instead of calling `unenroll()` with a reason string, you must call it
with an object returned from one of the `UenrollmentCause` utilities.
All tests that were calling `unenroll()` with a string like
"test" or "cleanup" have been updated to remove the reason string. When
it is not present, the `UnenrollmentCause` will default to an "unknown"
reason, which is good enough for test cleanup.
Differential Revision: https://phabricator.services.mozilla.com/D243212
This adds a new `advance_screens` pseudo-action to the `feature_callout`
messaging system template, which can be added to any action in the same
way `navigate` is used currently. This should generally be used instead
of `navigate`, which only advances the inner content but not the outer
wrapper, the anchor, the page event listeners, and other state relevant
for panel callouts.
Some callouts want to show even if the `cfr.features` pref is disabled,
such as surveys and important onboarding messages, and hard-coding that
check in FeatureCallout is heavy-handed as we already have better ways
to do it. Most callouts belong to the `cfr` group, which locks them
behind the features pref. Any that can't use groups, like the PDF
annotations callouts, can check the pref directly in their targeting.
Aside from blocking messages that don't want to be blocked by that pref,
it also results in a weird behavior where, if the pref changes while the
callout is still open (pretty unlikely), the callout will hide when you
click "next" instead of advancing to the next screen. If you clicked the
Next button, it should show you the next screen, even if you somehow
blocked the message by pref before clicking.
So it's cleaner to just let ASRouter handle that in the targeting phase.
If the pref changes while a message is showing, then we'll just leave it
open, and it won't apply until targeting is checked.
It's appropriate to make the change here, because this patch adds a new
screen advancement scheme, separate from the tour pref system. Since
we're reproducing the advancement logic, we're forced to decide whether
to reproduce the feature pref check in this new scheme. I realized 99%
of the time it's doing nothing, and in the rare cases where it does have
an effect that differs from the regular pref targeting, it's a negative
effect. And at that point, we might as well rip the bandaid off all at
once, to keep everything consistent.
Additionally, while adding documentation for this new feature, I also
corrected some existing errors in the documentation.
Differential Revision: https://phabricator.services.mozilla.com/D229084
Previously we were recording enrollment status telemetry everywhere we
were calling into `enroll` and `_unenroll` from onRecipe and
`updateEnrollment`. Now we record the enrollment status telemetry from
inside `NimbusTelemetry.recordEnrollment` and
`NimbusTelemetry.recordUnenrollment`, so that the enrollment status
telemetry is recorded from *every* enrollment event, not just during
`RemoteSettingsExperimentLoader.updateRecipes()`.
To help with this, a new helper has been added, `UnenrollmentCause`,
which carries all the metadata required to unenroll from an experiment
(similar how `CheckRecipeResult` carries all the metadata for enrollment
and updating enrollment) and to submit the corresponding telemetry. Now
instead of calling `unenroll()` with a reason string, you must call it
with an object returned from one of the `UenrollmentCause` utilities.
All tests that were calling `unenroll()` with a string like
"test" or "cleanup" have been updated to remove the reason string. When
it is not present, the `UnenrollmentCause` will default to an "unknown"
reason, which is good enough for test cleanup.
Differential Revision: https://phabricator.services.mozilla.com/D243212
Previously we were recording enrollment status telemetry everywhere we
were calling into `enroll` and `_unenroll` from onRecipe and
`updateEnrollment`. Now we record the enrollment status telemetry from
inside `NimbusTelemetry.recordEnrollment` and
`NimbusTelemetry.recordUnenrollment`, so that the enrollment status
telemetry is recorded from *every* enrollment event, not just during
`RemoteSettingsExperimentLoader.updateRecipes()`.
To help with this, a new helper has been added, `UnenrollmentCause`,
which carries all the metadata required to unenroll from an experiment
(similar how `CheckRecipeResult` carries all the metadata for enrollment
and updating enrollment) and to submit the corresponding telemetry. Now
instead of calling `unenroll()` with a reason string, you must call it
with an object returned from one of the `UenrollmentCause` utilities.
All tests that were calling `unenroll()` with a string like
"test" or "cleanup" have been updated to remove the reason string. When
it is not present, the `UnenrollmentCause` will default to an "unknown"
reason, which is good enough for test cleanup.
Differential Revision: https://phabricator.services.mozilla.com/D243212
