socket.io icon indicating copy to clipboard operation
socket.io copied to clipboard

Event listeners on dynamic namespaces break when server emits on matching custom namespace before client connects

Open thombohlk opened this issue 4 years ago • 7 comments

Describe the bug

Generic description When a server emits an event on a custom namespace before any client has connected to that namespace, event listeners on a dynamic namespace that matches the custom namespace will not work. When a client connects to the custom namespace after an event has been emitted on it, the event listeners of the matching dynamic namespace will not trigger.

Our concrete use case We have a setup where we have a node cluster with multiple workers. Each worker is both used to communicate with clients using socket.io websockets and to run long running jobs on the background and emit progress about these jobs. The setup uses the Redis adapter to let socket.io communicate between workers.

We setup the "connect" event listeners on the serverside with a dynamic namespace, lets say /^\/dynamic-\d+$/. Then, when a worker starts on a job it will start emitting on namespace dynamic-7 for instance.

We have experienced that the event listeners on the dynamic namespace do not work anymore for a particular worker when that worker starts emitting on a matching custom namespace before any client connects to the worker with the same custom namespace.

So, this works:

  1. Worker starts and initiates socket.io setup and adds listener to dynamic namespace /^\/dynamic-\d+$/
  2. Clients connects to custom namespace /dynamic-7
  3. Same worker starts working on long running job and emits on custom namespace /dynamic-7
  4. Client receives updates

But this doesn't:

  1. Worker starts and initiates socket.io setup and adds listener to dynamic namespace /^\/dynamic-\d+$/
  2. Same worker starts working on long running job and emits on custom namespace /dynamic-7
  3. Clients connects to custom namespace /dynamic-7
  4. Client doesn't receive updates

To Reproduce

Be sure to start the server script before starting the client script.

Socket.IO server version: 4.3.2

Server

const socketio = require("socket.io");

const io = new socketio.Server(3000, {});

io.of(/^\/dynamic-\d+$/).on("connection", (socket) => {
  // we only get here if the bottom line is commented out, otherwise this listener is never triggered
  console.log(`connected with ${socket.id} based on dynamic namespace`);

  io.of("/dynamic-1").emit("some_event", {"foo": "bar"});
});

// this initial emit causes the bug. comment out this line to see the expected behavior
io.of("/dynamic-1").emit("some_event", {"foo": "bar"});

Socket.IO client version: 4.3.2

Client

const socketio = require("socket.io-client");

const socket = socketio.io("ws://localhost:3000/dynamic-1", {});

socket.on("connect", () => {
  console.log(`connected with id ${socket.id}`);
});

// we will only receive this event if the server doesn't emit before we connect
socket.on("some_event", (data) => {
  console.log("received event:", data);
});

Expected behavior I would expect the server to be able to start emitting events on a custom namespace that matches a dynamic namespace, even if no client has yet connected to that namespace.

Platform:

  • Device: Chrome
  • OS: Ubuntu

Additional context Note that for us the cluster setup caused the bug to arise, but it is not necessary as the reproduction example demonstrates.

Our current workaround is to execute io._checkNamespace("/dynamic-1", {}, () => {}) before we execute the first emit. This ensures that the custom namespace is added to the dynamic namespace and the event listeners of the dynamic namespace are copied to the custom namespace.

thombohlk avatar Nov 11 '21 15:11 thombohlk

Yes, this is expected, as the existing namespace has priority over the dynamic namespace.

I'll update the documentation to make it clearer.

darrachequesne avatar Nov 12 '21 06:11 darrachequesne

@darrachequesne, thank you for the quick reply!

We did not expect this behavior so I would just like to clarify. Given a socket.io setup with a dynamic namespace, is it expected that depending on whether a client first makes a connection or the server first emits on a child namespace, it changes the behavior? And if so, is the above mentioned example bad practice?

We currently have a setup where the server listens on a dynamic namespace and can start emitting on a child namespace before a client connects. This causes clients to not trigger the event listeners on the matching dynamic namespace when they do eventually connect. We are looking for a solution without using the workaround motioned above. Any suggestions would be appreciated.

thombohlk avatar Nov 12 '21 08:11 thombohlk

These issues sound quite similar to what we experience, there might be a connection: https://github.com/socketio/socket.io/issues/4015, https://github.com/socketio/socket.io/issues/3960

thombohlk avatar Nov 12 '21 10:11 thombohlk

I've added a note here; https://socket.io/docs/v4/namespaces/#dynamic-namespaces

In your example, the connection handler is only called if there is no existing namespace that matches. But since the "dynamic-1" was already registered (by calling io.of("/dynamic-1")), the connection handler is not called.

That being said, I'm open to suggestions on how we could improve this.

darrachequesne avatar Nov 13 '21 05:11 darrachequesne

We also hit this issue and lost a day trying to figure it out before I found this issue. It seems like the client or the server should be able to create a namespace in the dynamic namespace range and have it handled by the dynamic namespace handler. Can anyone think of a use case where the current behavior would be required?

dave-apex avatar Dec 01 '21 14:12 dave-apex

@thombohlk Does your workaround fix the issue in a cluster as well?

dave-apex avatar Dec 01 '21 15:12 dave-apex

@dknapp47 Yes, we are running multiple workers in our cluster. The workaround ensures that a child namespace on which a worker wants to broadcast (e.g. /dynamic-1) is first assigned to the dynamic namespace that was created for the listener (e.g. /^\/dynamic-\d+$/. So currently we are doing this check before any emit on a child namespace of a dynamic namespace.

thombohlk avatar Dec 02 '21 09:12 thombohlk

I've added a note here; https://socket.io/docs/v4/namespaces/#dynamic-namespaces

In your example, the connection handler is only called if there is no existing namespace that matches. But since the "dynamic-1" was already registered (by calling io.of("/dynamic-1")), the connection handler is not called.

That being said, I'm open to suggestions on how we could improve this.

Well I would certainly make sense to register all middlewares / event handlers that were defined on the dynamic namespace level when .of() is called with a string and it matches a dynamic namespace, not only when the first connection is done. The current flow is confusing and unexpected. Especially in the multi instance (cluster) usecase where you want to emit event to a namespace even if there were no connections in your instance yet.

We will use the workaround from @thombohlk to match that behavior for now.

Beretta1979 avatar Oct 04 '22 09:10 Beretta1979

Our team is being affected by this unexpected behavior (middlewares and events not executed when having emitted an event to a dynamic namespace for which a socket has not connected yet). It is really serious because from that moment on, the middlewares that check the jwts that the sockets carry are not verified, exposing our application to malicious actors.

We do not think that this behavior should be the default.

dgg avatar Feb 17 '23 13:02 dgg

@dgg this should be fixed by https://github.com/socketio/socket.io/commit/0d0a7a22b5ff95f864216c529114b7dd41738d1e, included in version 4.6.1.

A namespace whose name matches the regex of a parent namespace will now be added as a child of this namespace:

const parentNamespace = io.of(/^\/dynamic-\d+$/);

parentNamespace.on("connection", (socket) => {
  console.log(`connection on namespace ${socket.nsp.name}`);
});

And then somewhere else:

io.of("/dynamic-101");

Could you please check?

darrachequesne avatar Feb 20 '23 21:02 darrachequesne