Websocket connection argument

bennycode / coinbase-pro-node

Coinbase API written in TypeScript and covered by tests.

https://bennycode.com/coinbase-pro-node

MIT License

254 stars 62 forks source link

Websocket connection argument #105

Closed baloian closed 4 years ago

baloian commented 4 years ago

Hi @bennyn, I have been running the library and so far so good. I would like to point out an issue I got today, though this is not a library issue, I think.

So, today websocket was trying to reconnect for more than 15 minutes and it couldn't. I restarted the program and it connected. Even though the Coinbase Pro documentation says that it is 4 seconds interval I think it prevents to reconnect at some point (if you keep reconnecting).

Don't you think it would be a good idea to provide an option to set a reconnection interval when websocket connects (in connect() method)?

Please let me know what do you think.

bennycode commented 4 years ago

Hi @baloian, accepting reconnecting options in the connect() method is an excellent idea! I will add this in a few hours. This will also help @Slurpgoose's use case in bennyn/coinbase-pro-node#102. 😃

baloian commented 4 years ago

Great! please let me know when you will add it.

P.S. I have never written TypeScript (though it is the same as JS) code, otherwise I would have created a PR.

bennycode commented 4 years ago

The changes are there!

You can now pass reconnection options to the connect method.

There are a few things to mention:

You can avoid automatic reconnects with options set to: {maxRetries: 0}
Calling connect twice (without calling disconnect inbetween) will cause an error
The connect method does not return a Promise anymore and instead returns you the instance of ReconnectingWebSocket directly, so you have to listen to the WebSocketEvent.ON_OPEN event to know if it is connected

All changes mentioned above are in coinbase-pro-node v1.6.0.

Please report if it solves your issue. ✏️

Slurpgoose commented 4 years ago

@bennyn this is great thank you! I will close my issue as well.

baloian commented 4 years ago

@bennyn thank you very much, I will test it. For now I am closing the issue.