Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssh2-sftp-client crashes Node process with no error and exit code 0 #168

Closed
LEQADA opened this issue Sep 20, 2019 · 11 comments
Closed

ssh2-sftp-client crashes Node process with no error and exit code 0 #168

LEQADA opened this issue Sep 20, 2019 · 11 comments
Labels
bug Bug in module needing fix client Error Incorrect API use or other client code error test Fix applied - needs testing

Comments

@LEQADA
Copy link

LEQADA commented Sep 20, 2019

I have an issue which looks exactly like this one #49

I'm using:

  • Node.js 12
  • ssh2-sftp-client 4.1.0

Here is a minimal reproducible code

const Client = require('ssh2-sftp-client')
const inspect = require('util').inspect

const sftpClient = new Client();

sftpClient.on('error', (err) => {
  console.log('SFTP client emitted ERROR: ' + inspect(err))
})
sftpClient.on('end', () => {
  console.log('SFTP client emitted END')
})
sftpClient.on('finish', () => {
  console.log('SFTP client emitted FINISH')
})
sftpClient.on('close', () => {
  console.log('SFTP client emitted CLOSE')
})

const options = {
  host: '',
  port: '22',
  username: '',
  password: ''
};

// below infinite loop is to simulate large number of 
// connections in my production code
(async () => {
  while (true) {
    try {
      await new Promise(resolve => setTimeout(() => resolve(sftpClient.connect(options)), 1000))
    } catch {
      console.log('boom')
    }
  }
})()

process.on('exit', code => console.log(`Catched!. Exit code: ${code}`))

process.on('uncaughtException', (err, origin) => {
  console.error('Unhandled exception. Please handle!', err.stack || err)
  console.error(`Origin: ${JSON.stringify(origin)}`)
})

process.on('unhandledRejection', (err, promise) => {
  console.error('Unhandled promise rejection. Please handle!', promise, err.stack || err)
})

process.on('warning', (warning) => {
  console.warn(warning.name)
  console.warn(warning.message)
  console.warn(warning.stack)
})

process.on('rejectionHandled', (promise) => {
  console.log('rejectionHandled event triggered')
})

As you can see, it shouldn't escape the infinite loop unless there is an Error. And it in fact does not for some iterations but eventually it escapes (usually after <10 iterations). And the only log I see is

SFTP client emitted END
Catched!. Exit code: 0

On the SFTP server side I have the following limitations in the sshd_config file

MaxSessions 1
MaxStartups 1

I don't know how is it possible to exit with code 0 without any error trace without actually calling process.exit(0). This makes investigation even harder, because I don't know what to look for.

@theophilusx theophilusx added the bug Bug in module needing fix label Sep 21, 2019
@theophilusx
Copy link
Owner

I'll need to look at this closer. However, I do see a problem with your code. You are using the same client object to try and make all the connections. You can't do that. You need to create a new Client() object for each connection or you need to end each connection before trying to open another one.

@theophilusx theophilusx added client Error Incorrect API use or other client code error and removed bug Bug in module needing fix labels Sep 21, 2019
@theophilusx
Copy link
Owner

Not 100% sure, but I think your code is faulty. Apart from the mentioned issue of trying to do multiple connections with the same client concurrently, I suspect that errors are not able to bubble up from the connect promise due to how you are calling them. It looks to me like you are resolving the promise you wrap the call to connect with immediately, so there is nothing waiting on the connect promise itself.

With the below code, errors are created when the remote server starts rejecting the connections. If I don't limit the while loop, I end up just getting memory and gc errors from node (which would be expected).

There is one minor issue though. The close listener should be getting a boolean passed to it which states whether the connection was closed due to errors or just after a normal request. For some reason, it is getting undefined, so I need to look at that.

'use strict';

const dotenvPath = __dirname + '/../.env';

require('dotenv').config({path: dotenvPath});

const Client = require('../src/index');
const inspect = require('util').inspect;

const config = {
  host: process.env.SFTP_SERVER,
  username: process.env.SFTP_USER,
  password: process.env.SFTP_PASSWORD,
  port: process.env.SFTP_PORT || 22
};

let clients = [];
let promises = [];
let count = 0;

function makeConnection() {
  count++;
  let c = new Client();
  c.on('error', err => {
    console.log('SFTP client emitted ERROR: ' + inspect(err));
  });
  c.on('end', () => {
    console.log('SFTP client emitted END');
  });
  c.on('finish', () => {
    console.log('SFTP client emitted FINISH');
  });
  c.on('close', hadError => {
    console.log(`SFTP client emitted CLOSE: Had Error: ${hadError}`);
  });
  clients.push(c);
  promises.push(c.connect(config));
  console.log(`Connection ${count} requested`);
}

process.on('exit', code => console.log(`Catched!. Exit code: ${code}`));

process.on('uncaughtException', (err, origin) => {
  console.error('Unhandled exception. Please handle!', err.stack || err);
  console.error(`Origin: ${JSON.stringify(origin)}`);
});

process.on('unhandledRejection', (err, promise) => {
  console.error(
    'Unhandled promise rejection. Please handle!',
    promise,
    err.stack || err
  );
});

process.on('warning', warning => {
  console.warn(warning.name);
  console.warn(warning.message);
  console.warn(warning.stack);
});

process.on('rejectionHandled', () => {
  console.log('rejectionHandled event triggered');
});

let conCount = 0;
let maxConn = 100;

while (conCount < maxConn) {
  try {
    conCount++;
    setTimeout(() => makeConnection(), 1000);
  } catch (err) {
    console.log(`Boom: ${err.message}`);
  }
}

@theophilusx theophilusx added the bug Bug in module needing fix label Sep 21, 2019
@theophilusx
Copy link
Owner

There is something very odd going on, but tracking it down is difficult. I'm fairly certain it is something within the SSH2 module and it is at the networking layer. Something is causing node to exit early. The connections appear to be successful and you can even query them. However, the script just seems to exit (without any errors) before the final then block in the promise chain is completed. However, when I first run the test script, it works fine. If I then immediately run it a second time, it exits early. If I wait (until all TIME_WAIT tcp connections have expired - usually 2 minutes on most Linux systems) it runs fine.

I have made some changes to the code in the fix-error-propogation branch of the repository. I have added the ability to set a client name to be associated with each client to make it easier to see which client is throwing errors etc e.g. you can do

let client = new Client('some-name');

I have also added a check which will generate an error if you try to connect using a client which is already connected.

The test script I'm using is also in the examples directory of the repository. It is called loadup2.js and I have copied it below.

'use strict';

const dotenvPath = __dirname + '/../.env';

require('dotenv').config({path: dotenvPath});

const Client = require('../src/index');

const config = {
  host: process.env.SFTP_SERVER,
  username: process.env.SFTP_USER,
  password: process.env.SFTP_PASSWORD,
  port: process.env.SFTP_PORT || 22
};

let maxConnections = 15;
let clientCount = 0;
let clients = [];
let promises = [];

function makeConnection() {
  clientCount++;
  let c = new Client(`Client-${clientCount}`);
  c.on('error', err => {
    console.log(`${c.clientName}: Error - ${err.message}`);
  });
  clients.push(c);
  promises.push(
    c
      .connect(config)
      .then(() => {
        console.log(`${c.clientName}: Connected`);
        return c.cwd();
      })
      .then(d => {
        console.log(`${c.clientName}: ${d}`);
        return c.end();
      })
      .then(() => {
        console.log(`${c.clientName}: Connection closed`);
        return true;
      })
  );
  console.log(`Connection ${clientCount} requested`);
}

while (clientCount < maxConnections) {
  makeConnection();
}

Promise.all(promises)
  .then(vals => {
    console.log('Script Completed');
    console.dir(vals);
  })
  .catch(err => {
    console.log(`First Error: ${err.message}`);
  });

I have done a lot of checking and am fairly confident that the ssh2-sftp-module is handling errors when they are generated correctly. The problem seems to be that whatever is causing node to exit is not thorwing/rasiing any error or exception, so there is nothing to catch and respond to.

Next Steps

  1. Write a script which only uses ssh2 and sshsftp-streams to see if a similar problem occurs. If it oes, we can then raise an issue against those modules.

  2. Test with different node versions to see if the same issue exists for all versions.

@theophilusx
Copy link
Owner

Now have a work-around for this issue which is currently being tested.

@theophilusx
Copy link
Owner

Work-around for end() event appears to fix the issue and all tests pass.

@lojzatran
Copy link

@theophilusx can you share your workaround for this issue? I'm also experiencing it and it's quite critical for us. Thank you.

@theophilusx
Copy link
Owner

theophilusx commented Sep 23, 2019 via email

@KshamaShrey
Copy link

Hi Team,

Even I am facing this issue.Could you please let me know how you have fixed this issue? Thanks

@anurag-shj
Copy link

@LEQADA @KshamaShrey did you guys find any fix for this issue? I'm facing the same issue.

@LEQADA
Copy link
Author

LEQADA commented Jun 25, 2024

@LEQADA @KshamaShrey did you guys find any fix for this issue? I'm facing the same issue.

It was a long time ago and I don't remember how I resolved it.

@anurag-shj
Copy link

anurag-shj commented Jun 25, 2024

@LEQADA @lojzatran @theophilusx @KshamaShrey I need to read a file from an SFTP server. Whenever I try to get the file using either the ssh2-sftp-client or ssh2 library, my Express server restarts. This issue does not occur when I run the code locally, but it causes the microservice to restart on my Nginx server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in module needing fix client Error Incorrect API use or other client code error test Fix applied - needs testing
Projects
None yet
Development

No branches or pull requests

5 participants