1907649 - PrefFlips feature can throw during pref setting

Status: RESOLVED FIXED

(Firefox :: Nimbus Desktop Client, defect, P1)

Description

[Beth Rennie [:beth] (she/her)]

If the pref flips feature receives a configuration that would set a pref to a value of a different type, it will throw, which will cause issues with enrollment.

Comment 1

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1907649 - Unenroll if prefFlips feature cannot set prefs r?chumphreys

If the prefFlips feature receives a configuration that conflicts with the local
pref store (e.g., a pref has a default branch value of one type and the feature
configuration contains that same pref with a value of a different type), then
the call to set the pref will throw. We now catch those errors and trigger
unenrollment with reason prefFlips-failed.

Comment 2

[Beth Rennie [:beth] (she/her)]

Attachment: data-review-1907649.md

Comment 3

[Charlie Humphreys [:charlie] [:jeddai]]

Comment on attachment 9414482 [details]
data-review-1907649.md

Data Review Form

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?
   This collection is documented in its definitions files Histograms.json, Scalars.yaml, and/or Events.yaml and in the Probe Dictionary at https://probes.telemetry.mozilla.org.

2. Is there a control mechanism that allows the user to turn the data collection on and off?
   opt-out telemetry.

3. If the request is for permanent data collection, is there someone who will monitor the data over time?

• Beth Rennie ([e-mail])

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?
   Category 2 — Interaction Data

5. Is the data collection request for default-on or default-off?
   default-on

6. Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?
   No

7. Is the data collection covered by the existing Firefox privacy notice? If unsure: escalate to legal if:
   Yes

8. Does the data collection use a third-party collection tool? If yes, escalate to legal.
   No

data-review+

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c20bfcc7f5df
Unenroll if prefFlips feature cannot set prefs r=chumphreys

Comment 5

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/c20bfcc7f5df

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:beth, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox129 to wontfix.

For more information, please visit BugBot documentation.

Comment 7

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1907649 - Unenroll if prefFlips feature cannot set prefs r?chumphreys

If the prefFlips feature receives a configuration that conflicts with the local
pref store (e.g., a pref has a default branch value of one type and the feature
configuration contains that same pref with a value of a different type), then
the call to set the pref will throw. We now catch those errors and trigger
unenrollment with reason prefFlips-failed.

Original Revision: https://phabricator.services.mozilla.com/D217502

Comment 8

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: We could break the enrollment flow for other experiments/rollouts if we launch a prefFlips rollout/experiment
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: 1. Create a new prefFlips rollout that sets "extensions.logging.enabled" to 123 2. Attempt to enroll in that rollout 3. Verify that enrollment happened but unenrollment also happened (there should be an enrollment event for the recipe and an unenrollment event, with reason="prefFlips-failed", prefType="bool", prefName="extensions.logging.enabled"
• Risk associated with taking this patch: low
• Explanation of risk level: only affects prefFlips rollouts
• String changes made/needed: none
• Is Android affected?: no

Comment 9

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/22d5e592ae1d

Comment 10

[Patriciu Pop [:ppop], Ecosystem QA]

Hey Beth! Does this change also affect rollouts that set preferences to the default branch or only the user branch?

I've tried testing with the provided steps from comment #8, using a prefFlip experiment, user branch rollout, and default branch rollout. On the latest Nightly and Beta builds, the preference remains unchanged, but the default branch rollout remains active.

Comment 11

[Beth Rennie [:beth] (she/her)]

I filed bug 1910410 to track the issue :ppop mentioned.

Comment 12

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1907649 - Unenroll if prefFlips feature cannot set prefs r?chumphreys

If the prefFlips feature receives a configuration that conflicts with the local
pref store (e.g., a pref has a default branch value of one type and the feature
configuration contains that same pref with a value of a different type), then
the call to set the pref will throw. We now catch those errors and trigger
unenrollment with reason prefFlips-failed.

Original Revision: https://phabricator.services.mozilla.com/D217502

Comment 13

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: We will not be able to target ESR128 with prefFlips for incident response
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: verified by QA. Only affects prefFlips feature
• String changes made/needed: none
• Is Android affected?: no

Comment 14

[Patriciu Pop [:ppop], Ecosystem QA]

I've verified this issue is no longer reproducible together with Bug 1910410. Tested using Firefox Beta 129.0 (Build ID 20240801122119) on Windows 11 x64, Ubuntu 22.04 x64, and macOS 14.4.

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/e7ae9446ca97