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

Missing callback when SFTP server disconnects during sftp operation #1175

Open
jeffrson opened this issue May 2, 2022 · 4 comments
Open

Comments

@jeffrson
Copy link

jeffrson commented May 2, 2022

I have a problem with, maybe, cleanupRequests. Unfortunately I cannot get hold on how it works in the end.

Sometimes, mostly for network reasons, or because it reboots for maintenance, an SFTP server, based on SSH2, "disappears". It could be simulated by

session.on('sftp', (accept, _reject) => {
  const sftpStream = accept()
 ...
  sftpStream.on('READDIR', async (reqId, aPath) => {
     process.exit(0)
...

A client uses ssh2-promises and sometimes the promises are not rejected by disconnection, but stay pending. I tried to debug the situation and I found, that in

function cleanupRequests(sftp) {
  let keys = Object.keys(sftp._requests);
  if (keys.length === 0)
    return;

  let reqs = sftp._requests;
  sftp._requests = {};
  const err = new Error('No response from server');
  for (let i = 0; i < keys.length; ++i) {
    const req = reqs[keys[i]];
    if (typeof req.cb === 'function')
      req.cb(err);
  }
}

the callback I need is added to sftp._requests while another request is processed through req.cb(err);. WIth other words, if cleanupRequests would be executed twice the callback is called correctly and the promises are rejected.

This may sound strange - any idea why this could happen? May this be a problem in ssh2 or ssh2-promise? I couln't reproduce it so far in simple app with ssh2 alone.

@jeffrson
Copy link
Author

jeffrson commented May 2, 2022

Might be because sock.on('close') is calling this._chanMgr.cleanup(err) which propagates until cleanupRequests, which then calls back into this.readdir(handle, opts, (err, list) => {} where it calls this.close(), adding another request.

@jeffrson
Copy link
Author

jeffrson commented May 3, 2022

Here is a simple repro: https://github.com/jeffrson/ssh2-missing-cb

When the server crashs in "READDIR" the callback is missing - however, if the "crash" occurs in OPENDIR the callback is called correctly.

I seem to remember similar problems some time ago during delete as well (yes, in development the server crashed to often ;-)).

@jkovalchik
Copy link

@jeffrson any luck with this? Find any workarounds? It seems like I may also be experiencing this problem based on my stacktrace.

Error: No response from server
at cleanupRequests (/home/appuser/project/node_modules/ssh2/lib/protocol/SFTP.js:2580:15)
at SFTP.push (/home/appuser/project/node_modules/ssh2/lib/protocol/SFTP.js:191:7)
at onCHANNEL_CLOSE (/home/appuser/project/node_modules/ssh2/lib/utils.js:50:13)
at ChannelManager.cleanup (/home/appuser/project/node_modules/ssh2/lib/utils.js:200:7)
at Socket.<anonymous> (/home/appuser/project/node_modules/ssh2/lib/client.js:769:21)
at Socket.emit (events.js:314:20)
at TCP.<anonymous> (net.js:675:12)

I'm running the latest ssh2 at the time of writing this (1.11.0).

@bijoythomas
Copy link

I'm seeing the same error as well on v1.10.0, but in my case it happens when I'm closing the connection to the SFTP server after retrieving a file from it. I'm doing something like below

let outStream = fs.createWriteStream('localFileName')
let inStream = sftp.createReadStream('remoteFileName')

inStream.pipe(outStream)
outStream.on('close', function(err) {
  if (err) {
    // log the error
  }
  connection.end()
}

I've gotten around this error by setting the {autoClose: false} option on the input stream like:
let inStream = sftp.createReadStream('remoteFileName', {autoClose: false})

which I assume prevents the callback from being queued.

It's not ideal, but in my case the connections are created on demand and are not pooled or kept around. I believe closing the connection will also result in any open file handles being freed up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants