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

Server-side namespaced disconnect #795

Closed
DanielBaulig opened this Issue Mar 18, 2012 · 8 comments

Comments

Projects
None yet
3 participants
Contributor

DanielBaulig commented Mar 18, 2012

As far as I can tell disconnecting from a namespace is only possible from the client side. It does not seem to be possible to disconnect a client from just a namespace from the server side.

Forced disconnection from the server is an essential feature. Multiplexing over a single connection is hampered a lot if a server can only disconnect a client from all namespaces at once. E.g. a module based on socket.io might disconnect a socket if something goes wrong badly. One couldn't pull in that module and use it on a multiplexed connection in it's own namespace without it interfering with other stuff going on on that connection.

The code for dealing with namespaced disconnects essentially is present. The protocol also specifies it. And it is already possible, from the client side.

What do you guys think?

Contributor

rauchg commented Mar 18, 2012

socket.disconnect should only close the transport if no other multiplexed sockets are open I think

Contributor

DanielBaulig commented Mar 18, 2012

I have the following setup:

io.of('/one').on('connection', function (socket) {
  socket.log.debug('socket connected to namespace one', socket.id);
  socket.disconnect();
});

io.of('/two').on('connection', function (socket) {
  socket.log.debug('socket connected to namespace two', socket.id);
});

Note, that I do not disconnect the namespace two namespaced socket. However, on the client both on('disconnect') events will fire.

PS: output of socket.io

info  - booting client
info  - transport end by forced client disconnection
debug - websocket writing 0::
info  - transport end
Contributor

rauchg commented Mar 18, 2012

I see. That's definitely a bug!

Contributor

DanielBaulig commented Mar 18, 2012

Actually, from looking at the source code it looks more like a "feature":

https://github.com/LearnBoost/socket.io/blob/master/lib/socket.js#L290
https://github.com/LearnBoost/socket.io/blob/master/lib/transport.js#L202

There doesn't seem to be any code to handle that use-case.

Contributor

rauchg commented Mar 18, 2012

Yep, that's definitely something we need to change =] Thanks for bringing this up

Contributor

DanielBaulig commented Mar 18, 2012

I played around a bit and it seems that doing the following will actually perform a graceful namespaced disconnect:

socket.packet({type: 'disconnect'});
socket.namespace.manager.onLeave(socket.id, socket.namespace.name);
socket.$emit('disconnect', 'booted');

It does not take into account what happens, if the namespace was the last namespace the client was connected to though. But as I think about it, I don't think anything special should happen when the client disconnects from the last namespace, since it will still be connected to the global namespace anyway.

Also I thin there is another bug, that is not directly related but I found while doing research on this bug: if a client disconnects from a namespace you would expect it to also leave all rooms associated with that namespace. This is not the case though. I will open up another issue and describe that more in details there.

@DanielBaulig DanielBaulig added a commit to DanielBaulig/socket.io that referenced this issue Mar 19, 2012

@DanielBaulig DanielBaulig Add disconnect from namespace test-case for issue #795 da95094

@DanielBaulig DanielBaulig added a commit to DanielBaulig/socket.io that referenced this issue Mar 19, 2012

@DanielBaulig DanielBaulig Fix issue #795 00694a8
Contributor

DanielBaulig commented Mar 19, 2012

The second commit fixes the problem.

@lautis lautis added a commit to lautis/socket.io that referenced this issue Apr 6, 2012

@lautis lautis Merge remote-tracking branch 'refs/remotes/learnboost/master' into fl…
…owdock

* refs/remotes/learnboost/master:
  Release 0.9.5
  Added test for polling with connection close.
  Ensure close upon request close.
  Fix disconnection reason being lost for polling transports.
  Ensure that polling transports work with Connection: close
  Log disconnection reason
  Release 0.9.4
  Release 0.9.4
  Release 0.9.4
  Release 0.9.3
  Firefox will try to parse the response from POST requests, causing a syntax error message in the Web Console. Basically an addition to socketio#501
  Fix issue #795
  Add disconnect from namespace test-case for issue #795

Conflicts:
	lib/transports/http.js
	package.json
a64bcc1

nh2 commented Jun 16, 2012

Can this be closed?

@yujiosaka yujiosaka referenced this issue in socketio/socket.io-client May 14, 2014

Closed

socket should be disconnected when using namespace #664

This issue was closed.

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