theophilusx / ssh2-sftp-client

a client for SSH2 SFTP
Apache License 2.0
828 stars 200 forks source link

Seeing unhandled error and nodejs crash #562

Open devgk882 opened 4 days ago

devgk882 commented 4 days ago

Hi, I am using the latest ssh2-sftp-client library

And this is what I am trying to do

const Client = require('ssh2-sftp-client');

function connectServer(config) {
    return new Promise((resolve, reject) => {
        const sftp = new Client();
        sftp.on('error', (err) => {
            console.error('SFTP connection error:', err.message);
            reject(err);
        });

        sftp.connect(
            config
        )
        .then(() => resolve(sftp))
        .catch((error) => reject(error));
    });
}

// Usage Example

    const config = {
        host: 'example.com',
        port: 22,
        username: 'yourUsername',
        password: 'yourPassword',
    };

    try {
        const sftpConnection = await connectServer(config);
        console.log('Connected successfully');

        // Perform operations with the SFTP connection
       await sftpConnection.get('/remote/path', '/local/path');

        // Close the connection
        await sftpConnection.end();
        console.log('Connection closed');
    } catch (error) {
        console.error('Error during SFTP connection:', error.message);
    }

I got uncaught error handler err with errorreadtimeout with CONNRESET message no response from server i saw the log SFTP connection error: which is from the .onevent listener and Error during SFTP connection: from catch part and it crashed the nodejs application itself not sure what I should do here

And also before the code was like this:

const Client = require('ssh2-sftp-client');

function connectServer(config) {
    return new Promise((resolve, reject) => {
        const sftp = new Client();

        sftp.connect(
            config
        )
        .then(() => resolve(sftp))
        .catch((error) => reject(error));
    });
}

// Usage Example

    const config = {
        host: 'example.com',
        port: 22,
        username: 'yourUsername',
        password: 'yourPassword',
    };

    try {
        const sftpConnection = await connectServer(config);
        console.log('Connected successfully');

        // Perform operations with the SFTP connection
       await sftpConnection.get('/remote/path', '/local/path');

        // Close the connection
        await sftpConnection.end();
        console.log('Connection closed');
    } catch (error) {
        console.error('Error during SFTP connection:', error.message);
    }

and the ssh2-sftp-client version 9.x.x and during the connection reset i didnt saw crash and it just logged properly the error

theophilusx commented 4 days ago

Get rid of the promise and just use the API directly. There is no need to use the promise or add an error listener, it is all taken care of within the library itself. Just wrap the calls to the library within a try/catch and call await sftp.connect() and await sftp.get() or await sftp.put() or whatever you need and then await sftp.end(). Also look at the README for the lib as this outlines how error events are handled and what you need to do if the default setup does not meet your requirements and the limitations wrt handling errors. Main thing is get rid of the call to new Promise as it is unnecessary and is just causing unnecesary additional complexity.

FYI the reason you have and uncaught exception is because your error handler runs outside the try/catch block. This is one of the major challenges associated with wanting to wrap and event based API inside an async promise based API. There use to be a section about the problems with usinmg try/catch and event emitters in the node documentation - not sure if it is still there as I've not checked the most recent docs.

devgk882 commented 2 days ago

Thanks for the help