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

Request for repeated heartbeat option #227

Closed
rc1 opened this issue Jul 29, 2014 · 8 comments
Closed

Request for repeated heartbeat option #227

rc1 opened this issue Jul 29, 2014 · 8 comments

Comments

@rc1
Copy link

rc1 commented Jul 29, 2014

Some hosts are closing web socket connections if there is no traffic over a certain period. For example, heroku.

It would help if there was an option to keep the connection alive. This fork|commit has implemented a ping/pong like service. But a heartbeat need not be bi-directional.

Here is a (not well tested) workaround:

// pass the peer instance, and it will start sending heartbeats
var heartbeater = makePeerHeartbeater( peer );

// stop them later
// heartbeater.stop();

function makePeerHeartbeater ( peer ) {
    var timeoutId = 0;
    function heartbeat () {
        timeoutId = setTimeout( heartbeat, 20000 );
        if ( peer.socket._wsOpen() ) {
            peer.socket.send( {type:'HEARTBEAT'} );
        }
    }
    // Start 
    heartbeat();
    // return
    return {
        start : function () {
            if ( timeoutId === 0 ) { heartbeat(); }
        },
        stop : function () {
            clearTimeout( timeoutId );
            timeoutId = 0;
        }
    };
}
@jure
Copy link

jure commented Jul 30, 2014

Oh so this is just keeping the connection alive because it's sending the HEARTBEAT, the sever doesn't have to do anything with it. Does this work?

For a while I had implemented a ping pong service combined with the signalling server, which would remove peers that were no longer responding to pings. However, it came with the cost of even more centralisation.

I have to say I too have been having issues with long-lived peers and WebSocket connections being dropped. Would love a complete/tested example of how to approach such a situation. Right now it's mostly trial and error for my case. How could you even test this?

@rc1
Copy link
Author

rc1 commented Jul 30, 2014

It works – at least with Heroku. The server need not do anything. As it stands the server should be parsing the message and printing "Message unrecognised" . But this would be an easy fix if a feature like this were adopted.

I'm not sure that a ping with a pong at JS level would be required to keep a connection alive. Perhaps there is by-directional communication happen at a lower level anyway; just a guess. For the server detecting closed peers, I would have thought WS's 'close' event would be fired and then the clean up happens? If not, perhaps the issue is with WS? Alternatively the server could periodically send a heartbeat back (not in sync with the other heartbeat) and catch the failed write callback.

You are right about the trickiness testing. I wish I had a better way than trail and error. In this case that's what I've done. However here is Heroku's docs on connection timeouts and this being an acceptable practise.

@RangerMauve
Copy link

Has this been merged into the master?

@rc1
Copy link
Author

rc1 commented Aug 8, 2014

There is not really anything to merge here. The code above is for copy/pasting.

@RangerMauve
Copy link

Oh, I thought the fork was going to be merged.

@RangerMauve
Copy link

As for whether or not you need a ping/pong, I can safely say that just sending data down the wire on either side is enough in my experience. (At least with Heroku). Would a PR having a simple heartbeat be welcome?

@michelle
Copy link
Member

michelle commented Aug 8, 2014

Yeah, if that's sufficient :).

Michelle

On Fri, Aug 8, 2014 at 1:48 PM, RangerMauve notifications@github.com
wrote:

As for whether or not you need a ping/pong, I can safely say that just
sending data down the wire on either side is enough in my experience. (At
least with Heroku). Would a PR having a simple heartbeat be welcome?


Reply to this email directly or view it on GitHub
#227 (comment).

@afrokick
Copy link
Member

Done #502

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

No branches or pull requests

5 participants