Connect: Uncaught Exception on Connection Error

[MutableLoss]

Issue: Failed connections persist after error, causing the connection to stay open for a long period of time.

Error

(node:13401) UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 192.168.1.101:22
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1162:14)
(node:13401) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block,or by rejecting a promise which was not handled with .catch(). (rejection id: 16)
(node:13401) UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 192.168.1.101:22

Adding a catch to the wrapper promise causes this error to duplicate (just catching the same error). It seems to stop after 12 occurrences in VSCode, and 30 times in a spec test.

Following the process to see where it's hitting, it looks like this is happening directly on the emit method after the connect in the SSHConnection. After that it throws the unhandled promise, but it doesn't seem to close the connection after that, or even hit the error event. Throwing a catch on the connect does wrap the error, but that's about it.

Anyways, I hope that helps, and at least it stops eventually. 😄

[sanketbajoria]

@3DEsprit Actually retry logic is inbuilt, by default this options goes into into sshconfig

It won't retry only on Authentication error, otherwise it will retry. We can override this property, by setting it to appropriate value. Actually i should update the documentation, for this.

[MutableLoss]

Ah-ha, great to know. Thanks!

Leaving open as reminder to the document addition. 👍

[sanketbajoria]

@3DEsprit , Ah, I forgot, i have already defined that in documentation. (Readme)

[MutableLoss]

Thanks!

[MutableLoss]

I completely forgot, is there a way to catch the exceptions thrown? I'm getting this on shell, and spawn even with successful connections, and adding to the connection promise isn't doing it.

[sanketbajoria]

It should not throw exception on successful connection.

Can you please give some more detail

[MutableLoss]

It's only the failed connections. I'm just looking to catch the specific connection error to return, instead of the unhandled exception that is thrown. It is not a huge deal, but I thought I would ask.

[samhon18]

It seems the promise object only handles the first connection error. Once we enable the reconnect function, the exception thrown by "reconnect" will not call any promise reject callback. And Promise warn it

The first error throw by connect function was caught and pass to promise reject callback.

[2018-11-11T00:26:56.949] [ERROR] FilePush - [10.xx.xx.xx] Failed to connect to server { Error: Timed out while waiting for handshake at Timeout._onTimeout (E:\coinSource\DEV\app\node_modules\ssh2\lib\client.js:695:19) at ontimeout (timers.js:436:11) at tryOnTimeout (timers.js:300:5) at listOnTimeout (timers.js:263:5) at Timer.processTimers (timers.js:223:10) level: 'client-timeout' }

The error thrown by the reconnect function are not passed to promise reject callback and we get a unhandledPromiseRejectWarning.

(node:5228) UnhandledPromiseRejectionWarning: Error: Timed out while waiting for handshake at Timeout._onTimeout (E:\coinSource\DEV\app\node_modules\ssh2\lib\client.js:695:19) at ontimeout (timers.js:436:11) at tryOnTimeout (timers.js:300:5) at listOnTimeout (timers.js:263:5) at Timer.processTimers (timers.js:223:10) (node:5228) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1) (node:5228) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. (node:5228) UnhandledPromiseRejectionWarning: Error: Timed out while waiting for handshake at Timeout._onTimeout (E:\coinSource\DEV\app\node_modules\ssh2\lib\client.js:695:19) at ontimeout (timers.js:436:11) at tryOnTimeout (timers.js:300:5) at listOnTimeout (timers.js:263:5) at Timer.processTimers (timers.js:223:10) (node:5228) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2) (node:5228) UnhandledPromiseRejectionWarning: Error: Timed out while waiting for handshake at Timeout._onTimeout (E:\coinSource\DEV\app\node_modules\ssh2\lib\client.js:695:19) at ontimeout (timers.js:436:11) at tryOnTimeout (timers.js:300:5) at listOnTimeout (timers.js:263:5) at Timer.processTimers (timers.js:223:10) (node:5228) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 3) (node:5228) UnhandledPromiseRejectionWarning: Error: Timed out while waiting for handshake at Timeout._onTimeout (E:\coinSource\DEV\app\node_modules\ssh2\lib\client.js:695:19) at ontimeout (timers.js:436:11) at tryOnTimeout (timers.js:300:5) at listOnTimeout (timers.js:263:5) at Timer.processTimers (timers.js:223:10) (node:5228) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 4)

Is there any suggestion so that we can add resolve or reject callback for the reconnect function?

[lqueryvg]

I'm suffering from this too.

I'm doing data = await sftp.readdir() after which I've got some more code I want to execute (if the read works). If the connect fails (e.g. ECONNREFUSED), it throws the error, which I catch and handle, but it retries in the background !? It doesn't make any sense. What's the point of throwing an error and retrying in the background ? If any of the subsequent retries was to work, the result would be useless to me because the original call stack has gone (because it threw the error).

Surely it should only throw the error once all retry attempts have failed ?

[sanketbajoria]

@3DEsprit @lqueryvg Didn't get much time to see this issue. Will be fixed by next release.

[sanketbajoria]

@3DEsprit , @lqueryvg Fixed it, Haven't published it to npm. Can you please verify it, if things are looking good at your end

[MutableLoss]

I'll try to find some time this weekend to review, if @lqueryvg doesn't find the time first.

[lqueryvg]

@sanketbajoria - yes this looks good to me.

The new behaviour I see is that it it doesn't throw the error until all retries have been exhausted, which is correct IMO.

Good work !

[sanketbajoria]

@lqueryvg Thank you, for verifying at your end.

Published a new version 0.1.3 to npm.