inconsistency between ws.WebSocket.readyState and websocket.io/hybi/WebSocket.readyState

[ruxkor]

While testing the current master of engine.io/websocket.io we encountered the following error: [1]

I think the reason for this is an inconsistency between WebSocket objects in websocket.io and ws; the former only calls ws.write if it's readyState == 'open', and the latter throws an error if it's readyState != WebSocket.OPEN

Changing ws/WebSocket's behavior to emit instead of throwing errors (since it inherits from EventEmitter) makes the handling easier, but does, of course, not provide a remedy for this problem.

I am still trying to figure out a way to reproduce the error and will update the issue if I find something of relevance.

[1]

/my/app/node_modules/long-stack-traces/lib/long-stack-traces.js:81
                throw e;
                      ^
Error: not opened
    at WebSocket.send (/my/app/node_modules/engine.io/node_modules/websocket.io/node_modules/ws/lib/WebSocket.js:175:16)
    at WebSocket.write (/my/app/node_modules/engine.io/node_modules/websocket.io/lib/protocols/hybi.js:101:13)
    at WebSocket.send (/my/app/node_modules/engine.io/node_modules/websocket.io/lib/socket.js:105:8)
    at WebSocket.send (/my/app/node_modules/engine.io/lib/transports/websocket.js:69:17)
    at Socket.flush (/my/app/node_modules/engine.io/lib/socket.js:237:20)
    at Socket.sendPacket (/my/app/node_modules/engine.io/lib/socket.js:223:10)
    at Socket.send (/my/app/node_modules/engine.io/lib/socket.js:207:8)
    at Socket.send_ns (/my/app/node_modules/engine.ns.io-base/lib/engine.ns.io-base.coffee:38:19)
    at /my/app/server.coffee:161:27
    at Socket.get (/my/app/server.coffee:120:14)
----------------------------------------
    at EventEmitter.on
    at Object.<anonymous> (/my/app/server.coffee:150:23)
    ....

[ruxkor]

tl;dr
I have not quite wrapped my head around this one, but can it be possible that the hybi Socket object emits a readyState open, although the connection is actually already closed again?

some details

In order to visualize the whole procedure, I drew a callgraph, (handleUpgrade.dot.png)[https://github.com/ruxkor/engine.io/blob/master/docs/handleUpgrade.dot.png].

A small legend:

• blue, dashed: synchronos process flow
• blue, dashed: async process flow
• green, solid: place where a function is constructed, and also called, if there is no other green, dashed arrow
• green, dashed: where the function is called
• red, dashed: assumed order if the asynchronous functions

combining the blue, solid arrow with the red, dashed arrows should present the actual flow of engine.handleUpgrade.

Back to the problem:
In ws.Websocket.establishConnection the object sets its own readyState on OPEN, after attaching the first and dataHandler to be called on process.nextTick and socket.on('data') respectively. It also attaches cleanupWebSocketResorces to its socket object. Only then it emits 'open',

Only then the w.io.hybi.WebSocket.onOpen (fn 1) will be called, which in turn attaches a function to process.nextTick, that will set the Sockets readyState on open and emit 'open'.

If a socket gets cleaned up, because of a sudden disconnect, I think the w.io.hybi.WebSocket (fn 1.1) will still be called, since it was already attached to process.nextTick.

possible solution
One possibility would be to check the readyState of the ws object inside (fn 1.1), and only set its own readyState and emit if it equals WebSocket.OPEN, similiar how the firstHandler is doing it in establishConnection.

An imo even better solution would be to implement getters and setters for the websocket.io.WebSocket readyState. In this way, it would be trivial to return th actual readyState, i.e. the one of the ws object, but still using an own attribute for the drafts.js WebSocket (since there is no ws attribute to use here).