Closed Bug 1176941 Opened 5 years ago Closed 4 years ago

Javascript errors in WebRTC IdP sandbox are swallowed

Categories

(Core :: WebRTC: Signaling, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed
Blocking Flags:

People

(Reporter: mt, Assigned: mt)

Details

Attachments

(2 files)

On a debug build, they show on stdout only.  That's not good.  The resulting IdpError instance has no fields.  Need to investigate why this happens.
backlog: --- → webRTC+
Rank: 35
Priority: -- → P3
Whiteboard: [investigation]
OK, I just investigated this and I think that it is now OK (maybe something changed somewhere).  However, we lose the internal line number information from the error.  I'll upload the tests that I used to test this and we can land those.  I will investigate the process for logging JS errors to the console so that these are surfaced there as well.
Bug 1176941 - Capturing more error info from IdP sandbox, r?jib
Attachment #8654340 - Flags: review?(jib)
Comment on attachment 8654340 [details]
MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r=jib

Oops, I forgot that I had reconsidered some of this.  I don't want to leak IdP details across origins.
Attachment #8654340 - Flags: review?(jib)
Comment on attachment 8654340 [details]
MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r=jib

Bug 1176941 - Capturing more error info from IdP sandbox, r?jib
Attachment #8654340 - Flags: review?(jib)
This should be much nicer, and safer.  The error only goes to the system console, not the current window, but that's a significant improvement.  So I'll put in a second patch that does that later.  The plumbing is annoying, so I thought that I might not do it, but I think that it's worth the effort now.
Comment on attachment 8654340 [details]
MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r=jib

Bug 1176941 - Capturing more error info from IdP sandbox, r?jib
Bug 1176941 - Moving console warning to the current window, r?jib
Attachment #8654399 - Flags: review?(jib)
I've been writing some WebRTC identity code and the changes proposed here all looks like they will make it much easier to for developers to debug their WebRTC identity stuff.
Comment on attachment 8654399 [details]
MozReview Request: Bug 1176941 - Moving console warning to the current window, r=jib

https://reviewboard.mozilla.org/r/17677/#review16077

::: dom/media/IdpSandbox.jsm:250
(Diff revision 1)
> +                                 Ci.nsIScriptError.warningFlag,

Shouldn't this be errorFlag?
Attachment #8654399 - Flags: review?(jib) → review+
Comment on attachment 8654340 [details]
MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r=jib

https://reviewboard.mozilla.org/r/17661/#review16059
Attachment #8654340 - Flags: review?(jib) → review+
Comment on attachment 8654340 [details]
MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r=jib

Bug 1176941 - Capturing more error info from IdP sandbox, r?jib
Comment on attachment 8654399 [details]
MozReview Request: Bug 1176941 - Moving console warning to the current window, r=jib

Bug 1176941 - Moving console warning to the current window, r?jib
Keywords: checkin-needed
Whiteboard: [investigation]
Comment on attachment 8654340 [details]
MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r=jib

Bug 1176941 - Capturing more error info from IdP sandbox, r=jib
Attachment #8654340 - Attachment description: MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r?jib → MozReview Request: Bug 1176941 - Capturing more error info from IdP sandbox, r=jib
Comment on attachment 8654399 [details]
MozReview Request: Bug 1176941 - Moving console warning to the current window, r=jib

Bug 1176941 - Moving console warning to the current window, r=jib
Attachment #8654399 - Attachment description: MozReview Request: Bug 1176941 - Moving console warning to the current window, r?jib → MozReview Request: Bug 1176941 - Moving console warning to the current window, r=jib
Just changed the commit messages.  Should be good to go.
You need to log in before you can comment on or make changes to this bug.