1553205 - FxA metrics values sometimes don't get set

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Ed Lee :Mardak]

stomlinson has been frequently running into missing flow/device_id. After some debugging, it looks like from componentWillMount, this.props.fxaEndpoint is undefined:

https://github.com/mozilla/activity-stream/blob/3e42f671b5ba812fbd55cb07240637a6dcc104ac/content-src/asrouter/templates/Trailhead/Trailhead.jsx#L53-L65

  async componentWillMount() {
…
    if (this.props.fxaEndpoint && !this.fxaMetricsInitialized) {
…
        const url = new URL(`${this.props.fxaEndpoint}/metrics-flow?entrypoint=activity-stream-firstrun&form_type=email`);

Curiously, from render where we also use this.props.fxaEndpoint he does see the form with the correct action:

https://github.com/mozilla/activity-stream/blob/3e42f671b5ba812fbd55cb07240637a6dcc104ac/content-src/asrouter/templates/Trailhead/Trailhead.jsx#L255

          <form method="get" action={this.props.fxaEndpoint} target="_blank" rel="noopener noreferrer" onSubmit={this.onSubmit}>

React is getting rid of componentWillMount and seem to be recommending doing fetches from componentDidMount:
https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data

I and others can't seem to reproduce the issue.

Comment 1

[Ed Lee :Mardak]

Attachment: try logging

stomlinson, I have a try mac build with a potential fix and logging with the attached console output for me, but i’m guessing sometimes for you the first line for "will mount” doesn’t have the url

try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e111968ce7bdf293b38e25ea41f8b739807e1480
old https://treeherder.mozilla.org/#/jobs?repo=try&revision=88f86778e9da14a1b680afad8c7495beec384af4

direct dmg:
https://queue.taskcluster.net/v1/task/K64NfzETQFmsGX8uoZ0Mkw/runs/0/artifacts/public/build/target.dmg
old https://queue.taskcluster.net/v1/task/Mg6gilj0Tp-y5bdsueXlyA/runs/0/artifacts/public/build/target.dmg

Can you run that and see if it fixes the bug for you?

Comment 2

[Ed Lee :Mardak]

We'll hope this fixes things for shane and land a potential fix now.

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/5053

Comment 4

[Ed Lee :Mardak]

One way to reproduce this issue is to set browser.newtabpage.activity-stream.prerender to true then open about:welcome and check that the form hidden input don't have flow values.

Comment 5

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/365a93ecb6ee

Comment 6

[Ed Lee :Mardak]

https://hg.mozilla.org/releases/mozilla-release/rev/df853c362664989e025d1206b59c37c435186a91

Comment 7

[Marius Coman [:mcoman], Ecosystem QA - PTO until 8th of November, for any requests ping :cmuresan]

I have verified that this issue is no longer reproducible with the latest Firefox Nightly (69.0a1 Build ID - 20190522152821) installed, on Windows 10 x64, Arch Linux and Mac 10.14.5. Now, the hidden input forms have flow values.

Comment 9

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: Moving tracking from meta https://bugzilla.mozilla.org/show_bug.cgi?id=1552282#a669715_580382 to the 2 individual bugs that made it to 67.0.5 and 69 but not 68

Comment 10

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/5094

Comment 11

[Ed Lee :Mardak]

Attachment: Bug 1553205 - FxA metrics values sometimes don't get set

Comment 12

[Ed Lee :Mardak]

Comment on attachment 9070340 [details]
Bug 1553205 - FxA metrics values sometimes don't get set

Beta/Release Uplift Approval Request

• User impact if declined: Fxa account creations would lack attribution affecting metrics
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Comment 4
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small refactoring change to more consistently fetch metrics and already verified on both 69 and 67
• String changes made/needed: none

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9070340 [details]
Bug 1553205 - FxA metrics values sometimes don't get set

approved for 68.0b11

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1468b59c2570

Comment 15

[Alin Ilea, Desktop Test Engineering]

Verified - Fixed in latest Beta 68.0b11 on Windows 10 x64, Mac OS 10.14 and Ubuntu 18.04.

Comment 16

[Alin Ilea, Desktop Test Engineering]

Verified - Fixed in candidates 67.0.3 Build ID 20190618025334, on Windows 10 x64, Mac OS 10.14 and Ubuntu 18.04.