pgriess / node-websocket-client

A Web Socket client for NodeJS
BSD 3-Clause "New" or "Revised" License
150 stars 68 forks source link

If close() isn't passed a timeout, finishClose is never called and the socket isn't cleaned up #6

Closed josephg closed 13 years ago

josephg commented 13 years ago

In close():

        if (timeout) {
            setTimeout(finishClose, timeout * 1000);
        }

Currently if you don't pass a timeout to close, finishClose is never called, and the socket is never cleaned up.

Maybe something like this would be appropriate:

        if (timeout) {
            setTimeout(finishClose, timeout * 1000);
        } else {
            finishClose();
        }

... or maybe a have a default value for timeout?

pgriess commented 13 years ago

The sendClose() method immediately above the code you've quoted initiates a graceful shutdown sequence with the server. We intentionally wait for the server's close frame to complete shutdown of the connection. This happens in serverCloseHandler().

The nonstandard timeout parameter was added to the close() method to handle the case of servers that do not correctly handle this close flow. When I last tested this, miksago/node-websocket-server didn't handle close, for example.

josephg commented 13 years ago

Ah.

miksago/node-websocket-server is the one I'm using. I'll file an issue there, and keep using the timeout value for now.

Thanks :)