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

waitFor latency causes the test-suite to miss some packets #32

Open
Totodore opened this issue Oct 14, 2023 · 1 comment
Open

waitFor latency causes the test-suite to miss some packets #32

Totodore opened this issue Oct 14, 2023 · 1 comment

Comments

@Totodore
Copy link
Contributor

Currently, for each message check this function is called to wait for a message. However it causes a lot of failed checks because the latency of this function is quite high (because of the new event listener and the promise).

function waitFor(socket, eventType) {
    return new Promise((resolve) => {
        socket.addEventListener(
            eventType,
            (event) => {
                resolve(event);
            },
            { once: true }
        );
    });
}

It became really problematic with buffered messages. Recently I have added a flush mechanism on socketioxide to avoid flushing the websocket for each socket.io packet. Also because the engine.io connect packet is sent immediately after the websocket initialisation it may not be handled because of the same issue.

Here is an example where the waitFor function makes the test break and a hacky solution. However this happens almost everywhere. This issue can be reproductible even with the official test server with the following command to overload a bit the nodejs event loop :

seq 10 | parallel -j 10 'npm test -- -f "should connect then disconnect from a custom namespace"; echo '

it("should connect then disconnect from a custom namespace", async () => {
    const socket = await initSocketIOConnection();

    await waitFor(socket, "message"); // ping

    // Create a waiting promise *before* sending the trigger packet "40/custom" makes the trick
    const handshakeHandler = waitForPackets(socket, 2);
    socket.send("40/custom");

    // await waitFor(socket, "message"); // Socket.IO handshake
    // await waitFor(socket, "message"); // auth packet
    await handshakeHandler;

    socket.send("41/custom");
    socket.send('42["message","message to main namespace"]');

    const { data } = await waitFor(socket, "message");
    expect(data).to.eql('42["message-back","message to main namespace"]');
});

I didn't find any good solution to propose a PR. If there is no solution to this issue. I may consider rewriting the test suite in another language to avoid event loop issues. Maybe @darrachequesne you have an idea ?

@Totodore Totodore changed the title waitFor latency cause the test-suite to miss some packets waitFor latency causes the test-suite to miss some packets Oct 14, 2023
@Totodore
Copy link
Contributor Author

Totodore commented Oct 15, 2023

I found a solution with the nodejs stream adapter from the ws lib and the async iterator provided by node :

const socket = createWebSocketStream(new WebSocket(
    `${WS_URL}/socket.io/?EIO=4&transport=websocket`
  ), { objectMode: true, readableObjectMode: true });

socket.binaryType = "arraybuffer";

// The async iterator does not spawn a Promise + an event listener at each call
// Also the stream handles bufferred packets

await socket.iterator().next() // Engine.IO handshake

socket._write("40");

await socket.iterator().next(); // Socket.IO handshake
await socket.iterator().next(); // "auth" packet

return socket;

I don't know yet if I make a thin adapter for the ws socket so there is nothing much changed in all the test suites (socket.io + engine.io v3 & v4) or if I rewrite all the test suites with the async iterator way.

I'm open to suggestions :)

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

No branches or pull requests

1 participant