sockjs / sockjs-node

WebSocket emulation - Node.js server
http://sockjs.org
MIT License
2.09k stars 309 forks source link

Close event not fired on Heroku #57

Closed eralha closed 12 years ago

eralha commented 12 years ago

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?

majek commented 12 years ago

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:

https://github.com/sockjs/sockjs-mud/commit/64853997ea115c8e72fdd609a93328b1c59f55f1

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

majek commented 12 years ago

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.

majek commented 12 years ago

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

eralha commented 12 years ago

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

eralha commented 12 years ago

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?

majek commented 12 years ago

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 12 years ago

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.

majek commented 12 years ago

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 12 years ago

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.

majek commented 12 years ago

Yup, that might work as well.

NodeGuy commented 11 years ago

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.

majek commented 11 years ago

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

NodeGuy commented 11 years ago

@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.

majek commented 11 years ago

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

NodeGuy commented 11 years ago

Thanks for the suggestion.

ghost commented 10 years ago

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

billdami commented 10 years ago

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)?

brycekahle commented 10 years ago

@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.

hems commented 9 years ago

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 https://github.com/Zarel/Pokemon-Showdown/pull/1804 ( Pokemon-Showdown!! i knew this was a serious issue! )