Closed Bug 1441366 Opened 6 years ago Closed 6 years ago

Replace internal UUIDs for extensions in browser error reports with their add-on IDs

Categories

(Firefox :: General, enhancement, P1)

Product:
Firefox
Component:
General
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 61
Tracking Flags:
Tracking Status
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: osmose, Assigned: osmose)

References

Details

Attachments

(2 files, 1 obsolete file)

Data Collection Request
2.53 KB, text/markdown
francois
: review+
Details
Bug 1441366: Document BrowserErrorCollection.jsm.
59 bytes, text/x-review-board-request
francois
: review+
Details 
Branched off from bug 1439193.

Stack traces in browser JS error reports may contain traces from add-on code. The filenames for these files currently show up with an internal UUID in them, e.g. moz-extension://9927af15-ca2c-a048-afd4-a9851b0d207a/. 

This makes it really difficult to tell which extensions are causing errors in Firefox, so we'd like to instead replace them with add-on IDs.
Attachment #8954234 - Flags: review?(francois)
rweiss: If we want to include new category 1 and 2 data (like this extension data) to browser error collection (which already got approved, is only in Nightly and behind a pref, and restricted to approved Mozilla employees), will we still need to put up data review requests? 

I'm happy to, but didn't want to inundate you with requests as we've had a bunch of little bits of data like that that we've considered adding to the collection.
Flags: needinfo?(rweiss)
Comment on attachment 8954234 [details]
Bug 1441366 Use internal add-on IDs in browser error stacktrace reports.

https://reviewboard.mozilla.org/r/223382/#review229444

Thanks!

::: browser/modules/BrowserErrorReporter.jsm:190
(Diff revision 1)
>          values: [
>            {
>              type: errorName,
>              // Error messages may contain PII; see bug 1426482 for privacy
>              // review and server-side mitigation.
> -            value: errorMessage,
> +            value: mangleExtURL(errorMessage),

`errorMessage, false`

::: browser/modules/test/browser/browser_BrowserErrorReporter.js:5
(Diff revision 1)
> +ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm", this);
>  ChromeUtils.import("resource:///modules/BrowserErrorReporter.jsm", this);
>  
> +AddonTestUtils.initMochitest(this);

Not necessary.

::: browser/modules/test/browser/browser_BrowserErrorReporter.js:424
(Diff revision 1)
> +  const id = "browsererrorcollection@example.com";
> +  const xpi = AddonTestUtils.createTempWebExtensionFile({
> +    manifest: {
> +      applications: {
> +        gecko: { id },
> +      },
> +    },
> +    background() {
> +      throw new Error("testAddonIDMangle error");
> +    },
> +  });
> +  const startupPromise = AddonTestUtils.promiseWebExtensionStartup(id);
> +  await AddonTestUtils.promiseInstallFile(xpi);
> +  Services.obs.notifyObservers(xpi, "flush-cache-entry");
> +  await startupPromise;

:

    const extension = ExtensionTestUtils.loadExtension({
      ...
    });
    await extension.startup();

::: browser/modules/test/browser/browser_BrowserErrorReporter.js:451
(Diff revision 1)
> +  const stackFrame = body.exception.values[0].stacktrace.frames[0];
> +  ok(
> +    stackFrame.filename.startsWith(`moz-extension://${id}/`),
> +    "Stack frame filenames use the proper add-on ID instead of internal UUIDs.",
> +  );
> +

`    await extension.unload();`
Attachment #8954234 - Flags: review?(kmaglione+bmo) → review+
I'm gonna just put up a data review for this now that I have more time to fill one out.
Flags: needinfo?(rweiss)

        Attachment #8958585 -
        Flags: review?(francois)

    
    

    
  
Comment on attachment 8958585 [details]
Data Collection Request

BTW, until bug 1421032 is fixed, it's easier if the request forms are attached as a .txt (i.e. that way they can be displayed inline in Bugzilla).

> 1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Is there documentation for the data that this feature collects? Has it been updated with this change?

I'd like to include the URL in my data review if possible.

        Attachment #8958585 -
        Flags: review?(francois) → review-

    
    

    
      
  


  

    
    

    
      
  

    
    

    
  
(In reply to François Marier [:francois] from comment #7)
> Is there documentation for the data that this feature collects? Has it been
> updated with this change?
> 
> I'd like to include the URL in my data review if possible.

Just added a new patch for your review that includes docs for the module, including the changes in this patch.

    
    

    
  
Comment on attachment 8958585 [details]
Data Collection Request

1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, in browser/docs/BrowserErrorReporter.rst.

2) Is there a control mechanism that allows the user to turn the data collection on and off?

Checkbox in about:preferences.

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

Not permanent. Osmose will be responsible for it.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Category 1 (3rd-party software interacting with Firefox).

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?

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

Not permanent.

        Attachment #8958585 -
        Flags: review- → review+

    
    

    
  
Comment on attachment 8958965 [details]
Bug 1441366: Document BrowserErrorCollection.jsm.

https://reviewboard.mozilla.org/r/227812/#review233676

Looks good. The only thing I would suggest is an explicit note about how to opt out (i.e. the checkbox in about:preferences).

        Attachment #8958965 -
        Flags: review?(francois) → review+

    
    

    
      
  

    
    

    
  
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 791bb219ae237ea2d7aef516e931f3af4699291b -d ad1e07a06363: rebasing 452298:791bb219ae23 "Bug 1441366 Use internal add-on IDs in browser error stacktrace reports. r=kmag"
merging browser/modules/BrowserErrorReporter.jsm
merging browser/modules/test/browser/browser_BrowserErrorReporter.js
warning: conflicts while merging browser/modules/BrowserErrorReporter.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Priority: -- → P1

    
    

    
      
  

    
  

        Attachment #8954234 -
        Attachment is obsolete: true

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/80349ef1ff5c
https://hg.mozilla.org/mozilla-central/rev/f1d85f2b9ac9
Status: NEW → RESOLVED
Closed: 6 years ago

          status-firefox61:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: