903539 - When a peer connection object is created with a TURN server without creds provided, fire a more useful exception, not NS_ERROR_FAILURE or NS_ERROR_UNEXPECTED

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Jason Smith [:jsmith]]

Per the discussion on bug 902615 - creating a peer connection right now with a TURN server without creds included does fire an exception, but not a web developer friendly one. We are firing either NS_ERROR_UNEXPECTED or NS_ERROR_FAILURE. Let's throw a more friendly exception message so that the developer is aware of what goes wrong clearly.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

In general, in the few places where PeerConnection.js makes synchronous calls into c++ (which is PeerConnection construction, and that's pretty much it) we have a problem where synchronous failures from c++ don't propagate useful info up to the JS as an exception, and we get opaque error codes like NS_ERROR_FAILURE instead.

To compensate, the approach we've taken is to verify inputs in the JS, which catches most pilot errors early. 

In this case, since the JS already has the job of validating the iceServer inputs, improving that validation code to require the username and credential and throw a nice message otherwise, is trivial.

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Readable exceptions for missing username/credential on turn servers

iceServer validation code now also checks for missing username and password on turn servers, and throws nice messages (which show in the error console, not the web console, just like our other errors here. I've filed Bug 903741 for that).

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Try - https://tbpl.mozilla.org/?tree=Try&rev=ad4c6a8ad463

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fea52772b607

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fea52772b607

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Arguably, this is a regression in error message from FF22.

In FF22, if you pass in a turn uri without both username and credential (which is invalid):

   { iceServers: [{ url: "turn:turn.example.org" }] }

you get this warning in web console:

   In RTCConfiguration passed to RTCPeerConnection constructor:
   TURN servers not yet supported. Treating as STUN: "turn:turn.example.org"

Whereas in FF23 (without this patch uplifted), you get this instead:

   NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE)
  [IPeerConnection.initialize]

Too minor or too late to uplift?

Comment 7

[Jason Smith [:jsmith]]

(In reply to Jan-Ivar Bruaroey [:jib] from comment #6)
> Too minor or too late to uplift?

Probably too late for Fx23, but given that this is low risk and has a test, it probably doesn't hurt to ask for at least Aurora approval. Don't know about beta approval, although we could see what Alex & company think.

Comment 8

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Note that Aurora regresses on ALL webrtc exception messages, not just this one, so uplifting this wouldn't help without Bug 903741.