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

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

Open
ruxkor opened this issue Jun 20, 2012 · 1 comment

Comments

@ruxkor
Copy link
Contributor

ruxkor commented Jun 20, 2012

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
Copy link
Contributor Author

ruxkor commented Jun 27, 2012

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).

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