oortcloud / node-ddp-client

A callback style DDP (Meteor's Distributed Data Protocol) node client.
Other
263 stars 80 forks source link

Only have at most one reconnect timer active. #30

Closed jagill closed 10 years ago

jagill commented 10 years ago

We were trying to reconnect on both error and close, which when both were thrown, would start two reconnect attempts. This was bad because each of the reconnect event would emit both an error and a close event, which quickly went to an exponentially infinite loop crashing my machine very very hard.

Now we have at most one reconnect timeout (which is cleared on successful connection, for good measure), and we don't try to reconnect on error. faye-websocket provides the error event purely for informational purposes; reconnection should be attempted on close.

Tarang commented 10 years ago

Im not too sure what the faye-websockets behavior is

If there is a socket error such as a connection interruption shouldn't it attempt to reconnect?

When a socket is closed is it not when when its been closed gracefully? Is it called when there is an 'error' type disconnection?

jagill commented 10 years ago

Copying from https://github.com/faye/faye-websocket-node/blob/master/README.md :

From the documentation, and from my experimentation, close is emitted whenever the socket is closed, gracefully or not. error seems to be an additional event emitted, and since it says we don't need to implement error recovery, presumably it's not the place for that?

But as long as there can only be one reconnection timeout, I'm find either way. It'll prevent the exponential infinite loop in either case.

Tarang commented 10 years ago

Thanks for this. Btw do you know how faye-websockets handles ssl connections? How would we provide a root ca to it if the machine doesn't have one? Are there more docs than the one on github

jagill commented 10 years ago

I'm assuming you mean as a client? Generally, you just tell it whether the protocol is ws or wss. We use it for wss all the time. We haven't tested it much against a machine with a missing cert or root ca.