nknorg/nkn-shell-daemon

Gracefully handle non-open sockets before sending data

Closed this issue · 1 comments

Hi,

Currently the connection logic works by setting 2 timers in the client.on('connect', ... path.
My other report (#2) was for the first timer (pinging). This report is for the second timer (sending data).

In that second timer a message is attempted to be send. But if the connection went broken after the last ping then this code will fail in the client.send(...) path as that then uses a broken socket connection.

The real issue is that this timer is fired regardless of socket connection state. If a connection got lost, that timer shouldn't fire at all. A temporary fix would be to add a check in the try{...} part to verify the socket is still open when you want to send data with client.send(...).

Note that you can probably combine those 2 timers as they both use pingInterval. That would make the logic simpler i think.

Cheers,
Mark

The real issue is that this timer is fired regardless of socket connection state. If a connection got lost, that timer shouldn't fire at all.

Actually it is designed to be so. If the connection is not open or get lost, send will fail (or send success but cannot get response before timeout) and lastUpdateTime will not be updated. If lastUpdateTime is not updated for a while (3 * pingInterval), it means the client has some problem (regardless of the cause), so it will try to reconnect.