Issue with .to(room).emitWithAck()

[terryluan12]

Describe the bug I came across a bug where if you broadcast an event to a room, it times out unless you explicitly set a timeout variable.

To Reproduce

Please fill the following code example:

Socket.IO server version: ^4.0.0

Server

import { createServer } from "node:http";
import { readFile } from "node:fs/promises";
import { Server } from "socket.io";

const port = process.env.PORT || 3000;

const httpServer = createServer(async (req, res) => {
  if (req.url !== "/") {
    res.writeHead(404);
    res.end("Not found");
    return;
  }
  // reload the file every time
  const content = await readFile("index.html");

  res.writeHead(200, {
    "Content-Type": "text/html",
    "Content-Length": Buffer.byteLength(content),
  });
  res.end(content);
});

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

const roomCode = "Test room code"

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

  socket.on("joinRoom", async (username, ack) => {
    console.log(`${username} joining`);
    socket.join(roomCode);

    // This doesn't work
    const response = await socket.to(roomCode).emitWithAck("userJoined", username);

    // This does
    // const response = await socket.to(roomCode).timeout(5000).emitWithAck("userJoined", username);

    ack(response);
  })

});

httpServer.listen(port, () => {
  console.log(`server listening at http://localhost:${port}`);
});

Socket.IO client version: ^4.0.0

Client

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

const port = process.env.PORT || 3000;

const socket = io(`http://localhost:${port}`);
const socket2 = io(`http://localhost:${port}`);


socket.on("userJoined", (username, ack) => {
  ack("username1")
})

const response1 = await socket.emitWithAck("joinRoom", "username1");
console.log(`Response 1 is ${response1}`);

const response2 = await socket2.emitWithAck("joinRoom", "username2")
console.log(`Response 2 is ${response2}`);

Expected behavior It should work with or without the explicit timeout set.

Platform:

• OS: Ubuntu 22.04.5 LTS

[terryluan12]

In addition, can I get some clarification whether a emitWithAck chained to a to(room) is supported. Officially, it seems not to be per this, but I've seen multiple places where people are using it, such as here, or even in the code(if I'm reading it correctly). Am I misunderstanding the documentation, or are the documents outdated?

[darrachequesne]

@terryluan12 the timeout value is indeed mandatory, in order to prevent memory leaks (for example, if some clients fail to reply).

We could maybe add a default timeout in the options though (mirroring the ackTimeout on the client-side).

Officially, it seems not to be per this,

Oops, I've updated the documentation, because acknowledgements are now supported when broadcasting (since [email protected], released in 2022). Good catch!

[ZachHaber]

If we add a default ackTimeout, we should also allow emitWithAck on the io class directly (and update the typedef to allow that as well) since that was turned off as it was just a footgun without a way to set an timeout.

If we aren't defaulting the ackTimeout option to anything if the user doesn't specify, then I think it would make sense to have emit throw an error telling the dev to add a timeout if timeout isn't specified when there's an ack (which should apply for emitWithAck as well - as that's just a wrapper around emit - that way there's a better error message than just operation has timed out for those instances.