core.peerconnection.setup unexpectedly defers setup

[trevj]

I'm writing a little app that uses peerconnection. Unexpectedly, setup() seems to perform lazily -- that is, no messages are sent over the signalling channel until I call openDataChannel().

At the very least, this is surprising behaviour and should be made clear in the documentation. However, I feel it's a bug since now I don't have a good way to know when my peerconnection has been estasblished (and this is daft, because WebRTC totally provides that).

[willscott]

This may be due to webrtc not actually doing setup until that point? My understanding is that core.peer connection is meant to be a thin wrapper around the peerconnection object exposed by the browser. On Jun 19, 2014 6:00 PM, "trevj" notifications@github.com wrote:

I'm writing a little app that uses peerconnection. Unexpectedly, setup() seems to perform lazily -- that is, no messages are sent over the signalling channel until I call openDataChannel().

At the very least, this is surprising behaviour and should be made clear in the documentation. However, I feel it's a bug since now I don't have a good way to know when my peerconnection has been estasblished (and this is daft, because WebRTC totally provides that).

— Reply to this email directly or view it on GitHub https://github.com/freedomjs/freedom/issues/66.

[trevj]

From the various sample apps I've created that use the WebRTC libraries directly, it's my understanding that description and candidate gathering starts immediately after an RTCPeerConnection is created.

Additionally, I just tried setting the last argument ("initiate immediately") to true and it seems to have no effect: https://github.com/freedomjs/freedom/blob/master/interface/core.js#L204

[willscott]

@dborkan who added that.

Why do we have this complexity 2 places? Im in favor of core.peerconnection really just being a direct exposure of the webrtc methods to freedom. On Jun 19, 2014 6:47 PM, "trevj" notifications@github.com wrote:

From the various sample apps I've created that use the WebRTC libraries directly, it's my understanding that description and candidate gathering starts immediately after an RTCPeerConnection is created.

Additionally, I just tried setting the last argument ("initiate immediately") to true and it seems to have no effect: https://github.com/freedomjs/freedom/blob/master/interface/core.js#L204

— Reply to this email directly or view it on GitHub https://github.com/freedomjs/freedom/issues/66#issuecomment-46586147.

[dborkan]

When I last tested the initiate immediately argument it worked fine for me, so I'm not sure why this isn't working anymore.

I think the setup promise fulfillment logic is correct though - we only want the promise returned by setup to be fulfilled when a webrtc connection is truly established. The problem is the webrtc connection doesn't actually get established until we send the first message over a data channel (possibly because no signaling messages are sent until we try to establish our first data channel). Before my change the setup promise fulfilled immediately, even though no webrtc connection was made, and often the webrtc connection would fail despite the setup promise being fulfilled.

Also can you clarify what you mean by "Why do we have this complexity 2 places?"?

[dborkan]

Also my understanding of the long term goal for this behavior is that going forward the core.peerconnection class should figure out on its own who should initiate the connection (possibly by both sides attempting to initiate and using a nonce to see who's connection "wins" if they both succeed). Once that is done we can probably remove the initiateConnection boolean arg, and also get rid of the 'HELLO' command we send from socks-to-rtc to rtc-to-net to force the first data channel to open

[iislucas]

A few thoughts:

1. Peer-connection is pretty far from a thin wrapper for webrtc peer connections. It does the handling of negotiation, STUN, and TURN config.
2. The peer-connection interface is pretty nice way to any multiplexed data stream that may have a configuration signaling channel. I think it's a nice high level interface.
3. I think we should do more in peer connection: I think it should be responsible for handling chunking, and then we can remove the buffer-size form the interface.
4. If we want a raw-webrtc-peer connection interface, we should make that, but it's different to the nice wrapper that we have with the current peer-connection class.
5. Setup should actually do the setup, and we should make the flag work. I think it doesn't work because we need to at least call open-data-channel (or send on a control channel).

[Paul-E]

I used setup rather than a constructor because when peerconnection was written constructors could not pass on errors. I don't know if this is still the case in v0.5.x. Peerconnection needed a setup function in order to return an error if the environment did not support WebRTC. It might be better to issue a warning on the console when freedom first sets up if none of those objects are present, and move what is currently in setup over to a freedom constructor. setup could then attempt to open a datachannel to actually test connection.

For the third point you made Lucas, I don't see the point in adding features to core.peerconnection while we still have a transport provider. The purpose of transport is to be a higher level abstraction over peerconnection, and it already has stuff like chunking. It would make more sense to move higher level functionality out of core.peerconnection and into transport. This would achieve point four in the way I think the UW crowed envisioned the two providers.

[Paul-E]

As an addition to my last message, I just saw issue #64, which I think is related to this and explains why uProxy is using peerconnection over transport.

[ryscheng]

Yea I agree with Paul. core.peerconnection should be as much of a thin-wrapper as possible.

On Thu, Jun 19, 2014 at 1:51 PM, Paul notifications@github.com wrote:

As an addition to my last message, I just saw issue #64 https://github.com/freedomjs/freedom/issues/64, which I think is related to this and explains why uProxy is using peerconnection over transport.

— Reply to this email directly or view it on GitHub https://github.com/freedomjs/freedom/issues/66#issuecomment-46614887.

[trevj]

I think I finally understand this.

I'm using peerconnection in Freedom 0.4.12.

I have a toy Freedom module which establishes a peerconnection:

var STUN_SERVERS = ['stun:stun.l.google.com:19302'];

Promise.all([freedom.core().createChannel(), freedom.core().createChannel()])
.then((values) => {
  var chanA = values[0];
  var chanB = values[1];

  // Connect the two signalling channels.
  chanA.channel.on('message', function(msg) { chanB.channel.emit('message', msg); });
  chanB.channel.on('message', function(msg) { chanA.channel.emit('message', msg); });

  var a = freedom['core.peerconnection']();
  var b = freedom['core.peerconnection']();

  a.setup(chanA.identifier, 'A', STUN_SERVERS, false).then(function() { console.log('a setup fulfilled'); }); // setup a
  b.setup(chanB.identifier, 'B', STUN_SERVERS, false).then(function() { console.log('b setup fulfilled'); }); // setup b

  a.openDataChannel('text').then(() => { console.log('datachannel open!'); });

Now, note the lines commented with // setup a and //setup b.

As printed above, everything works fine: the setup() calls fulfill only once a connection is established -- which I can achieve by creating a datachannel on either a or b.

Two things to node:

• If one side sets the final parameter of the setup() call (initiate) to true then everything works fine and don't need to call openDataChannel() in order to setup to succeed.
• The problem case is when both sides set initiate to true. In that case, both sides send an offer to each other and never send an answer to each other -- a peerconnection cannot be established and setup() never fulfills.

I think there's two things we can do:

• update the comments on the peerconnection interface (I'm happy to do this, since I seem to care so much about it)
• consider fixing the case where both sides attempt to initiate (IIRC, there are NAT configurations in which a connection can only be successfully established if one particular side initiates, e.g. https://github.com/uProxy/uProxy/issues/111)

[iislucas]

Proposal:

• short term: update the comments on the peerconnection interface
• Long term: fix it.

:)