Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a heartbeat strategy #27

Closed
DamonOehlman opened this issue Oct 22, 2014 · 9 comments
Closed

Implement a heartbeat strategy #27

DamonOehlman opened this issue Oct 22, 2014 · 9 comments

Comments

@DamonOehlman
Copy link
Member

With #26 complete (removal of peer:leave event) we now need a heartbeat mechanism for a signalling channel. I'm not certain what it should be yet, but I believe that sending regular /announce messages makes the most sense as the signaller already has logic to handle receiving more than one announce for a peer and generating peer:update events for those.

Theoretically it should be trivial to send a regular announce an have the peer:update event indicate that a peer continues to be active on a signalling channel.

@DamonOehlman DamonOehlman added this to the 5.0.0 milestone Oct 22, 2014
@silviapfeiffer
Copy link
Member

I think we should first discuss under which circumstances an application actually needs to stay in contact with the signalling server. Or under which circumstances it needs to know that a peer has disconnected from the signalling server.

@DamonOehlman
Copy link
Member Author

Agreed. If the firefox bug can be resolved, then we really have no real need for this behaviour. Thanks for adding your comment on that issue.

@silviapfeiffer
Copy link
Member

Need to keep poking them.

BTW: have you tried using both oniceconnectionstatechange and onsignalingstatechange to detect whether the Peer connection goes down? It seemed in the bug that Adam was able to work with one of these.

@DamonOehlman
Copy link
Member Author

I vaguely recall trying something early on using signalingState but I haven't recently so I'll give it another try.

@DamonOehlman
Copy link
Member Author

OK I've checked and sadly signalingState remains stable after disconnection. I wonder whether @aullman does have any tips he can share on a way that we can give some smarts to firefox connections without relying on the signalling server connection state...

@aullman
Copy link

aullman commented Oct 23, 2014

Are you guys trying to figure out when a peerconnection gets disconnected in Firefox? I can't remember off the top of my head how/if we do that. What's the bug you guys are referring to?

I know we do rely a lot on websocket signaling for notifying participants that streams have been destroyed.

@DamonOehlman
Copy link
Member Author

Yes, that's the primary objective - to have the P2P connection state information represented through P2P connections themselves rather than relying on state of the connection to the signalling server...

Admittedly it's a nice to have, and I'm happy to implement a stop gap (hence this issue) but I would like it to sit alongside the signalling layer rather than being tightly coupled to it.

The bug is one listed on the firefox tracker that you'd previously commented on:

https://bugzilla.mozilla.org/show_bug.cgi?id=852665

@aullman
Copy link

aullman commented Oct 23, 2014

Ah, yeah, I was more talking about the initial failure which works fine. Failure during an already established connection we don't handle very well. We assume that if a PeerConnection fails then it's because someone lost their internet connection and the web socket connection will disconnect too and we'll clean up there. But that's probably not a very good assumption. I'm going to bring this up actually and see if we can make improvements there. We're pushing heavily on connectivity right now.

@DamonOehlman
Copy link
Member Author

Implementation is in the correct place at a higher level as it is implemented in rtc-quickconnect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants