README update for onerror callback

[jonathanhoskin]

Adds:

The `onclose` callback is not invoked when a connection is closed as a result of this type of error.

[mloughran]

The change you've proposed is wrong:

• onclose is called if the connection was open (and onopen was previously called).

Handshake errors only call onerror because it makes no sense to call onclose for connections that were never open. Feel free to attempt another edit if you still feel the readme is unclear :)

[jonathanhoskin]

Hmm, I think I may be missing something with our code then. The problem we are having is that our app is leaking memory in the hash that statefully tracks all our websocket client connections.

We use a global hash @connected_sockets to track connected websocket clients. We were only ever removing sockets from the hash when onclose was called.

As you can see below, the only place we add objects to @connected_sockets is in onopen. To plug the leak we had to implement a cleanup in onerror because onclose was not being called.

We also call close_websocket in onerror, although that may just be voodoo.

Here is our pattern:

@connected_sockets = {}

EM::WebSocket.start(em_opts) do |ws|
  ws.onopen { |handshake|
    @connected_sockets[ws] = { :path => handshake.path }
  }

  ws.onmessage { |data|
    handle_websocket_message(ws, data)
  }

  ws.onclose {
    @connected_sockets.delete(ws)
  }

  ## We have added onerror to fix the leak
  ws.onerror { |error|
    @connected_sockets.delete(ws)
    ws.close_websocket
  }
end

[mloughran]

Do you every call close on the websocket in application code? If so there is a very rare bug and I can send you a workaround (but you wouldn't be getting onerror either). If you're not it definitely won't change anything, and I'm very surprised to hear this result. Could you add enough logging to find out which onerror calls are not associated with an onclose?

[jonathanhoskin]

The only place we explicitly call close_websocket is in onerror as above; I'll turn up our logging level and get you some more info.