Ws: Error thrown from library when pinging clients which went to closing state.

[jorenvandeweyer]

• [x] I've searched for any related issues and avoided creating a duplicate issue.

Description

Error thrown from library when pinging clients which went to closing state.

Reproducible in:

version: 5.1.1
docker: node:carbon

Steps to reproduce:

have a client close the connection exactly at the same moment when trying to ping a client from the server.

function create(server) {
    wss = new WebSocket.Server({
        server,
        verifyClient,
    });

    setInterval(ping, 30000);

    //...
}

function ping() {
    wss.clients.forEach((ws) => {
        if (!ws.isAlive) {
            return ws.terminate();
        }
        ws.isAlive = false;
        ws.ping();
    });
}

Expected result:

The server shouldn't trying to ping clients that are in readyState 2 (CLOSING)

Actual result:

/usr/src/app/node_modules/ws/lib/websocket.js:255
      throw err;
      ^

Error: WebSocket is not open: readyState 2 (CLOSING)
    at WebSocket.ping (/usr/src/app/node_modules/ws/lib/websocket.js:249:19)
    at wss.clients.forEach (/usr/src/app/src/websocket/index.js:104:12)
    at Set.forEach (<anonymous>)
    at Timeout.ping [as _onTimeout] (/usr/src/app/src/websocket/index.js:99:17)
    at ontimeout (timers.js:482:11)
    at tryOnTimeout (timers.js:317:5)
    at Timer.listOnTimeout (timers.js:277:5)

Attachments:

[lpinca]

Check the ready state before calling ws.ping() or use a callback, this is not a bug.

[jorenvandeweyer]

Not going to discuss if it's a bug or not, but ws.ping() should never be called on any closing socket. So it would be an improvement if it would be checked in the library instead of in your own code.

[lpinca]

The same happens with ws.send(), ws.pong() or any method that writes to the socket. It's user responsibility to check the readyState before writing the WebSocket.

I don't know why it was designed like this, I think because the browser WebSocket had the same behavior (not sure if it's still like this but an error was thrown when writing to a WebSocket whose readyState was not OPEN).

Changing this behavior after many years doesn't make much sense imho.

[lpinca]

I'm going to close this because as discussed this works as intended and will not change for now unless there is a big demand for it.