ruimarinho / bitcoin-core

A modern Bitcoin Core REST and RPC client.
480 stars 188 forks source link

add connection keep alive to command method #34

Closed yujinlim closed 6 years ago

pedrobranco commented 6 years ago

AFAIK bitcoind does not handle very well multiple keep-alive connections (https://github.com/bitcoin/bitcoin/issues/5262).

Despite that, this feature should be optional (a new option via client constructor) and also should be implemented a call with the header to close the connection (care to see this test).

Also, please do not commit dist/* files since they are committed in each release.

yujinlim commented 6 years ago

Thanks for the feedback. Will make the changes and recommit. Thanks

On 27 Sep 2017 9:51 PM, "Pedro Branco" notifications@github.com wrote:

AFAIK bitcoind does not handle very well multiple keep-alive connections ( bitcoin/bitcoin#5262 https://github.com/bitcoin/bitcoin/issues/5262).

Despite that, this feature should be optional (a new option via client constructor) and also should be implemented a call with the header to close the connection (care to see this test https://github.com/bitcoin/bitcoin/pull/5436/files#diff-58b857d414a55b5d060d30e0185217f9R65 ).

Also, please do not commit dist/* files since they are committed in each release.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ruimarinho/bitcoin-core/pull/34#issuecomment-332527397, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQB_4DmIeDF_sLwJXKjbcjJp2tEWAKVks5smlLLgaJpZM4PfSad .

yujinlim commented 6 years ago

@pedrobranco i have restructured it, please have a look.

ruimarinho commented 6 years ago

@yujinlim I took a deeper look at this issue and we already supports this feature. Just make sure you set the following option when creating the client:

new Client({ agentOptions: { keepAlive: true }});

The request module will pass the options to the http(s).Agent and keep alive will be enabled :)

yujinlim commented 6 years ago

ahh, noted.