ssh2-sftp-client: ECONNRESET when using fastGet exits my script instead of throwing an error

I have a script that loops over several thousands of files on the FTP. I’m using fastGet to download each file. The script runs for several hours. Sometimes ECONNRESET is thrown when using fastGet in (as it seems) a random moment (2 out of 3 last runs). And it breaks my script.

The problem is that the following code does not work as I’d expect it to:

try {
    await sftpClient.fastGet(filepath, targetFilepath);
} catch (error) {
    console.log("This never executes, but I expect it to.");
    console.log("I'd be happy to put some code to re-establish the FTP connection here.");
}

This is the output of the script:

/my-script/node_modules/ssh2-sftp-client/src/utils.js:80
    throw formatError(err, name);
    ^

Error: : read ECONNRESET
    at formatError (/my-script/node_modules/ssh2-sftp-client/src/utils.js:54:18)
    at Client.<anonymous> (/my-script/node_modules/ssh2-sftp-client/src/utils.js:80:11)
    at Client.emit (events.js:210:5)
    at Socket.<anonymous> (/my-script/node_modules/ssh2/lib/client.js:307:10)
    at Socket.emit (events.js:215:7)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  code: 'ECONNRESET'
}

So I understand that this is what happens: uncaughtException event is raised. And as described in node.js docs:

The ‘uncaughtException’ event is emitted when an uncaught JavaScript exception bubbles all the way back to the event loop. By default, Node.js handles such exceptions by printing the stack trace to stderr and exiting with code 1, overriding any previously set process.exitCode

I’m not sure whether this is a bug or my expectations about try/catch block around fastGet call are wrong.

edit: I’m using latest version (4.3.0)

edit 2: As node docs state:

The correct use of ‘uncaughtException’ is to perform synchronous cleanup of allocated resources (e.g. file descriptors, handles, etc) before shutting down the process. It is not safe to resume normal operation after ‘uncaughtException’. To restart a crashed application in a more reliable way, whether uncaughtException is emitted or not, an external monitor should be employed in a separate process to detect application failures and recover or restart as needed.

So unfortunately it’s not easy to recover…

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 42 (25 by maintainers)

Most upvoted comments

The econreset error is caused when the remote sftp server resets the connection unexpectedly. In general, you cannot use try/catch to catch exceptions generated via an event emitter (see the node manual for an explanation). In basic terms, the problem is that the try/catch block may have exited before the error event fires. The only reliable way to deal with emitted exceptions is with a listener.

The problem is that it appears this error is being emitted by the socket object and the SSH2/SFTPStreams modules do not provide any API to attach event listeners on the low level socket object. The ssh2 object does have listeners for both error and uncaughtException events, but it would appear that the SSH2 module is not catching and re-emitting these events, so the listeners are unable to be of any use.

So, in short, I’m not sure how to catch these events. It could be this is something which needs to be fixed in the ssh2 module itself.

I will try to write a very simple script which will just use the ssh2 module and which uses only the available events provided by that module which can be used to download a large number of files. We can then use that script to see if we still cannot catch the exception via the listeners. If this is the case, then we can raise an issue with the ssh2 author. On the other hand, if the ssh2 scirpts is able to deal with it, it means I’ve got something wrong with the ssh2-sftp-client setup and will need to dig deeper.

give me a couple of days and I’ll get back to you with a scirpt. If you can help by running it to try and generate the error, this will help a lot (one of the problems here is in being able to reliably reporduce the issue).

The issue here, which I’m not sure how to solve, is that the event is not being raised by the ssh2 or ssh2-streams objects the the ssh2-sftp-client module wraps. I have a listener for uncaught exceptions on the ssh2 object, but it isn’t that object which is emitting the error - the error is being emitted by a low level socket module and there is no API provided by the ssh2/sftp-stream modules which would allow me to attached a listener on it to handle the exception.

What I think may work is to put a listener on the close event.

theophilusx on Dec 13, 2019

As the original issue (script exiting due to uncaught exception) has been resolved and I cannot reproduce the other issues, I am going to close this issue. Will release version 5.0.0 later today.

theophilusx on Jan 14, 2020

The fact the script didn’t just blow up I will take as a win 😃. No, calling end() when the sftp connection has already been closed does not cause an error.

theophilusx on Jan 6, 2020

This would result in any subsequent attempts to use the connection failing, which could be handled in the ‘normal’ way. (it would be nice to have a solution which does not require clients to add an error listener IMO).

This sounds reasonable to me. It appears that I would not have to modify my script for this, so as a library user I am all up for this change 👍

OK, lets see if this can work. I will let you know. Note that I was just starting to add two new features (uploadDir and downloadDir) and have refactored the code in preparation for v5.0.0. I will make a branch of there as it will be easier in the long run. will let you know what the new branch is once done (could take a couple of days as family commitments are ramping up for Christmas, so a little busy).

– Tim Cross

theophilusx on Dec 23, 2019

Well, whether I’d have to modify my script, depends on how would you implement this. Would this error be thrown as ECONNRESET or as something else?

Short answer yes, it will have ECONNRESET as the error.code

Long answer -

When the library throws an error, if the error it is throwing is the result of an error the library has caught, I use the same error code (the error message may be modified to provide more context, but the code is left alone). The library will set an error code if the error is not from a caught error (in which case a library specific error code will be used) or if the caught error does not have an error code (error codes were only standardised in node v10.x, so some libs still don’t set an error.code property in errors they generate).

So basically, I don’t change error codes, but will add one if one does not exist. All errors thrown by the lib should have an error.code property.

– Tim Cross

theophilusx on Dec 23, 2019