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

SFTP Missing finish event #1184

Open
theophilusx opened this issue May 24, 2022 · 10 comments
Open

SFTP Missing finish event #1184

theophilusx opened this issue May 24, 2022 · 10 comments

Comments

@theophilusx
Copy link

It looks like SFTP write streams created with sftp.createWriteStream() may not be emitting a finish event. For example, with the following code

        const ws = sftp.createWriteStream(`${config.sftpUrl}/stream-t3.txt`);
        const rs = fs.createReadStream(`${config.localUrl}/test-file1.txt`);
        ws.on('error', (err) => {
          reject(err);
        });
        ws.on('finish', () => {
          console.log('ws finish event fired');
          resolve('Data streamed to remote file');
        });
        ws.on('close', () => {
          console.log('ws close event fired');
        })
        rs.pipe(ws);

only a 'close' event is triggered. If we go the reverse direction, with a fs.createWriteStream() and an sftp.creatgeReadStream(), we get first a finish event and then a close event.

This is problematic if we want to use a write stream with autoClose: false as it means no event is emitted and we cannot tell when (for example) a pipe operation ahs completed (unless we watch for events on the reader of course, but that could cause issues with knowing when all buffered data has been flushed to the write stream etc).

This was observed using ssh2 v1.10.0 on Linux running Node 18.1.0.

@theophilusx
Copy link
Author

@mscdex Are you able to confirm this is an issue or am I missing something or need to provide more details?

@theophilusx
Copy link
Author

@mscdex I would like to push out a new version of ssh2-sftp-client since you have released 1.11.0, but this question regarding no finish signal when closing a write stream has impact on some of the functionality in my module. Can you provide some feedback on your position with respect to this i.e. yes, it needs to be fixed, no, cannot reproduce, maybe, not sure best action if any, more information needed etc?

@mscdex
Copy link
Owner

mscdex commented Jun 14, 2022

I haven't had time to look at it. Node's implementation seems to change fairly often so it's difficult keeping up with it.

@theophilusx
Copy link
Author

theophilusx commented Jun 14, 2022 via email

@anzerr
Copy link

anzerr commented Dec 21, 2022

I've had the same problem it appeared when I updated from node version v16.5.0 to v18.12.1

We use it like this. Both the events "close" and "finish" don't fire.

const rstream = this.client.createReadStream(oldPath, {flags: 'r', autoClose: true});
const wstream = this.client.createWriteStream(newPath, {flags: 'w', autoClose: true});
rstream.pipe(wstream);
wstream.on('end', () => {
    console.log('end was emited');
});
wstream.on('finish', () => {
    console.log('finish was emited');
});

It does seem like a change in node and now that v18 is the LTS it'll probably affect more users.

@cormacbeagan
Copy link

Definately seeing this issue as soon as I updated to node 18.x - can use the on 'close' handler but neither the 'end' or 'finish' handlers are being called.
@anzerr that is 'end' and 'finish' are you sure you are having issues with the 'close' handler?
I am just paranoid that 'close' is not going to be reliable 😅

@SudhirkumarRamani
Copy link

Hello @anzerr and @cormacbeagan,

I am facing same issue while upgrading my code from NodeJS 16.x to NodeJS 18.x. Could you please suggest how did you fixed or applied workaround on this issue?

Your help will be much appreciated.

@ed-whittle
Copy link

@SudhirkumarRamani We are reading from a file in the local storage of the container and writing to an SFTP Server. Initially we were using a Node.JS 14 AzDo pipeline, and after changing to Node 18 the finish event was no longer called. The close event now resolves the promise of the function

const readStream = fs.createReadStream(`./source_file/${fileName}`);
const writeStream = sftp.createWriteStream(remoteFileName);

readStream.pipe(writeStream);
writeStream.on('error', (err) => {
  console.error(err);
  return reject(err);
});
writeStream.on('close', () => {
  console.log('All writes complete');
  return resolve(remoteFileName.split('/')[3]);
});
// Previous Implementation pre version 18, no longer working
writeStream.on('finish', () => {
  console.log('All writes complete');
  return resolve(remoteFileName.split('/')[3]);
});

@TsvetoslavVasev
Copy link

TsvetoslavVasev commented Jan 19, 2024

Hey, any update on this, we face the exact same issue after migrating from Node 16 to Node 18, the problem is present on Node LTS as well

@FranckFreiburger
Copy link

FranckFreiburger commented Feb 8, 2024

i have this problem in Node 20 as well

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

8 participants