Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro) Reporter
	Description • 6 years ago

[Affected versions]:
- 62.0a1 (2018-05-11)
- 61.0b4 build1 (20180510160705)

[Affected platforms]:
- Windows 10 x54
- macOS 10.13.3
- Ubuntu 16.04 x86

[Preconditions]
- You need access to SHIELD Control Panel ( https://normandy-admin.stage.mozaws.net/ )
- Set the 'app.normandy.dev_mode' preference to 'true' to run recipes immediately on startup.
- Set the 'app.normandy.logging.level' preference to '0' to enable more logging.
- Set the 'security.content.signature.root_hash' preference 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'.
- Set the preference value for 'app.normandy.api_url' to 'https://normandy.stage.mozaws.net/api/v1'.

[Steps to reproduce]:
1. Open the SHIELD Control Panel.
2. Start creating a new pref-experiment recipe (e.g. for a boolean type preference). 
3. For the preference name use a preference whose type is different (e.g. integer)
4. Approve and publish the recipe. 
5. Launch Firefox having the pre-requisites set and open:
   - browser console
   - about:telemetry (locate the Events tab and choose "dynamic" in top right corner dropdown)

[Expected result]:
- about:telemetry states the enroll failed status for the current experiment
- browser console shows that the recipe could not be executed

[Actual result]:
- No telemetry event is created for this particular recipe 
- starting with 2018-04-04, the "Could not execute recipe [...]" error is not displayed anymore in browser console (possible regressed by bug 1450153)

[Regression range]:
- Not a regression introduced by bug 1436113

[Additional notes]:
- See also bug 1447254

	:Gijs (he/him)
	Comment 4 • 6 years ago
mozreview-review

Comment on attachment 8979372 [details]
Bug 1460893 - Add event telemetry to preference study error cases

https://reviewboard.mozilla.org/r/245530/#review251760

	Pulsebot
	Comment 5 • 6 years ago

Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c28aa36127ad
Add event telemetry to preference study error cases r=Gijs

	Natalia Csoregi [:nataliaCs]
	Comment 6 • 6 years ago
bugherder

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

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 6 years ago

Please request Beta approval on this when you get a chance.

	Michael Cooper [:mythmon] Assignee
	Comment 8 • 6 years ago

Comment on attachment 8979372 [details]
Bug 1460893 - Add event telemetry to preference study error cases

Approval Request Comment
[Feature/Bug causing the regression]: None
[User impact if declined]: Very little. If a faulty recipe is sent out, Mozilla will receive less useful debugging information.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes, see comment 0
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is a very minor change to error handling.
[String changes made/needed]: None

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 6 years ago

Comment on attachment 8979372 [details]
Bug 1460893 - Add event telemetry to preference study error cases

Simple preference study fix. Approved for 61.0b8.

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 6 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/c081e26d4a89

	Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro) Reporter
	Comment 11 • 6 years ago

I can confirm that the fix is implemented on 62.0a1 (2018-05-25) and 61.0b8 build1 (20180524181234), across platforms (Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13.4). Now about:telemetry successfully displays the enrollFailed event when the recipe preference type doesn't match the Firefox preference type.
There is still one potential problem regarding Browser Console, as the following error is triggered: "Previous preference value is of type "128", but was given "64" (integer) PreferenceExperiments.jsm:337" (the recipe preference type is integer and the Firefox preference type is boolean in this particular case). Any thoughts about this?

	Michael Cooper [:mythmon] Assignee
	Comment 12 • 6 years ago

This code is designed to throw in the case of invalid data. I'm not sure if this is a good or bad design, but it isn't something I want to change right now. We'll likely be doing some significant rewriting of this code soon anyways.

	Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro) Reporter
	Comment 13 • 6 years ago

Thank you Michael! 
Per comment 11 and comment 12, I will mark 62 and 61 as verified fixed.