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

Duplicate socket.io events #490

Closed
kingcody opened this issue Aug 26, 2014 · 10 comments
Closed

Duplicate socket.io events #490

kingcody opened this issue Aug 26, 2014 · 10 comments
Assignees
Labels
Milestone

Comments

@kingcody
Copy link
Member

As pointed out to me by a user of this generator. When a socket.io connection is made and the socket is registered to the model's post('save') hook, there is no method to remove these listeners. As a side effect as connections continue to be made (reloads or new) the hook's event listener list grows proportionally. For him this is causing an unwanted amount of events to be fired off, and due to his modifications, redundant effects (don't know the details, doesn't affect the issue).

I've searched the documentation on mongoose and I don't believe you can remove middleware events, at least not easily?

With that "assumption" I've rewritten thing.socket.js to roughly be:

module.exports = function(socketio) {
  thing.schema.post('save', function (doc) {
    socketio.to('thing').emit('thing:save', doc);
  });
  thing.schema.post('remove', function (doc) {
    socketio.to('thing').emit('thing:remove', doc);
  });
};

Notice that I only export one function, and it receives socketio the socket.io server, not an individual socket. This is because I only bind to the pre save hook once at server startup, like so:

server/config/socketio.js

module.exports = function (socketio) {
  socketio.on('connection', function (socket) {
    socket.address = socket.handshake.address !== null ?
            socket.handshake.address.address + ':' + socket.handshake.address.port :
            process.env.DOMAIN;

    socket.connectedAt = new Date();

    // Call onDisconnect.
    socket.on('disconnect', function () {
      onDisconnect(socket);
    });

    // Call onConnect.
    onConnect(socket);
  });

  // Insert sockets below
  require('../api/thing/thing.socket')(socketio);
};

I use socket.io rooms to handle the different clients/listeners. Since rooms are managed by the socket.io server I added listeners for join and leave events from the client sockets:

// When the user connects.. perform this
function onConnect(socket) {
 ...
  socket.on('join', function(room) {
    socket.join(room);
  });

  socket.on('leave', function(room) {
    socket.leave(room);
  });
}

The client can then sync updates as usual and only needs to call socket.emit('join', modelName) when syncUpdates and call socket.emit('leave', modelName) when unsyncUpdates (in addition to the normal actions). This should have the added benefit of not sending events to clients that they are not subscribed to; instead of the client simply ignoring the events. Also, the server does not have a continually growing list of event listeners on a model's schema; which would most likely appear as a mem leak over time.

EDIT: typo

@kingcody
Copy link
Member Author

Some of that can definitely be cleaned and shortened, but the point is there.

I'd be more than willing to make an official PR with a concise implementation if this is an acceptable solution.

@thomporter
Copy link

👍

@JaKXz JaKXz added the bug label Aug 26, 2014
@kingcody
Copy link
Member Author

I'd like to also point out that in my example I do not handle socket.io reconnect events. Rooms need to be re-joined after a successful reconnect.

@kingcody
Copy link
Member Author

Here is my implementation of socket.service.js.

It handles room connections on syncUpdates, unsyncUpdates, and reconnects. Also unsyncUpdates now takes the synced array and not the event name. I do this so that we can bind and unbind specific arrays however we like without removing every handler from the event.

@kingcody
Copy link
Member Author

On the issue of socket.io rooms and reconnects. If a client(browser) looses connection and then reconnects, with the implementation above, any further updates will be received. However that doesn't account for any updates that happened while the client was disconnected. I'm wondering what would be more advantageous: a thing:resync event that the client can emit to the server to receive a fresh copy of things OR handle thing population on room join.

The second option could allow for a reduced dependency on the api/things endpoint when using socket.io. Not sure if that would be a good thing or not.

Thoughts?

@sparqueur
Copy link

Hi,
Thanks for your code, works great and make things more natural to me.
In my case, I would like to have dynamic rooms (for instance : a chat with multiple seperated chat rooms)
Currently, your implementation permits it but the emited message must precise the room id, which is a little strange as my event should not refer the 'object'.
So, what about being able to seperate the model from the room :

socket.syncUpdates('room'+$scope.roomId, $scope.messages);
...
function onSave(socket, doc, cb) {
      socket.to('room-'+doc.roomId).emit('room-'+doc.roomId+':save', doc);
}

VS

socket.syncUpdates('room'+$scope.roomId, 'message', $scope.messages);
...
function onSave(socket, doc, cb) {
      socket.to('room-'+doc.roomid).emit('message:save', doc);
}

I tried it and it works nicely. But maybe I'm completly wrong or it's nonsense. Just tell me.

Thank's in advance

Note: sorry for my bad english

@edgahan
Copy link

edgahan commented Oct 15, 2014

Thanks for this, helped me out!

@selipso
Copy link

selipso commented Oct 22, 2014

Using this implementation, how would one go about storing information specific to the individual socket to the database?

I want to create a chat where one person speaks with the admin. Having randomly generated rooms made this easier because I could store the room name to the database and exclude others from accessing that room except the admin and the person. How can this be accomplished?

@borzecki
Copy link

borzecki commented Dec 6, 2014

🍺 goes to you @kingcody

@vlaurenlee
Copy link

Thanks @kingcody, I was trying to work through this and finally found this answer after 2 hours of playing around with the mongoose hooks.

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

No branches or pull requests

9 participants