1348885 - Cu.reportError breaks the Browser Console when passed an error object from a sandbox during startup

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

[Hani Yacoub, Desktop QA]

[Prerequisites]:
You need access to Admin interface (for this Test Suite, https://normandy-admin.stage.mozaws.net/control
1. Obtain a copy of Firefox with the SHIELD recipe client system add-on installed. You can check about:support to ensure that you have it.
2. Set the extensions.shield-recipe-client.dev_mode preference to true to run recipes immediately on startup.
3. Set the extensions.shield-recipe-client.logging.level preference to 0 to enable more logging.
4. 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.
5. Set the preference value for extensions.shield-recipe-client.api_url set to https://normandy.stage.mozaws.net/api/v1

[Affected versions]: 
Aurora 54.0a2 (2017-03-20) (64-bit)
Nightly 55.0a1 (2017-03-20) (64-bit)

[Affected platforms]:
All platforms: Ubuntu 16.04 x64, Windows 10 x 64, Mac OS X 10.12

[Steps to reproduce]:
1. Open Admin Interface.
2. In the Control Interface, Click on Add New.
3. In the Name field, set a recipe name.
4. In the Filter Expression set a filter expression (e.g. 1==1).
5. From the action DropDown, select type "HeartBeat Prompt".
6. Type a survey Id in the survey ID textbox.
7. In the Message field, type in a custom message.
8. In the Engagement button label, introduce a label if your heartbeat has a button or leave empty for no button to be displayed.
9. Set "about:plugins" as a Post Answer URL.
10. Input a message into the Learn More Message field.
11. Input "about:support" into the learn More URL.
12. From the dropDown selector, select DO not show this prompt to users more than once.
13. Click On "Add new Recipe" button.
14. Restart FF.
15. Open FF with the Shield addon installed and having the pre-requisites set and open the browser console.

[Expected result]:
The Browser console will show that the recipe with the name you set at step 3 has been run and a heartbeat survey is being displayed.

[Actual result]:
No connection to any Shield/Normandy staging is shown: the heartbeat survey is not being displayed and the Browser console shows - http://pastebin.com/LJcaYv5r

[Note]:
The problem started to appear from Aurora 54.0a2 (2017-03-20)

Comment 1

[Osmose [:osmose, :mkelly]]

I've seen the error message from http://pastebin.com/LJcaYv5r intermittently over the weekend; I haven't yet been able to get consistent replication, but most of what I was doing when I saw it was re-installing the SHIELD system add-on (Link to recent build: https://queue.taskcluster.net/v1/task/Jc-kcBT6SuC69oWAcnpj9w/runs/0/artifacts/public/shield-recipe-client.xpi) and restarting a bunch.

The STR don't work for me to consistently replicate the issue. But I've definitely seen this several times, so it's real.

jimb: Based on the error log in http://pastebin.com/LJcaYv5r, should we move this bug to DevTools?

Comment 2

[Osmose [:osmose, :mkelly]]

I've tracked down how to cause the issue reliably, although I'm still not quite sure why it's happening. Apparently what's causing the issue is passing an error object from the sandbox to Cu.reportError when the add-on is being run during startup. For whatever reason, it causes the Browser Console to lose whatever connection it has to the browser such that almost nothing is logged to it after the error occurs, and it cannot be opened again if closed.

Doing this mid-session, such as when installing an add-on after opening Firefox, does not log anything and does not cause any issues. It only causes problems if the add-on does it during startup.

I've created a small XPI with the following code:

> const sandbox = new Cu.Sandbox(null, {
>   wantComponents: false,
>   wantGlobalProperties: ["URL", "URLSearchParams"],
> });
> const error = Cu.evalInSandbox(`
>   new Error("OH NO");
> `, sandbox);
> Cu.reportError(error);
> Cu.nukeSandbox(sandbox);

It consistently causes the issue if you have it installed on startup. I've only tested on the latest DevEdition, though.

For Normandy, we can fix this pretty easily by just not calling Cu.reportError, so I'm moving this bug to DevTools to track why this causes an error.

Comment 3

[Osmose [:osmose, :mkelly]]

Attachment: bug-1348885-test.xpi

Here's the XPI that replicates the issue. Install it, restart Firefox, and you should see the error linked to in the initial report in the Browser Console.

Comment 4

[Osmose [:osmose, :mkelly]]

> const sandbox = new Cu.Sandbox(null, {
>   wantComponents: false,
>   wantGlobalProperties: ["URL", "URLSearchParams"],
> });

I copied this from the SHIELD add-on, I suspect the global properties aren't required to replicate this, and maybe wantComponents too. A null principal probably matters, though.

Comment 5

[:Gijs (he/him)]

Flagging this up to :pbro for triage / fixing. :-)

Comment 6

[Patrick Brosset <:pbro>]

Thanks mkelly for insvestigating the issue providing an XPI!

This is a console crash, but only the browser console under some specific conditions, so flagging as P2.
Alex: do you mind taking a look at this one please?

Comment 7

[[github robot]]

Commits pushed to master at https://github.com/mozilla/normandy

https://github.com/mozilla/normandy/commit/48a11b083ff9feed3d3b6ec7f5272704b7b529ec
Bug 1348885: Do not call Cu.reportError on errors from the sandbox.

https://github.com/mozilla/normandy/commit/2172b155578f2cfb1bbcdb7cc6206e3590084ee1
Merge pull request #628 from Osmose/fix-reporterror

Bug 1348885: Do not call Cu.reportError on errors from the sandbox.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1348885 - Prevent throwing on dead wrapper when processing stacks of error from nuked globals.

Review commit: https://reviewboard.mozilla.org/r/123158/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123158/

Comment 9

[Alexandre Poirot [:ochameau]]

Thanks a ton for the addon. It helped me a lot figuring this out.

(sorry :jryans for this r? I imagine you are the best candidate to look at this given that :bgrins is off)

Comment 10

[Alexandre Poirot [:ochameau]]

This is such an edge case. You had to create an error and only later, pass it to the console. And in between you have to nuke the global from which you created the error. I'm not sure it is only related to Cu.reportError. I wouldn't be surprise if you get the same exception in some other codepaths.

Comment 11

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=62ba8ee9925717ee008f53b7b96bac8bad681f1f

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8850613 [details]
Bug 1348885 - Prevent throwing on dead wrapper when processing stacks of error from nuked globals.

https://reviewboard.mozilla.org/r/123158/#review125540

Looks good to me, thanks!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/957dd8b03edf
Prevent throwing on dead wrapper when processing stacks of error from nuked globals. r=jryans

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/957dd8b03edf

Comment 15

[Osmose [:osmose, :mkelly]]

Removing this from blocking bug 1342014. We landed a patch in https://github.com/mozilla/normandy/pull/628 that avoids this issue on the SHIELD end, meaning that the patch in this bug is not necessary for the SHIELD add-on to function properly.