-
Notifications
You must be signed in to change notification settings - Fork 10.1k
Emit to room not work with uWebsocketjs #5139
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
Comments
Hi! I was indeed able to reproduce the issue. It does not seem to happen with |
Thanks reply. Right now I can use fetchSockets get all socket , then foreach sockets to emit event. Hope this is not a hard work for you. |
I think I found the cause of this bug.This is because during the execution of the middleware, the socket has not yet been added to the adapter's namespace. const { Server: socket_server } = require("socket.io");
const { App } = require("uWebSockets.js");
const uapp = App();
const socket_instance = new socket_server({
path: "/socket",
});
socket_instance.attachApp(uapp);
socket_instance.use((scoket, next) => {
console.log("socket connect >>>", scoket.id);
// console.log adapter's namespace
console.log(scoket.adapter.nsp.sockets, '----');
scoket.join("test");
setTimeout(() => {
console.log("socket emit >>>", scoket.rooms);
socket_instance.to("test").emit("message", "hello world");
}, 2000);
return next();
});
uapp.listen(9999, (token) => {
console.log(token);
console.log("server is listening on port 9999");
}); ![]() when call the socket.io/packages/socket.io/lib/socket.ts Lines 554 to 561 in 582655f
when the socket.io/packages/socket.io/lib/uws.ts Lines 14 to 33 in 582655f
this bug happend here, we can't find the socket in the nsp now, so it's just return and didn't call the subscribe fun. const socket: Socket = this.nsp.sockets.get(id);
if (!socket) {
return;
} and why we can't get the socket by id from the nsp here is due to the execution order of the middleware and socket.io/packages/socket.io/lib/namespace.ts Lines 334 to 357 in 582655f
The |
for the example below, we don't use uWebsocketjs this time, even if the socket has not yet been added to the adapter's namespace, it also works well. const { Server: socket_server } = require("socket.io");
const socket_instance = new socket_server({
path: "/socket",
});
socket_instance.use((scoket, next) => {
console.log("socket connect >>>", scoket.id);
console.log(scoket.adapter.nsp.sockets, '----');
scoket.join("test");
setTimeout(() => {
console.log("socket emit >>>", scoket.rooms);
socket_instance.to("test").emit("message", "hello world");
}, 2000);
return next();
});
socket_instance.listen(9999, (token) => {
console.log(token);
console.log("server is listening on port 9999");
}); ![]() this is because by default, the join method use the socket.io/packages/socket.io-adapter/lib/in-memory-adapter.ts Lines 87 to 104 in 582655f
and get the socket when emit message. socket.io/packages/socket.io-adapter/lib/in-memory-adapter.ts Lines 333 to 358 in 582655f
|
I created a fix but don't know if it will have any other impacts, would appreciate if you could check it. |
@TXWSLYF thanks for the analysis 👍 There is indeed something wrong when calling io.use((socket, next) => {
socket.join("test");
next();
});
io.on("connection", (socket) => {
io.to("test").emit("hello"); // should work
}); That being said, I'm not sure the following should work, since the socket is not connected yet: io.use((socket, next) => {
socket.join("test");
io.to("test").emit("hello"); // should it really include the socket?
next();
}); I'm looking into this. |
I had the same issue here, but after upgrading to version 4.8.0 it seems to have been fixed |
For future readers: This should be fixed by b04fa64, included in |
Describe the bug
Emit some message to room not work with uWebsocketjs, but clients does`t got any message.
To Reproduce
Please fill the following code example:
Socket.IO server version:
x.y.z
Server
Socket.IO client version:
x.y.z
Client
Expected behavior
A clear and concise description of what you expected to happen.
Platform:
Additional context
Socket.io & Socket.io-client 4.7.5
uWebsocketjs 20.44.0
The text was updated successfully, but these errors were encountered: