Add `socket.request()` as promise wrapper around callbacks

[sebamarynissen]

I noticed that in the 4.4.0 release, a timeout flag was added:

socket.timeout(5000).emit("my-event", (err) => {
  if (err) {
    // the client did not acknowledge the event in the given delay
  }
});

Interestingly I implemented similar logic in an application of mine by patching client-side sockets, but I combined it with promises as well. Something like

await socket.request('event', 'some data');

where the promise times out after a fixed amount of time if the server did not acknowledge.

As a result, I was wondering whether it would a nice feature to add to the library as well. The api would look something like

// Awaits indefinitely for a server response
await socket.request('event', 'some data');

// Times out after 5 seconds
await socket.timeout(5e3).request('event', 'some data');

This could be accompanied by a reply() and replyAny() method as well of course, which looks something like this and hence hides the callbacks from the user alltogether:

socket.reply('event', async (data) => {
  return res.toJSON();
});
socket.reply('event', async (...data) => {
  throw new Error('Something went wrong');
});
socket.replyAny(async (event, ...data) => {
  return res.toJSON();
});

Additionally I think it might be useful to have a way of resurrecting errors as well, but I'm not sure how the api can look like. Perhaps something like

socket.errorBuilder(json => buildCustomErrorFromJSON(json));

or

io.errorBuilder(json => buildCustomErrorFromJSON(json));

where socket.errorBuilder can be used to override for that socket.

As always, I'd be happy to create a PR for this should you decide that it is a feature that is desirable to have in the library. In my opinion it fits nicely in the trend that callbacks are more and more being replaced by promises.

[sebamarynissen]

The implementation of the feature can be straightforward:

class Socket {

  constructor() {
    this.buildError = err => this.io.buildError(err);
  }

  errorBuilder(fn) {
    this.buildError = fn;
  }

  request(...args) {
    return new Promise((resolve, reject) => {
      args.push((err, response) => {
        if (err) {
          reject(this.buildError(err));
        } else {
          resolve(response);
        }
      });
    });
  }

  reply(event, handler) {
    this.on(event, (...args) => {
      let cb = args.at(-1);
      if (typeof cb === 'function') {
        cb = args.pop();
      } else {
        cb = (err) => this._error(err);
      }

      let resolve = res => cb(null, res);
      let reject = err => cb(err);

      try {
        let res = handler.call(this, ...args);
        if (res && res.then) {
          res.then(resolve, reject);
        } else {
          resolve(res);
        }
      } catch (e) {
        reject(e);
      }

    });
  }

}

[darrachequesne]

Yes, that was on my mind too, but then we'd have to include a polyfill for browsers that do not support Promises..

See also: https://github.com/socketio/socket.io/discussions/4144

[sebamarynissen]

Wouldn't it be an option to not include a polyfill and leave it up to the user? If the user targets browsers that don't support promises, he would either:

• Need to polyfill Promise globally
• Choose not to use request/reply.

[darrachequesne]

@sebamarynissen that makes sense :+1:

Let's keep this issue open, to see if other users want this functionality to be built-in.

[MiniSuperDev]

I would like this functionality, and it also helps to write code and tests more readable using async await syntax.

But I'm not convinced by the name request, as I understand, the idea is to allow to change the callback syntax of acknowledgement for an await async syntax, so in this way, and without breaking changes and aligned with current naming convention this would be better

• emitWithAck

and in the same way reply should be onWithAck , although I still see a lot of use for this one, so I appreciate if someone would like to show me an example where it would be useful to use await async when registering event handlers

For example this: https://github.com/socketio/socket.io/blob/3b7ced7af7e0a2a66392577f94af1ee5ed190ab1/examples/basic-crud-application/server/test/todo-management/todo.tests.ts#L165-L200

to something like this;

describe('update todo', () => {
  it('should update a todo entity', async () => {
    const todoId = 'c7790b35-6bbb-45dd-8d67-a281ca407b43';
    todoRepository.save({
      id: todoId,
      title: 'lorem ipsum',
      completed: true,
    });

    const todoUpdated = {
      id: todoId,
      title: 'dolor sit amet',
      completed: true,
    };

    const onTodoUpdated = jest.fn();

    otherSocket.on('todo:updated', onTodoUpdate);

    await socket.emitWithAck('todo:update', todoUpdated);

    const storedEntity = await todoRepository.findById(todoId);

    expect(storedEntity).toEqual(todoUpdated);
    expect(onTodoUpdated).toBeCalledWith(todoUpdated);
  });
});

Is there any plan to implement it?

Thank you.

[darrachequesne]

For future readers:

emitWithAck() was added in https://github.com/socketio/socket.io/commit/184f3cf7af57acc4b0948eee307f25f8536eb6c8, included in version 4.6.0.

io.on("connection", async (socket) => {
    // without timeout
  const response = await socket.emitWithAck("hello", "world");

  // with a specific timeout
  try {
    const response = await socket.timeout(1000).emitWithAck("hello", "world");
  } catch (err) {
    // the client did not acknowledge the event in the given delay
  }
});

Have a great day!