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

Events before disconnection dropped #5213

Open
asideofcode-dev opened this issue Oct 20, 2024 · 6 comments
Open

Events before disconnection dropped #5213

asideofcode-dev opened this issue Oct 20, 2024 · 6 comments

Comments

@asideofcode-dev
Copy link

asideofcode-dev commented Oct 20, 2024

Describe the bug
In a Socket.IO application, events emitted just before a client disconnects are not properly handled by specific on(...) listeners. While the onAny(...) listener detects these events, the specific on(...) listeners do not. This issue occurs with any event emitted just before disconnection. The reason onAny(...) works is that it runs before the dispatch, which relies on process.nextTick.

To Reproduce

Socket.IO server version: 4.x.x (replace with your version)

Server

import { Server } from "socket.io";

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

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

  // Example event listener (applies to any event)
  socket.on("exampleEvent", () => {
    console.log(`exampleEvent received before disconnect ${socket.id}`);
  });

  // Using onAny listener
  socket.onAny((event, ...args) => {
    console.log(`onAny received ${event} before disconnect ${socket.id}`);
  });

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

Socket.IO client version: 4.x.x (replace with your version)

Client

import { io } from "socket.io-client";

const socket = io("ws://localhost:3000/", {});

socket.on("connect", () => {
  console.log(`connect ${socket.id}`);
  
  // Emit any event just before disconnecting
  socket.emit("exampleEvent");
  socket.disconnect();
});

socket.on("disconnect", () => {
  console.log("disconnect");
});

Expected behavior
The on listener for any event (e.g., exampleEvent) should detect and handle the event before disconnection. Currently, the onAny listener detects the event, but the specific on(...) listener does not. This occurs because onAny runs before the dispatch, which relies on process.nextTick, allowing it to detect the event before disconnection, see

private onevent(packet: Packet): void {
const args = packet.data || [];
debug("emitting event %j", args);
if (null != packet.id) {
debug("attaching ack callback to event");
args.push(this.ack(packet.id));
}
if (this._anyListeners && this._anyListeners.length) {
const listeners = this._anyListeners.slice();
for (const listener of listeners) {
listener.apply(this, args);
}
}
this.dispatch(args);
}
.

Platform:

Not relevant

Additional context
This issue affects any event emitted just before disconnection. The onAny listener works because it operates before the event dispatch process, which uses process.nextTick. As a result, events are detected by onAny but missed by the specific on(...) listeners, which rely on the dispatch mechanism that happens after disconnection begins.

@asideofcode-dev asideofcode-dev added the to triage Waiting to be triaged by a member of the team label Oct 20, 2024
@darrachequesne
Copy link
Member

Hi!

Unfortunately, I wasn't able to reproduce the issue: https://github.com/socketio/socket.io-fiddle/tree/issues/socket.io/5213

Either with [email protected] or [email protected].

The socket._onpacket() method is also within a process.nextTick() here, which should prevent the issue. Isn't that the case for you?

@darrachequesne darrachequesne added needs investigation and removed to triage Waiting to be triaged by a member of the team labels Oct 23, 2024
@asideofcode-dev
Copy link
Author

asideofcode-dev commented Oct 23, 2024

@darrachequesne Thanks for setting up the fiddle. I just cloned it and visited the index.html page. The logs on the server are as follows:

server listening at http://localhost:1234
connect Pvc2MyvdRD_Z4K0cAAAB
onAny event
disconnect Pvc2MyvdRD_Z4K0cAAAB due to client namespace disconnect

The onevent line is missing from https://github.com/socketio/socket.io-fiddle/blob/issues/socket.io/5213/server.js#L33. Is it present in your tests ?

@darrachequesne
Copy link
Member

Yes it is:

server listening at http://localhost:3000
connect NQN6DbbcOZBzIYFTAAAB
onAny event
onevent
disconnect NQN6DbbcOZBzIYFTAAAB due to client namespace disconnect
connect 8ZYrenG0SM1w85vMAAAD
onAny event
onevent
disconnect 8ZYrenG0SM1w85vMAAAD due to client namespace disconnect

Ubuntu 22.04, Node.js v22.8.0 and v20.15.1.

@asideofcode-dev
Copy link
Author

asideofcode-dev commented Oct 23, 2024

OSX, Node v20.16.0 and Debian Bookworm, Node v22.6.0 both show this issue for me. How very strange!

@asideofcode-dev
Copy link
Author

Maybe easier to reproduce in a container

➜  socket.io-fiddle git:(issues/socket.io/5213) ✗ docker run -p 1234:1234 --rm -it -v $PWD:$PWD -w $PWD node:23-bookworm bash
root@952ad7e0d8bb:/Users/m1/Desktop/repos/socket.io-fiddle# PORT=1234 node ./server.js 
server listening at http://localhost:1234
connect KgT7cuONDu7NGHRoAAAB
onAny event
disconnect KgT7cuONDu7NGHRoAAAB due to client namespace disconnect

@asideofcode-dev
Copy link
Author

Ping @darrachequesne , thoughts on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants