Clients seem to be enrolled into multi-preference experiments, but they are not
Categories
(Firefox :: Normandy Client, defect, P1)
Tracking
()
People
(Reporter: ppop, Assigned: glasserc)
Details
Attachments
(3 files)
multipref_branchType_undefined.png
[Affected Platforms]:
- All Windows
- All Mac
- All Linux
[Affected Versions]:
- Firefox Nightly 70.0a1, Build ID 20190806214332
- Firefox Beta 69.0b11, Build ID 20190805120428
[Prerequisites]:
- Have an active multi-preference-experiment recipe. (https://delivery-console.stage.mozaws.net/recipe/797/)
- Have the "security.content.signature.root_hash" pref set to "DB:74:CE:58:E4:F9:D0:9E:E0:42:36:BE:6C:C5:C4:F6:6A:E7:74:7D:C0:21:42:7A:03:BC:2F:57:0C:8B:9B:90".
- Have the "extensions.shield-recipe-client.api_url" pref set to "https://stage.normandy.nonprod.cloudops.mozgcp.net/api/v1".
- Have the "app.normandy.dev_mode" pref set to "true".
- Have the "app.normandy.logging.level" pref set to "0"
- Have the "services.settings.server" pref set to "https://settings.stage.mozaws.net/v1".
[Steps to reproduce]:
- Open the browser with the profile from prerequisites.
- Navigate to about:studies and observe the page.
[Expected result]:
- The experiment is displayed on the page as active.
[Actual result]:
- The experiment is not displayed on the page.
[Notes]:
- The following browser console logs are displayed:
1565167192865 app.normandy.preference-experiments DEBUG PreferenceExperiments.has(mythmon-multi-preference-example)
1565167192867 app.normandy.preference-experiments DEBUG PreferenceExperiments.start(mythmon-multi-preference-example, twos) - The following error is displayed after checking the "Show Content Messages" cehckbox:
Invalid value for preferenceBranchType: undefined - On the about:telemetry page, events section, the following timestamp is displayed:
4250 normandy enrollFailed preference_study mythmon-multi-preference-example {"reason": "invalid-branch"} - I have attached a screenshot of the console output:
|
Reporter | |
Comment 1•5 years ago
|
||
@Michael, @Ethan Could one of you take a look at this please?
|
||
Comment 2•5 years ago
|
||
It looks like preferenceBranchType is required for each preference, even though the schema calls it optional. Can you try to add "preferenceBranchType": "default" to each of the preference sections? It would be a sibling of preferenceValue.
|
|
||
Comment 3•5 years ago
|
||
Ciprian told me on slack that my suggestion about the schema fixed the immediate problem. That means that the real problem here is that either
- The schema is wrong (allows a null value, and says there is a default)
- or the client is not using the schema correctly (assuming a non-null value when none is guaranteed)
I think we can fix it either by updating the schema to be stricter or the client to be more permissive. I think that making the schema stricter makes sense. Ethan, do you have an opinion?
|
Assignee | |
Comment 4•5 years ago
|
||
Hmm. I copied this part of the schema from the single-preference-experiment schema, and the single-preference-experiment schema code that predated me seemed to also require preferenceBranchType -- see for example https://phabricator.services.mozilla.com/D29872. I understood this to mean that the default would be substituted when the data was run through the schema. Looking at JsonSchemaValidator now, I see that isn't true. Other uses of default seem to mostly be safe because the client checks for truthiness, which would include missing values.
I think the more fundamental bug is in the client, and we should fix it there (either by fixing JsonSchemaValidator or working around it). But because we have to support old clients too, we should probably make the schema stricter as well, by adding preferenceBranchType to the list of required fields.
|
||
Comment 5•5 years ago
|
||
The priority flag is not set for this bug.
:mythmon, could you have a look please?
For more information, please visit auto_nag documentation.
|
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 6•5 years ago
|
||
Looking at this again, it seems like the behavior I expected for default does not seem to be intended by the spec or its authors. See e.g. https://github.com/json-schema/json-schema/issues/5 -- the default is only meant for documentation and does not play a role in validation. JsonSchemaValidator already makes some decisions about how to manipulate the data that do not conform to the spec (for example, converting numbers to booleans, or supporting types such as URL or URLorEmpty). On the other hand, it doesn't seem obvious to add unconditional support for default. (Do we support it in patternProperties?) So I took the approach of updating the schema to make preferenceBranchType required, and also updating code to use default if none is provided. The first patch is necessary in any case and the second may be considered overkill so maybe we should just stick with the first patch? I'm open to suggestions on this point.
I messed up my build configuration for Gecko so at the moment I'm relying on try to test this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=959b5d28ccaf091c8bd7833c239b6441709c92d8
|
|
Assignee | |
Comment 7•5 years ago
|
||
|
|
Assignee | |
Comment 8•5 years ago
|
||
|
|
Assignee | |
Comment 9•5 years ago
|
||
|
|
Assignee | |
Comment 10•5 years ago
|
||
|
||
Comment 11•5 years ago
|
||
Pushed by eglassercamp@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e9ba0489f0c8 Insist on preferenceBranchType. r=mythmon https://hg.mozilla.org/integration/autoland/rev/28bdd68aac91 Enforce default preferenceBranchType in code. r=mythmon
|
||
Comment 12•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/e9ba0489f0c8
https://hg.mozilla.org/mozilla-central/rev/28bdd68aac91
|
|
||
Updated•5 years ago
|
|
||
Updated•5 years ago
|
|
|
Reporter | |
Comment 15•5 years ago
|
||
I have verified that this issue is no longer reproducible after adding the "preferenceBranchType": "default" argument in the json blob.
Description
•