Wildhoney / EmberSockets

Socket.io (WebSockets) integrated with Ember.js' observer pattern.
http://ember-sockets.herokuapp.com/
MIT License
136 stars 22 forks source link

Socket.io connection issue can cause 'sockets' hash functions to get called too many times #9

Closed sarus closed 10 years ago

sarus commented 10 years ago

This is a an incomplete issue as I'm still debugging it but wanted to get some notes down in case someone is familiar with the problem.

If I get the following warning in socket.io on the server side:

warn - client not handshaken client should not reconnect 
     got new connetion

Each time the reconnection happens it seems as though ember-sockets registers the listener in the sockets hash. As a result, each emitted event actually gets called multiple times on the ember side (in the case of a function callback it is very noticeable as the function gets executed multiple times).

I'm still trying to track down what is causing the warning on the socket.io side.

sarus commented 10 years ago

Okay, so I think I have enough details on the issue:

By default socket.io will attempt to reconnect when a connection is broken. If your connection is broken and then reconnects the connect event is triggered.

Ember-sockets listens for the socket.io connect event and calls this._listen() each time the connect happens. this._listen() loops through each event name as specified in the sockets hash of the controllers and registers the respond callback to the socket.io event.

$ember.get(module, 'socket').on(eventName, respond.bind(eventName));

The problem is that socket.io will happily register the same callback multiple times for the same eventName. As a result, if your socket disconnects for any reason and then reconnects, you will end up with multiple callbacks registered for the same eventName. This results in the ember callback function provided in the sockets hash being called multiple times for each socket.io event received (or in the case of a property binding the binding is updated multiple times when it doesn't need to be)

My current fix is to check to make sure the eventName isn't already registered with socket.io like this:

if(!$ember.get(module, 'socket').$events[eventName]){
     $ember.get(module, 'socket').on(eventName, respond.bind(eventName));
}

Any thoughts?

Wildhoney commented 10 years ago

Great investigating, Sherlock! And yep, I think that's a good idea. Could you please create a pull request and I'll merge it in to master?

Wildhoney commented 10 years ago

Many thanks for this, @sarus

sarus commented 10 years ago

Just an FYI to anyone reading, this issue should be fixed by commit b6786e3911b5e5e78732258645ab7e8261e86a22 and is incorporated into the latest. Thanks!