socket.emitWithAck doesn't throw when an error was passed as the first argument

[unek]

Describe the bug

To Reproduce
Socket.IO server version: 4.8.1

Server

import { Server } from "socket.io";

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

io.on("connection", (socket) => {
  socket.on('test', (data, callback) => {
    callback({ error: 'message' });
  });
});

Socket.IO client version: 4.8.1

Client

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

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

socket.on("connect", async () => {
  const result = await socket.emitWithAck('test', {});
  console.log(result); // { error: 'message' }
});

Expected behavior
calling callback on the server-side with a first argument (error) should make the client-side emitWithAck throw

by removing the fn.withError = true from the emitWithAck function, the functions throws with the error object correctly,

onack() function seems to be inserting null as the first argument to packet data if withError is set to true, moving the error to the second position, resulting in the promise resolving, not rejecting.

i'm able to fix it with

  socket.emitWithAck = function emitWithAck (event, ...args) {
    return new Promise((resolve, reject) => {
      socket.emit(event, ...args, (err, result) => {
        if (err) return reject(err);
        resolve(result);
      });
    });
  }

[darrachequesne]

Hi! Yes, you're right, acknowledgements indeed do not expect an optional error argument:

io.on("connection", (socket) => {

  // what we have now
  socket.on("hello", (data, callback) => {
    callback("hi");
  });

  // what we could have had instead
  socket.on("hello", (data, callback) => {
    callback(null, "hi");
  });
});

It has been designed this way a while ago, so changing this behavior now seems unlikely...

The withError flag is an internal flag used to handle emit with or without timeout value:

socket.timeout(3000).emitWithAck('test', {}); // withError true
socket.emitWithAck('test', {}); // withError false