fix disconnect detection bug

[snorpey]

Hi,

I'm working on an installation where I need to be able to tell reliably when a client was disconnected. For this, I'm using sb-admin. It looks like sometimes Spacebrew has issues with detecting correctly if (or when) a client disconnected.

I wrote a little test app (download here) to have a closer look at the behavior.

In the video capture you see a normal client on the left side and an admin client on the right side. Ideally, they should always be in sync. As you can see often times the admin client on the right side is not notified if the client disconnects.

I was able to fix this issue by setting a flag on the connection object when the socket is closed. In the cleanupClosedConnections I'm then checking if the flag was set instead of looking at the socket.

While this is fixing an issue I'm having, I'm not sure if my changes interfere with other use cases or if this behavior is actually intended.

[quinkennedy]

there are a few other flags stored on the connections object. They are using the prefix spacebrew_ in an attempt to namespace them to avoid potential conflicts with changes in the connections object or other objects.

could you use the same "namespace" on this flag?

[quinkennedy]

My other question is:

is it guaranteed that a "close" event will be triggered before the _socket becomes null (or falsey)? In other words, is your new logic handling a superset of cases as compared to the _socket == null approach?

[snorpey]

@quinkennedy just added the spacebrew_ prefix.

according to the source code of the ws module, the close event is emitted before _socket is set to null: https://github.com/websockets/ws/blob/c311b7ef225e4546a7d1cfad457708dffe55ae84/lib/WebSocket.js#L895

[robotconscience]

@quinkennedy this seems good, what do you think?