socket.io: Using await within emit causes payload to be sent to all connected clients

@tommoor Not sure why this issue was highjacked but I’ve been dealing with an issue like this for the past few days.

The problem isn’t with async/await directly. Using async/await will work fine as long as you ensure .emit(..) or .clients(..) isn’t called elsewhere before the Promise resolves. This is because .to(..) doesn’t return a new instance, and .emit(..) and .clients(..) reset Namespace.prototype.rooms, which is set by .to(..).

In your example:

[1, 2, 3].forEach(async () => {
  console.log("sending to private room");
  io.to('my-private-room').emit('event', {
    data: await sleep(100)
  });
});

The call order is:

io.to(roomId)   // rooms: []       -> [roomId]
io.to(roomId)   // rooms: [roomId] -> [roomId]
io.to(roomId)   // rooms: [roomId] -> [roomId]
io.emit(..)     // rooms: [roomId] -> []        Promise resolved
io.emit(..)     // rooms: []       -> []        Promise resolved
io.emit(..)     // rooms: []       -> []        Promise resolved

After the first Promise resolves, io.emit(..) sends to roomId and clears rooms. So subsequents emits send to every socket in the namespace.

The simple fix for this is to move the await before io.to(..) like such:

[1, 2, 3].forEach(async () => {
  console.log("sending to private room");
  let data = await sleep(100);
  io.to('my-private-room').emit('event', {
    data
  });
});

This yields a call order of:

io.to(roomId)   // rooms: []       -> [roomId]  Promise resolved
io.emit(..)     // rooms: [roomId] -> []
io.to(roomId)   // rooms: []       -> [roomId]  Promise resolved
io.emit(..)     // rooms: [roomId] -> []
io.to(roomId)   // rooms: []       -> [roomId]  Promise resolved
io.emit(..)     // rooms: [roomId] -> []

Hope this helps

Sometime my app has issue : Cannot read property 'emit' of undefined A part of my code :

    socket.on('typing', function (data) {
     
            if (data.to) {
                let strangerSocket1 = io.sockets.sockets[data.to];
                socket.emit('typing', data);
                strangerSocket1.emit('typing', data);
            }
    });

What can I do to resolve this problem ? Thank you !

This was fixed by https://github.com/socketio/socket.io/commit/ac9e8ca6c71e00d4af45ee03f590fe56f3951186 and included in socket.io@4.0.0

Calling io.to() (or any other broadcast modifier) will now return an immutable instance:

const operator1 = io.to("room1");
const operator2 = operator1.to("room2");
const operator3 = socket.broadcast;
const operator4 = socket.to("room3").to("room4");

operator1.emit(/* ... */); // only to clients in "room1"
operator2.emit(/* ... */); // to clients in "room1" or in "room2"
operator3.emit(/* ... */); // to all clients but the sender
operator4.emit(/* ... */); // to clients in "room3" or in "room4" but the sender

Documentation: https://socket.io/docs/v3/migrating-from-3-x-to-4-0/#io-to-is-now-immutable

Hey, I see two possible problems with your code:

1. I don’t believe “strangerSocket1” is defined anywhere? Perhaps you intended to pass it as an argument?
2. Not so much an issue more like bad practice. You shouldn’t need to await emit. It uses callbacks to be asynchronous, it doesn’t return a Promise (instead it returns an EventEmitter). There’s nothing in your async function that actually needs to be awaited 😃 (since nothing there returns a promise) it can all be called synchronously (and it should) because it’ll behave the same way.

To solve this problem you’ll simply need to define/grab strangerSocket1 somewhere 😃. Hope this helped!

I’m assuming data.to is the socketId of the socket you want to emit to. If that’s the case, the correct way to emit from server would be: io.to(data.to).emit('typing', data) (so strangerSocket1 can be completely removed)

Thank you ! I think it’s a best answer ! Have a nice day 😄

I’m assuming data.to is the socketId of the socket you want to emit to. If that’s the case, the correct way to emit from server would be: io.to(data.to).emit('typing', data) (so strangerSocket1 can be completely removed)

I’m here because there is apparently a big bug but after reading I see absolutely no bug at all

What I do see is a very big misunderstanding of a fundamental and simple JavaScript evaluation order. If that evaluation order is taken into consideration the perceived bug turns out to live in the implementer code not socket.io

To be concise, we are talking about ‘method chaining’. i.e. obj.chain_func1().chain_func2() where:

• obj is io
• chain_func1 is to
• chain_func2 is emit

The example provided and discussed is io.to('my-private-room').emit('event', data), and the evaluation is as expected:

• 1st .to() will run, and it return an instance to io which then;
• io has .emit() chained and given the input of data that is expressed as a dictionary with an await function as a value - so the await function sleep(100) executed 2nd and it’s return value is stored to the dictionary key
• 3rd to execute is .emit() with the value it was given and had executed in the 2nd order

I understand many who claim there is a bug have a perceived race-condition, and having io.to immutable is required to fix that. This evaluation order is extremely obvious to me, and not a race-condition bug. There is nothing that any JavaScript library can do about evaluation order of chaining and inputs.

Maybe they could remove the ability for chaining? That would make the evaluation order of chaining a non-issue - but it seems so obvious (chaining evaluation is basic JavaScript syntax) that I am surprised this issue exists.

I really did not mean this to read like a mad burn to the people who claim there is a bug, I really genuinely intend this explanation to educate and help readers to see that there is no bug here at all.

Tip: you don’t understand chaining yet? or the evaluation order confuses you, then it is a good idea to simply avoid chaining and the so-called work around is actually not a work-around in reality, it’s just the more explicit version of using socket.io that avoids chaining

EDIT: the change in ac9e8ca and included in socket.io@4.0.0 added a very nice feature, which does not actually change the evaluation order I describe above and there is still a perceived race-condition in the provided example. i.e. sleep will still evaluate after io.to and before io.emit so I’d still not rely on the value of io passed from .to to .emit to be synchronous in sequence… that’s not how JavaScript event loop works. So now that we have io.to immutable, we are giving .emit a new io object and in practice we have a non-standard approach that looks like chaining, but the instance of io was forked from the original io instance at .to and is no longer the same instance that was chained to .to because .to now returns a fork of it

Hey, I see two possible problems with your code:

1. I don’t believe “strangerSocket1” is defined anywhere? Perhaps you intended to pass it as an argument?
2. Not so much an issue more like bad practice. You shouldn’t need to await emit. It uses callbacks to be asynchronous, it doesn’t return a Promise (instead it returns an EventEmitter). There’s nothing in your async function that actually needs to be awaited 😃 (since nothing there returns a promise) it can all be called synchronously (and it should) because it’ll behave the same way.

To solve this problem you’ll simply need to define/grab strangerSocket1 somewhere 😃. Hope this helped!

Sorry, I have some mistake when upload my code . I have been update it . And strangerSocket1 has been define be let strangerSocket1 = io.sockets.sockets[data.to]; In detail : It’s is socket of free member from array of object .

My issue Cannot read property 'emit' of undefined is sometimes happen not always. What can I do ? Thanks for your reponse .

@darrachequesne I saw your fix for this issue with ac9e8ca and just wanted to mention that, as you already pointed out yourself, it is indeed not backwards compatible. In a codebase of mine I used a fragment like this:

for (let id of ids) {
  io.to(id);
}
io.emit('some', 'message');

which will break. I can imagine that I’m not the only one doing something like this, so I think this has to go into a semver major.

The behavior is indeed quite surprising and should be fixed IMO. I’m not sure if we could fix this in a backward-compatible way though.

Until then, I added a note here: https://socket.io/docs/v3/rooms/#Usage-with-asynchronous-code

To get around this for my case, I store socket object in-memory in a map of immutablejs with the roomID as they key. And then I emit directly to the sockets.

Yep, this seems right and how we resolved it. The bug is really bad though and leaked production data to the wrong user in my circumstance so felt it worth filing as a warning to others