Add timeout method to remote socket

[walde1024]

The kind of change this PR does introduce

• a bug fix
• a new feature
• an update to the documentation
• a code change that improves performance
• other

Current behavior

The RemoteSocket does currently not expose a timeout method. With that I encountered problems when emitting events by using a remote socket that I fetched from another instance by using the redis adapter.

When I emit a message like this:

remoteSocketFromServer2.emit('send', { key: 'value' }, (err, res) => {
    if (err) {
        ...
    } else {
        ...
    }
});

then the client receives the emitted message but the callback of the sender is invoked immediately with a timeout error. Reason for that is that the flags.timeout of the BroadcastOperator is not set when emitting the message.

New behavior

Exposes a timeout method that can be called on the RemoteSocket to set a timeout.

Other information (e.g. related issues)

I encountered this problem by using the following dependencies:

{
    "socket.io": "^4.5.4",
    "socket.io-client": "^4.5.4",
    "@socket.io/redis-adapter": "^8.0.0",
}

The following piece of code can be used to reproduce the issue:

import Redis from 'ioredis';
import { createAdapter } from '@socket.io/redis-adapter';

import { Server } from 'socket.io';

import { io as ioClient } from 'socket.io-client';

describe('Redis socket io store System Test', () => {
    let io1: Server;
    let io2: Server;

    beforeAll(async () => {
        io1 = new Server();
        io1.listen(5001);
        setupRedisAdapterForServer(io1);

        io2 = new Server();
        io2.listen(5002);
        setupRedisAdapterForServer(io2);
    });

    function setupRedisAdapterForServer(io: Server) {
        const pubClient = new Redis({
            port: 6379, // Redis port
            host: '127.0.0.1', // Redis host
            username: 'default', // Not used
            password: 'my-top-secret', // Not Used
            db: 0 // Defaults to 0
        });

        const subClient = pubClient.duplicate();
        io.adapter(createAdapter(pubClient, subClient, { requestsTimeout: 5000 }));
    }

    it('scenario', async () => {
        const socket2 = await connectSocket(5002);

        socket2.on('send', (msg, ack) => {
            ack('OK');
        });

        const socketsFromServer2 = await io1.in(socket2.id).fetchSockets();
        const socketFromServer2 = socketsFromServer2[0];

        let result;
        socketFromServer2.emit('send', { key: 'value' }, (err, res) => {
            if (err) {
                result = err;
            } else {
                result = res[0];
            }
        });

        while (!result) {
            await delay(250);
        }

        expect(result).toBe('OK');
    });
});

const delay = (ms) => new Promise((resolve) => setTimeout(resolve, ms));

async function connectSocket(port) {
    const socket = ioClient('http://localhost:' + port, {
        transports: ['websocket']
    });

    const promise = new Promise((resolve, reject) => {
        socket.on('connect', () => {
            resolve('success');
        });
        socket.on('connect_error', (err) => {
            reject(err);
        });
    });

    await promise;

    return socket;
}

[olocwtowns]

I ran into the same issue today. The bug means you can't actually receive client ack's in all scenarios when using an adapter in clustered servers. I am working around it for now by writing to the operator.flags.timeout property on my RemoteSocket instance 🤢.

[darrachequesne]

@walde1024 thanks for opening this pull request 👍

The only problem is that local and remote Sockets will not behave similarly, because local Socket will expect one single response:

socket.timeout(1000).emit("hello", (err, res) => {
  console.log(Array.isArray(res)); // false
});

While a remote Socket will return an array with a single response (since it's a broadcast under the hood):

remoteSocket.timeout(1000).emit("hello", (err, res) => {
  console.log(Array.isArray(res)); // true
});

[walde1024]

@darrachequesne :

Thanks for your reply. My understanding of socket.io and how it works under the hood is very basic but isn't the problem you are mentioning already there? My PR just exposes the timeout method. The handling of the response stays as it is. Right now the caller of the emit method needs to know that he is calling emit on a socket or on a remote socket because:

1. Calling .timeout on a remote socket will fail
2. Handling of the result is different

My PR would just solve the first point including the bug that the timeout hits immediately.

[darrachequesne]

@walde1024 you are right, I was just wondering whether we could elegantly fix both problems.

[darrachequesne]

Merged as 0c0eb00. Thanks a lot 👍

[Geczy]

thanks for fixing! i came here to report this issue but see its finished