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

Emit to room not work with uWebsocketjs #5139

Open
SaltFish001 opened this issue Jul 18, 2024 · 5 comments · May be fixed by #5158
Open

Emit to room not work with uWebsocketjs #5139

SaltFish001 opened this issue Jul 18, 2024 · 5 comments · May be fixed by #5158
Labels
bug Something isn't working needs investigation

Comments

@SaltFish001
Copy link

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

import { Server as socket_server } from 'socket.io';
import { App } from '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);
  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 8888');
});

Socket.IO client version: x.y.z

Client

import { io } from 'socket.io-client';
const client = io('http://localhost:9999', {
  path: '/socket',
  transports: ['websocket'],
});

client.onAny((...arg) => {
  console.log('onany >>>', arg);
});
client.connect();

Expected behavior
A clear and concise description of what you expected to happen.

Platform:

  • Device: [e.g. Samsung S8]
  • OS: [e.g. Android 9.2]

Additional context
Socket.io & Socket.io-client 4.7.5
uWebsocketjs 20.44.0

@SaltFish001 SaltFish001 added the to triage Waiting to be triaged by a member of the team label Jul 18, 2024
@darrachequesne
Copy link
Member

Hi! I was indeed able to reproduce the issue.

It does not seem to happen with uWebSockets.js#v20.30.0 though, this needs some additional investigation.

@darrachequesne darrachequesne added needs investigation bug Something isn't working and removed to triage Waiting to be triaged by a member of the team labels Jul 22, 2024
@SaltFish001
Copy link
Author

Hi! I was indeed able to reproduce the issue.嗨!我确实能够重现这个问题。

It does not seem to happen with uWebSockets.js#v20.30.0 though, this needs some additional investigation.然而,这种情况在 身上似乎并未出现,这需要进一步的调查。

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.

@TXWSLYF
Copy link

TXWSLYF commented Aug 7, 2024

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");
});
image

when call the scoket.join("test"), it's actually call:

public join(rooms: Room | Array<Room>): Promise<void> | void {
debug("join room %s", rooms);
return this.adapter.addAll(
this.id,
new Set(Array.isArray(rooms) ? rooms : [rooms])
);
}

when the socket_instance.attachApp is called, the Adapter.prototype.addAll would be rewrite:

Adapter.prototype.addAll = function (id, rooms) {
const isNew = !this.sids.has(id);
addAll.call(this, id, rooms);
const socket: Socket = this.nsp.sockets.get(id);
if (!socket) {
return;
}
if (socket.conn.transport.name === "websocket") {
subscribe(this.nsp.name, socket, isNew, rooms);
return;
}
if (isNew) {
socket.conn.on("upgrade", () => {
const rooms = this.sids.get(id);
if (rooms) {
subscribe(this.nsp.name, socket, isNew, rooms);
}
});
}
};

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 _doConnect method.

this.run(socket, (err) => {
process.nextTick(() => {
if ("open" !== client.conn.readyState) {
debug("next called after client was closed - ignoring socket");
socket._cleanup();
return;
}
if (err) {
debug("middleware error, sending CONNECT_ERROR packet to the client");
socket._cleanup();
if (client.conn.protocol === 3) {
return socket._error(err.data || err.message);
} else {
return socket._error({
message: err.message,
data: err.data,
});
}
}
this._doConnect(socket, fn);
});
});

The _doConnect method will only be executed when all the middleware has been executed, which track the socket.

@TXWSLYF
Copy link

TXWSLYF commented Aug 7, 2024

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");
});
image

this is because by default, the join method use the in-memory-adapter,it don't check if the socket exists now but just store the relation.

public addAll(id: SocketId, rooms: Set<Room>): Promise<void> | void {
if (!this.sids.has(id)) {
this.sids.set(id, new Set());
}
for (const room of rooms) {
this.sids.get(id).add(room);
if (!this.rooms.has(room)) {
this.rooms.set(room, new Set());
this.emit("create-room", room);
}
if (!this.rooms.get(room).has(id)) {
this.rooms.get(room).add(id);
this.emit("join-room", room, id);
}
}
}

and get the socket when emit message.

private apply(opts: BroadcastOptions, callback: (socket) => void): void {
const rooms = opts.rooms;
const except = this.computeExceptSids(opts.except);
if (rooms.size) {
const ids = new Set();
for (const room of rooms) {
if (!this.rooms.has(room)) continue;
for (const id of this.rooms.get(room)) {
if (ids.has(id) || except.has(id)) continue;
const socket = this.nsp.sockets.get(id);
if (socket) {
callback(socket);
ids.add(id);
}
}
}
} else {
for (const [id] of this.sids) {
if (except.has(id)) continue;
const socket = this.nsp.sockets.get(id);
if (socket) callback(socket);
}
}
}

@TXWSLYF TXWSLYF linked a pull request Aug 7, 2024 that will close this issue
5 tasks
@TXWSLYF
Copy link

TXWSLYF commented Aug 7, 2024

I created a fix but don't know if it will have any other impacts, would appreciate if you could check it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs investigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants