Application goes blank if network drops while ongoing ssh connection with a remote system

[einkal]

We are using the version "5.1.2" in our desktop application(electron based) and every time the network drops , the application crashes without logging any errors. Is this fixed in any higher version? Even if not fixed , can we expect graceful handling?

[steelbrain]

Can you please provide some logging or debug information about the crash? It's difficult to fix something without knowing what to fix

[einkal]

Can you please provide some logging or debug information about the crash? It's difficult to fix something without knowing what to fix

The error happens while the ssh tunnel is connected and network goes off...

**"node-shh @5.1.2 ".

Error: read ECONNRESET at TCP.onread (net.js) uncaughtException: read ECONNRESET**

[einkal]

@steelbrain The above error is uncaught one and makes my app crash

[steelbrain]

@einkal Thank you for the extra information. Can you please share which SSH command you execute?

[einkal]

the getFile or putFile method throws this @steelbrain

[einkal]

@steelbrain any update on this bug?

[brtheb]

I've been fighting this issue as well (with getFile and putFile) and node-ssh@12.0.0, so I'll share what I've found.

Both ECONNRESET and ETIMEDOUT seem to produce this issue. Most node-ssh commands properly handle errors and result in the top level promise for the method call to be rejection, just as one would expect. The two errors above, however, aren't correctly handled in node-ssh and essentially result in errors being emitted in a situation where there are no listener callbacks registered to handle them; this results in a promise rejection which cannot be caught by a try/catch in an async function or via .catch() directly on the promise in the user code.

From what I've been able to tell, one or both of the errors above trigger events after the close event occurs on the connection, at which point node-ssh no longer has any error handlers registered.

@einkal I'm willing to bet you're using Node 16.x, that's part of the reason why you're application is terminating. In Node 15.x, the default behavior for unhandled promise rejections was changed from warn to throw (see this), which essentially results in your application exiting when it encounters an unhanded promise rejection.

---

My best attempt at reproducing the conditions which trigger these events was to create a NodeSSH instance, and connect to a client. Then manually force the client to lose connection, and attempt to call a NodeSSH command, and wait for it to time out. This doesn't, however, explain why we've been having issues with getFile or putFile specifically.

[steelbrain]

Hi y'all!

I've been trying to figure out why it would happen, and so far I've got nothing, at least on the node-ssh side of things. When you do getFile, these are the async operations that could go wrong:

• requestSFTP
  • connection.sftp
• fastGet

All of them have appropriate promise error handling, and the errors are propogated downwind with promises, so long as you are adding a .catch to your .getFile, this should be a-ok.

In addition, we are adding a general error-handler to the connection itself in .connect().

Based on the above information, I think it would be really helpful if someone who is able to reproduce this error consistently, uses the source code to write minimal reproducable error case, that's built directly on top of ssh2 which is what this package uses.

I think mscdex may be able to help us here, as the underlying library is much more complex and I have no understanding of it, so we'll have to cross-post with the above information to the ssh2 issue tracker

[sowizz]

Hi,

I have similar problem, in my case it's Error: write EPIPE error which is also crashing the app. I've encountered this one as I've tried to reproduce ECONNRESET that is sometimes happening during putFiles. Error: write EPIPE is much easier to reproduce though. You just need to start upload of a big file and reboot the machine you're connected to during upload.

Is there any way I can help to narrow it down?

[jonathanlafite]

Hi, I found where the bug comes from. This is a critical bug because it crashes node and the entire application if the resource disconnect while you're doing something on it.

It's easy to reprocude :

• connect
• breakpoint just before a getfile for e.g
• shutdown the remote resource
• release the break point
• node crashes

In the connect function you register the the error event and when the connection promise is resolved you remove the listener on the error event. await new Promise((resolve, reject) => { connection.on('error', function (err) { reject(err) }); if (config.onKeyboardInteractive) { connection.on('keyboard-interactive', config.onKeyboardInteractive); } connection.on('ready', () => { connection.removeListener('error', reject); resolve(); }); Later when an operation is performed on the ssh (getFile or mkdir or whatever) if an error appears the ssh2 lib executes the following code that produces the crash. this.emit('error', err); Because no one is listening to the "error" event.

I fixed that outside of the lib (in my code) just by adding : ssh.connection.on("error", ()=>{}) after being connected

@steelbrain you should probably add a listener to the error event before each promise and remove it when the promise is resolved

EDIT: my fix worked for exec but not for putDirectory. It loops forever..

[jackpap]

I fully agree with @jonathanlafite. The promises are not being rejected in any other methods than the connect method. This means the client code can't handle properly connection drops or other issues during operations such as putDirectory etc...

[jackpap]

@steelbrain When do you think this will be published to npm ?

[steelbrain]

Within the day, Eastern European Timezone

[steelbrain]

Published in node-ssh@12.0.3 A thanks to everyone involved in reporting and fixing this! Closing for now, will re-open if the error re-surfaces

[sowizz]

Hello everyone,

I've finally had time to update to 12.0.3 version and the error still happens for me. I was testing the same scenario as described above.

I think the reason for this is that the listener is removed in finally and there is a very good chance it might be removed before connection.sftp(…) or connection.shell(…) are done. I've added simple console.log to the requestSFTP function, like so:

async requestSFTP() {
    const connection = this.getConnection();
    return new Promise((resolve, reject) => {
        connection.on('error', reject);
        connection.on('error', () => {
            console.log('requestSFTP - error happened!')
        })
        try {
            connection.sftp((err, res) => {
                if (err) {
                    reject(err);
                }
                else {
                    resolve(res);
                }
            });
        }
        finally {
            console.log('requestSFTP - removing listener')
            connection.removeListener('error', reject);
        }
    });
}

and here is what was written to the console:

requestSFTP - removing listener
requestSFTP - error happened!

The same applies to requestShell function.

[sowizz]

@steelbrain Can you reopen this issue? The problem wasn't solved yet.

[steelbrain]

I've published a beta version, you may install it with npm install node-ssh@beta.

Please try it out and let me know. Thanks!

[sowizz]

Installed and tested. It is working now. Thank you.

[steelbrain]

Published beta as v12.0.4 stable