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

Get all clients in a room across all nodes #109

Merged
merged 6 commits into from
Sep 24, 2016

Conversation

fgodino
Copy link

@fgodino fgodino commented May 21, 2016

This pull request adds the ability to get all clients IDs connected to a room or rooms. It's up to date with Adapter so a call to Namespace#clients(fn:Function) will return the same result as RedisAdapter.clients(rooms:Array, fn:Function)

@artofspeed
Copy link

artofspeed commented May 21, 2016

can i safely use socket.rooms to get all rooms a socket has joined, across multiple nodes?

@artofspeed
Copy link

artofspeed commented May 21, 2016

Also, this is incompatible with socket.io 1.3.7, as .clients function returns undefined.

var socketio = require('socket.io');
var server = http.createServer(app);
var socketRedis = require('socket.io-redis');
var adapter = socketRedis({ host: 'localhost', port: 6380 });
var io = socketio(server);
io.adapter(adapter);

console.log(io.of('/').clients); // undefined
console.log(io.clients); // undefined

@fgodino
Copy link
Author

fgodino commented May 21, 2016

@artofspeed, socket.rooms is not implemented although it would not require a lot of work. Regarding the incompatibility with v1.3.7, you can access this function via adapter attribute.

console.log(io.of('/').adapter.clients); // [Function]

If you need a rooms functionality I can add it to this PR

@dakairus
Copy link

Is there any intentions soon by the maintainer to implement this PR? is a great feature that I guess a lot of people is wanting as me.

@naholyr
Copy link

naholyr commented May 31, 2016

+1 this is a really lacking feature in current implementation

@darrachequesne
Copy link
Member

Hi @fgodino ! Thanks for your PR. I'll try to test that ASAP.

Is anybody already using it? Any shortcomings?

@fgodino
Copy link
Author

fgodino commented Jul 22, 2016

Hey @darrachequesne, we are already using it in our website https://www.varsityviews.com/collections/ and no problems so far.

@hardeep-proptiger
Copy link

Hey @fgodino,
I have tried to implement it, Although clients is a known function, But rooms paramater is coming out to be undefined.

Any solution?

try {
var decoded = JSON.parse(msg);
} catch(err){
self.emit('error', err);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: self seems undefined here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@darrachequesne
Copy link
Member

@artofspeed a socket should only be connected to one node at a time, so socket.rooms should be safe.

Besides, I think socket.io-redis v1.0.0 is already incompatible with socket.io v1.3.7, so that should not matter much 👼

@darrachequesne
Copy link
Member

@fgodino thanks a lot!

@fgodino
Copy link
Author

fgodino commented Sep 26, 2016

@darrachequesne np!

@hems
Copy link

hems commented Sep 27, 2016

the funny thing is: if a user opens multiple tabs and go to different rooms in multiple tabs it might get a different socket.id in each of the connections and then you might not be able to know all the different socket.ids represent the same user?

Does that make sense on your application context? On my app every time a user logs in ( for instance using facebook or chrome ) i have to save his socket.id to the database, then if the user open another tab he will have two socket.id ( one in each tab ) and things start to get a bit complicated... It's manageable, but not exactly "programmer friendly".. Am i doing something wrong? It sounds like there must be an easier way.

@darrachequesne
Copy link
Member

@hems Managing users (and not sockets) is out of the scope of that library (what is a user is very much context-dependant)

Nonetheless, couldn't you create a room dedicated to each user, and make every socket join that room once authenticated?

@hems
Copy link

hems commented Sep 27, 2016

@darrachequesne yeah it makes sense. Still i have to keep a list of socket.ids a logged user has so when retrieving the list of users from a room i can find the user profile on my database and display for instance his name/avatar.

@artofspeed
Copy link

artofspeed commented Nov 1, 2016

@darrachequesne @fgodino is it possible to get all rooms a socket has joined, given the socket id only? socket.rooms only works when executed on the node instance that contains that socket object, a lot of time it's useful to fetch the rooms a socket joined given only its id.

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

Successfully merging this pull request may close these issues.

7 participants