-
Notifications
You must be signed in to change notification settings - Fork 664
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
Comments
@mscdex Are you able to confirm this is an issue or am I missing something or need to provide more details? |
@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? |
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. |
OK, thanks for the feedback.
Note that creating write streams using node streams and fs modules seem
to behave correctly i.e. a finish event is raised before the close
event. Only the ssh2 sftp write stream seems not to raise the finish
event before the close event.
I have made some changes to my module so that things should work
correctly. The one area of concern is that I've now added the ability
for users to get a write stream using sftp.createWriteStream. Should
they set autoClose to false on one of these streams, they will likely
run into issues as they won't get any signal when for exxample, a pipe()
finishes (the 'close' event is not raised if autoClose is false). Means
they will have to rely on the finish signal from the reader, but that
doesn't take account for possible data buffering etc (i.e. reader might
be finished, but writer may not).
|
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 " 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. |
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. |
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. |
@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
|
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 |
i have this problem in Node 20 as well |
It looks like SFTP write streams created with sftp.createWriteStream() may not be emitting a finish event. For example, with the following code
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.
The text was updated successfully, but these errors were encountered: