Flaw in disconnect path of the library

sanjayssk
sanjayssk edited May 25 in Node JS client
KiteConnect Node.js ticker has a serious design flaw in the disconnect path.

If I want to behave like a good citizen and want to disconnect a ticker after use (for example after completing tracking of a Nifty options contract), the library terminates the entire Node.js process unexpectedly. The issue comes from this flow:
  • disconnect()` sets `should_reconnect = false`
  • websocket closes normally
  • triggerDisconnect()` still calls `attemptReconnection()`
  • attemptReconnection()` checks `!should_reconnect`
  • library calls `process.exit(1)`
This means a normal manual disconnect call from the App is treated as a fatal application failure. A library should never terminate the host process internally. That decision must belong to the application layer.

Current code:
if (
(current_reconnection_count > reconnect_max_tries)
|| !should_reconnect
) {
trigger('noreconnect');
process.exit(1);
}
Suggested fix:

if (!should_reconnect) {
// Manual disconnect requested.
// Do not reconnect.
// Do not terminate process.

trigger('noreconnect');
return;
}

if (current_reconnection_count > reconnect_max_tries) {
trigger('noreconnect');
return;
}
This problem can unexpectedly terminate long-running trading systems that dynamically create and destroy tickers/contracts during live trading.

Anyone listening?
I now also have a small test script to demonstrate this. If I create a KiteTicker object with reconnect set to false, there is no problem disconnecting the ticker. But if I set it to true, process dies as soon as I disconnect. A normal trade app is supposed to subscribe to multiple tickers during its normal course of operation and it also wants the reconnect feature. But if the app performs the "good programming practice" of cleaning up by disconnecting the ticker after use is over, it dies. This is a library design fault. Don't point me to previous tickets as I have seen them. One must be able to use reconnect feature and at the same time disconnect and connect as many times in the App lifetime as needed.
  • sanjayssk
    Update: I bypassed this locally and found the real problem. Disconnect as implemented does not work at all and the feed keeps coming. That's why someone just killed the process instead of fixing that disconnect code. Have to live with this now. The best solution is to make a map of open contracts and either reuse or open a new feed based on whether it is found in the map or not.
  • salim_chisty
    We're currently checking this from our end. Kindly allow us some time to verify the behaviour and investigate it further.
Sign In or Register to comment.