PeerConnection instances throw errors without any details

VERIFIED FIXED in mozilla22

Status

()

Core
WebRTC
P2
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: jsmith, Assigned: jib)

Tracking

Trunk
mozilla22
Points:
---

Firefox Tracking Flags

(firefox21 affected)

Details

(Whiteboard: [WebRTC][blocking-webrtc+])

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Steps:

1. Load https://bug829856.bugzilla.mozilla.org/attachment.cgi?id=701383
2. Click crash

Expected:

If we're getting an error in the error console, I'd expect we would see what the error is.

Actual:

We get an error, but there's no output. Are we throwing an exception without any error info here?
The fact that errors from PeerConnection.js show as blank in Web Console is a long-standing bug I noticed as well (the easiest way to reproduce is to disable peerconnection in about:config and create mozPeerConnection). I wrapped your call in try{ ...} catch(e) { console.log(e);} to see the error.

I suspect the problem strikes peerconnection.js because it is an xpcom js module or because it uses old XP IDL, or both, or we're throwing the wrong Error class.

We could try lobbing this bug to the team doing Web console and see if it comes back with instructions. :-)
After all, this may be a bug in web console.
(Reporter)

Comment 3

5 years ago
Let's send this bug over to dev tools then and see what those guys think.
Component: WebRTC → Developer Tools: Console
Product: Core → Firefox
QA Contact: jsmith
(Reporter)

Comment 4

5 years ago
Tracking nom. A big point of the building WebRTC-based apps on desktop requires that developers actually understand the errors they are hitting. A non-functional error console for WebRTC-based errors sounds quite problematic against that objective.
tracking-firefox21: --- → ?
Created attachment 706743 [details]
Simpler testcase with workaround

Here's a simpler test-case contrasted with a workaround.
Thank you for looping us in. I'm going to look into this bug next week.
(In reply to Mihai Sucan [:msucan] from comment #6)
> Thank you for looping us in. I'm going to look into this bug next week.

Tracking and assigning this to you based on your comment. Please feel free to reassign if someone else would be working on this.Thanks !
status-firefox21: --- → affected
tracking-firefox21: ? → +
QA Contact: mihai.sucan
(Reporter)

Updated

5 years ago
Assignee: nobody → mihai.sucan
QA Contact: mihai.sucan
Investigation shows that the nsIScriptErrors we receive in the Web Console are without any message. They are empty.

This is what also affects the Error Console.

There's not much we can do in the Web Console to fix this issue. I don't see why exceptions/errors thrown in PeerConnection.js lack messages. Might be a problem related to XPCOM/XP IDL stuff which is beyond me.

Taking myself off the assignment because there's not much to do for me at this point. I also suggest the bug is moved into a different component (related to webrtc?).

Please let me know if there's anything I can help with. Feel free to ping me.
Assignee: mihai.sucan → nobody
I see the same when running the WebRTC crashtests in the terminal. The exception thrown when calling addStream() on a closed PeerConnection instance doens't show any details.

I haven't seen this before with other components so as Mihai said it's not a web console problem. Moving back to WebRTC.
Component: Developer Tools: Console → WebRTC
Product: Firefox → Core
QA Contact: jsmith
Summary: The test case from bug 829856 is throwing an error in the error console with no output of what the error is → PeerConnection instances throw errors without any details
(Reporter)

Comment 10

5 years ago
(In reply to Henrik Skupin (:whimboo) from comment #9)
> I see the same when running the WebRTC crashtests in the terminal. The
> exception thrown when calling addStream() on a closed PeerConnection
> instance doens't show any details.
> 
> I haven't seen this before with other components so as Mihai said it's not a
> web console problem. Moving back to WebRTC.

This happens in multiple cases and we've specified in above comments we wanted in that group to investigate to confirm where the problem is, as that doesn't provide sufficient info. Moving back over to dev tools again.
Component: WebRTC → Developer Tools: Console
Product: Core → Firefox
QA Contact: jsmith
(In reply to Jason Smith [:jsmith] from comment #10)
> (In reply to Henrik Skupin (:whimboo) from comment #9)
> > I see the same when running the WebRTC crashtests in the terminal. The
> > exception thrown when calling addStream() on a closed PeerConnection
> > instance doens't show any details.
> > 
> > I haven't seen this before with other components so as Mihai said it's not a
> > web console problem. Moving back to WebRTC.
> 
> This happens in multiple cases and we've specified in above comments we
> wanted in that group to investigate to confirm where the problem is, as that
> doesn't provide sufficient info. Moving back over to dev tools again.

Please let me know your questions and I'll try to answer them.
(Reporter)

Updated

5 years ago
Component: Developer Tools: Console → WebRTC
Product: Firefox → Core
QA Contact: jsmith
(Reporter)

Comment 12

5 years ago
Ah, I didn't see comment 8. So this could be an IDL issue.
Whiteboard: [WebRTC][blocking-webrtc+]
Assignee: nobody → adam
There ARE details in the exception, though probably not in the expected format (the attached workaround catches the exception in JS and outputs it to the console which shows the message).

But C++ Sub-Systems throw correctly:

Notably, errors from c++ sub-systems called BY PeerConnection.js DO get through, which narrows the problem to exceptions originating in PeerConnection.js itself.

To see this, search for NS_ERROR_MALFORMED_URI in PeerConnection.js, modify it to let this exception through unadulterated and pass in an invalid stun-url: new mozRTCPeerConnection({ "iceServers": [{ "url": "@#$%^&*(^)*&stun.example.org" }] });

Not only does this error appear fine in web console, but interestingly, clicking the "Workaround" button (console.log(e)), shows you this KNOWN-GOOD format that does get through, and it is vastly different from what PeerConnection.js produces:

[12:17:44.375] [Exception... "Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService.newURI]"  nsresult: "0x804b000a (NS_ERROR_MALFORMED_URI)"  location: "JS frame :: file:///Users/Jan/moz/mozilla-central/obj-x86_64-apple-darwin12.2.1-debug/dist/NightlyDebug.app/Contents/MacOS/components/PeerConnection.js :: nicerNewURI :: line 351"  data: no]
Adam: any progress here?

Comment 15

5 years ago
Lukas: I have several higher-priority bugs in my list at the moment. I'll begin investigation on this when those are under control.
Created attachment 716729 [details]
Some more probing

Saw this go by and wanted to write down my analysis from some probing I did earlier in the week when this bugged me. Good luck.

Updated

5 years ago
Priority: -- → P2
Does this continue to block the webrtc feature? If so, this seems as if it should be P1. If the priority was set to P2 to denote being lower priority than other work in your queue Adam, perhaps we should assign elsewhere?
There really isn't an elsewhere. Everyone else on WebRTC is already working on other high priority stuff.

Comment 19

5 years ago
Alex:

I have several WebRTC blockers in my queue (seven at the moment). They can't all be more important than each other, and slamming them all up to P1 just because they're marked as blockers is just as useless as leaving them all unmarked. I've prioritized the actual crasher bugs -- like "browser fall down go boom" -- as P1. Certainly you understand that a bug that happens to produce weak diagnostics doesn't rise to that level.

If you know of someone else who can work on this immediately and do a reasonable job with it, feel free to reassign elsewhere. WebRTC has far too few engineers working on it, and I doubt we'll turn away competent workers. I'd be happy to skip over this bug and work on other blockers.
(Reporter)

Comment 20

5 years ago
Let's drop this from tracking. I don't think we need to start making use of the tracking flags just yet until we get close to release (< 10 blockers).
tracking-firefox21: + → ---
Assignee: adam → jib
Created attachment 725125 [details] [diff] [review]
PeerConnection.js throws Components.Exception so errors are readable

Simple fix in the end. Thanks dmose and bent!
Attachment #725125 - Flags: review?(rjesup)
Attachment #725125 - Flags: review?(bent.mozilla)

Updated

5 years ago
Attachment #725125 - Flags: review?(rjesup) → review+
Comment on attachment 725125 [details] [diff] [review]
PeerConnection.js throws Components.Exception so errors are readable

Review of attachment 725125 [details] [diff] [review]:
-----------------------------------------------------------------

I don't own any of this code but it looks good to me!
Attachment #725125 - Flags: review?(bent.mozilla) → feedback+
Keywords: checkin-needed
Attachment #725125 - Flags: checkin?(rjesup)
Attachment #725125 - Flags: checkin?(rjesup)
https://hg.mozilla.org/mozilla-central/rev/d08461205ce2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(Reporter)

Updated

5 years ago
Keywords: verifyme
(Reporter)

Comment 26

5 years ago
Verified on trunk with attached test case.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.