Closed Bug 1426482 Opened 3 years ago Closed 3 years ago

Collect chrome JS errors in Nightly

Categories

(Firefox :: General, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: osmose, Assigned: osmose)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

As per bug 1426479, we'd like to prototype collecting JavaScript errors from the browser chrome, limited to the Nightly channel. This bug covers the client-side changes we need to land to collect and send this data.
Based on the docs, this seemed like the place to put the data/legal review, even though the review will have to consider the server-side component of this as well.

francois: Since the patch and service deployment depend on the results of the data review, I can't attach a single patch for review like the process page assumes; instead, the contents of this file are the filled-out questions from the review request form.

The PRD for this project is up at https://docs.google.com/document/d/1FAoRLP19hvVFniQniOC9N5jxSUpITCrUs1SiXdhrOEM/edit and has already been approved by dcamp. Let me know if I can answer any more questions.
Attachment #8938189 - Flags: review?(francois)
Comment on attachment 8938189 [details]
Data Collection Request.md

This data review is a lot more complicated than what I can do due to the wide range of data. I will let Rebecca figure out what needs to be done here since I'm just about to go on PTO for several weeks.
Attachment #8938189 - Flags: review?(francois) → review?(rweiss)
Flagging :merwin for additional feedback.
Flags: needinfo?(merwin)
Mike, 

I don't believe the trackback counts as category 3 data. We've historically treated the fact that a user has been in private browsing mode as sensitive because of the particular threat model we have in mind for private browsing. This is a policy established by the Firefox team and security engineering, so maybe Francois can comment on that specifically.  But from a data review perspective based on the definitions we've created, an indication that a user was in private browsing mode would be category 2.  That is unless there are other particularly sensitive things a trackback could tell us.  I can't see what those things might be.

Can you explain the IP address collection?  We receive IP addresses anytime the client sends us data, but we don't deliberately collection IP address in pings and associate those addresses with the other data in the ping.  Is something different happening here?  Is the IP address actually collected and retained as part of the JS error?

Marshall
Flags: needinfo?(merwin) → needinfo?(mkelly)
(In reply to Merwin from comment #4)
> Can you explain the IP address collection?  We receive IP addresses anytime
> the client sends us data, but we don't deliberately collection IP address in
> pings and associate those addresses with the other data in the ping.  Is
> something different happening here?  Is the IP address actually collected
> and retained as part of the JS error?

Sentry (the server software we want to use to collect the errors) does collect and save IP addresses with errors by default, and I didn't know if it was controllable, which is why I included it. Looking closer, their docs seem to say that we can disable IP address collection if we want[1]. I don't see much use in collecting IP addresses, so we'll make sure to disable that setting when setting up the server.

[1] https://docs.sentry.io/learn/sensitive-data/. The docs talk about their third-party hosted solution, but we'll be hosting our own instance for this prototype.
Flags: needinfo?(mkelly)
So to answer the bottom-line question, I think we can do this for nightly but given the category #4 potential with the error messages, I don't anticipate that we could collect this by default in release. So long as that expectation is clear.  If this is a test towards the objective to get this to release, it might not make sense.  If nightly is the goal, then it is doable.
(In reply to Merwin from comment #6)
> If this is a test towards the objective to get this
> to release, it might not make sense.  If nightly is the goal, then it is
> doable.

Nightly is the goal for this project. Once we're done (like second half or end of the year), if we decide this collection is useful and needs to happen on release as well, we'd look closer at either not collecting the messages or structuring the messages so that they don't have personal info in their messages. So yay!

I dunno if you or rweiss should answer this, but should we add a checkbox or hook off an existing checkbox in the "Nightly Data Collection and Use" section of the preferences? The "prompt user if we should submit" model that crashes currently use by default (I think?) wouldn't really work given how often these types of errors occur (and the data they typically contain vs what a crash has is much smaller and likely less identifying). 

Adding a new checkbox would be a bit awkward since the difference between a crash and a JavaScript error is, I'd expect, not obvious to most users. But we don't want the behavior between the two to default to the same thing.
Flags: needinfo?(rweiss)
Flags: needinfo?(merwin)
Discussion about what goes into the preferences UI is not my domain.  I think you'd have to talk to UX about that.
Flags: needinfo?(rweiss)
Gotcha, will follow up with them on Monday.
Flags: needinfo?(merwin)
Tina: We're going to add a new preference that controls whether Firefox sends a report to Mozilla when an error occurs in the Firefox UI (may not be a visible error). I think we should have something in the preferences UI to toggle this off (on by default) because the errors may contain personal information (although it's not likely). It'd be Nightly-only and would be removed after 6 months or so (although we would probably add it back afterwards if the data proves useful).

Do you have any guidance on how we should include this in the preferences UI?

My thought was that we should add a checkbox under Data Collection and Use, but there's already a checkbox there that controls whether Firefox auto-submits crash reports. I suspect users don't understand the difference between a crash report and UI error, so having two checkboxes might be confusing. However, combining them into one wouldn't work because auto-submitting crash reports is disabled by default. Also, the behavior is different (crash reports can be manually submitted when they happen, but UI errors wouldn't due to how often they occur).
Flags: needinfo?(thsieh)
Switching to mheubusch for UX guidance.
Flags: needinfo?(thsieh) → needinfo?(mheubusch)
My recommendation is an additional checkbox immediately above the crash report item that is left- aligned with the technical and interaction item and the crash report item (ie it's not indented with the studies item) 

The checkbox is checked by default and reads:  [ ] Allow Nightly to send browser error reports (including error messages) to Mozilla Learn more

the learn more link will go to a post on SUMO (draft here: https://docs.google.com/document/d/1EI6D9WnpXAzR9-EDxqiify8Iirh50okSRXeLvl6JvTY/edit?usp=sharing
Flags: needinfo?(mheubusch)
(In reply to mheubusch from comment #12) 
> 
> the learn more link will go to a post on SUMO (draft here:
> https://docs.google.com/document/d/1EI6D9WnpXAzR9-EDxqiify8Iirh50okSRXeLvl6JvTY/edit?usp=sharing

Here's the SUMO KB article that was just published:
https://support.mozilla.org/en-US/kb/firefox-nightly-error-collection
Attachment #8949232 - Attachment is obsolete: true
Comment on attachment 8949234 [details]
Fix bug 1426482: Report browser errors in Nightly to Mozilla.

:Gijs has approved the revision.

https://phabricator.services.mozilla.com/D561
Attachment #8949234 - Flags: review+
Attachment #8950306 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/e491f731de97
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Blocks: 1438103
Filed bug 1438103 on the preferences in preferences not being the preferences we use.
Depends on: 1438323
Blocks: 1439193
Depends on: 1439930
You need to log in before you can comment on or make changes to this bug.