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

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

Closed
thombohlk opened this issue Nov 11, 2021 · 10 comments
Labels
bug Something isn't working
Milestone

Comments

@thombohlk
Copy link

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 thombohlk added the bug Something isn't working label Nov 11, 2021
@darrachequesne
Copy link
Member

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

I'll update the documentation to make it clearer.

@thombohlk
Copy link
Author

@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
Copy link
Author

These issues sound quite similar to what we experience, there might be a connection: #4015, #3960

darrachequesne added a commit to socketio/socket.io-website that referenced this issue Nov 13, 2021
@darrachequesne
Copy link
Member

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.

@dave-apex
Copy link

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
Copy link

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

@thombohlk
Copy link
Author

@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.

@Beretta1979
Copy link

Beretta1979 commented Oct 4, 2022

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.

@dgg
Copy link

dgg commented Feb 17, 2023

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.

darrachequesne added a commit that referenced this issue Feb 20, 2023
Namespaces that match the regex of a parent namespace will now be added
as a child of this namespace:

```js
const parentNamespace = io.of(/^\/dynamic-\d+$/);
const childNamespace = io.of("/dynamic-101");
```

Related:

- #4615
- #4164
- #4015
- #3960
@darrachequesne
Copy link
Member

@dgg this should be fixed by 0d0a7a2, 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 darrachequesne added this to the 4.6.1 milestone Feb 20, 2023
haneenmahd pushed a commit to haneenmahd/socket.io that referenced this issue Apr 15, 2023
Namespaces that match the regex of a parent namespace will now be added
as a child of this namespace:

```js
const parentNamespace = io.of(/^\/dynamic-\d+$/);
const childNamespace = io.of("/dynamic-101");
```

Related:

- socketio#4615
- socketio#4164
- socketio#4015
- socketio#3960
dzad pushed a commit to dzad/socket.io that referenced this issue May 29, 2023
Namespaces that match the regex of a parent namespace will now be added
as a child of this namespace:

```js
const parentNamespace = io.of(/^\/dynamic-\d+$/);
const childNamespace = io.of("/dynamic-101");
```

Related:

- socketio#4615
- socketio#4164
- socketio#4015
- socketio#3960
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants