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

Already on GitHub? Sign in to your account

Close event not fired on Heroku #57

Closed
eralha opened this Issue Apr 19, 2012 · 19 comments

Comments

Projects
None yet
7 participants

eralha commented Apr 19, 2012

Hello im experimenting with sockjs and when testing on localhost the close event fires and gives all connected users the message that a user just leave. But when i put my app on a online server it doesn´t fire that event!!

You can check it here.
http://er-u-count.herokuapp.com/

var cons = {};

var sockjs_echo = sockjs.createServer(sockjs_opts);
sockjs_echo.on('connection', function(conn) {
console.log('connection' + conn);

//SAY WELCOME
conn.write("Welcome");
//NOTIFY OTHER USERS THAT WE HAVE ARRIVED
broadcastmsg(conn.sesion+" Joined");

cons[conn.id] = conn;

conn.on('data', function(message){
    broadcastmsg(conn.sesion+" SAY: "+message);
});

conn.on('close', function() {
    console.log('close ' + conn);
    delete cons[conn.sesion];
    broadcastmsg(conn.sesion+" is leaving");
});

});

//HELPERS
function broadcastmsg(msg){
for(c in cons){
cons[c].write(msg);
}
}

Is there any way to see if a connection is still on?

Owner

majek commented Apr 20, 2012

Heroku is known to cause problems with SockJS and other long polling / comet fallbacks. Their load balancer is not closing long polling requests to your server when client goes away. Previously, I came up with this workaround:

sockjs/sockjs-mud@6485399

But that may not work in recent SockJS 0.3. Hold on, I'll try to cook a new workaround.

Owner

majek commented Apr 20, 2012

Yup, the hack still works. See: https://gist.github.com/2427902#file_server.js

// Heroku hack
service.on('connection', function(conn){
    console.log(" [.] open event received");
    var t = setInterval(function(){
        try{
            conn._session.recv.didClose();
        } catch (x) {}
    }, 15000);
    conn.on('close', function() {
        console.log(" [.] close event received");
        clearInterval(t);
    });
});

I deployed this gist as: http://cold-spring-8713.herokuapp.com/ and it's working fine. heroku logs show:

2012-04-20T11:23:54+00:00 app[web.1]:  [.] open event received
2012-04-20T11:24:17+00:00 app[web.1]:  [.] close event received

The close event may be delayed more than usual (15 + 5 seconds, instead of the usual 5) , but should eventually be delivered.

Owner

majek commented Apr 20, 2012

I'm closing this issue. Feel free to file a new bug or reopen if you encounter more problems.

@majek majek closed this Apr 20, 2012

eralha commented Apr 20, 2012

Ok i will try this and report here! Thanks for fast reply keep up the good work!

eralha commented Apr 20, 2012

It worked like a charm...

if lower the intreval between (conn._session.recv.didClose();) to 10 secs it will generate overhead on server?

didClose() <- this function send a ping back to the connection?

Owner

majek commented Apr 20, 2012

You can reduce the timer if you wish. You won't feel the overhead until you have hundreds of connections.

didClose() is a function that, in effect, closes a long-polling request.

eralha commented Apr 20, 2012

So it closes the connection but first it test the connection right?
if the connection dont respond or seems to be anactive it closes it right?

Thanks for you fast help.

Owner

majek commented Apr 20, 2012

No, in Heroku on the server side you don't know if connection was closed by the browser. You can't detect if browser went away.

When browser goes away the connection from browser to heroku will get closed, but from heroku to your server will remain open.

The solution is to periodically (I propose every 15 seconds) close the connection from the server side. If the browser is still active - it will reconnect and the session will continue.

eralha commented Apr 21, 2012

Hmm i see, im thinking of implementing a heartbeat process, instead of have X timers runing, maybe if i have one timer runing every 10 or 15 seconds and sending a ping to all connections, connections that didn´t respond in x-seconds will be disconected.

This will increase the traffic but reduce the memory consuption i think.

Owner

majek commented Apr 21, 2012

Yup, that might work as well.

NodeGuy commented Feb 6, 2013

Be careful not to use this hack on code that uses other transports as well. I couldn't figure out why my WebSocket connection kept closing and then finally remembered it's because I put this hack in code that's sometimes used on Heroku and sometimes elsewhere.

Owner

majek commented Feb 6, 2013

@ballbearing Hold on, since when websockets can be tunneled through Heroku's load balancer? Is there an official documentation about it?

NodeGuy commented Feb 6, 2013

@majek, no, that would be exciting but you misunderstood me. My code uses WebSocket only when deployed somewhere other than Heroku.

I need my code to test whether it's on Heroku before employing the hack.

Owner

majek commented Feb 6, 2013

Okay, consider disabling websockets alltogether! (websocket: false option on the server side)

NodeGuy commented Feb 7, 2013

Thanks for the suggestion.

sbnoemi commented Dec 13, 2013

For the benefit of those who encounter this ticket in the future: apparently Heroku curently has websocket support in public beta.

billdami commented Oct 5, 2014

WebSocket support on heroku left beta in July and is now generally available;

https://blog.heroku.com/archives/2014/7/7/websockets_now_ga

However, will this hack still need to be employed for browsers that don't support websockets (aka IE <10)?

Contributor

brycekahle commented Oct 6, 2014

@billdami I'm not sure on the current state of non-websocket transports on Heroku. Some testing would be needed to see if you still need the hack.

@Morfent Morfent referenced this issue in Zarel/Pokemon-Showdown Apr 20, 2015

Merged

Remove Heroku hack #1804

hems commented Jun 24, 2015

i'm not sure if "session affinity" on heroku will sort us out, but might help in some cases.

https://blog.heroku.com/archives/2015/4/28/introducing_session_affinity

I'm giving a go today, hopefully will get great results and not many surprises in the future.

related to Zarel/Pokemon-Showdown#1804 ( Pokemon-Showdown!! i knew this was a serious issue! )

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