sanketbajoria / ssh2-promise

ssh with promise/async await and typescript support
https://www.npmjs.com/package/ssh2-promise
MIT License
148 stars 24 forks source link

Spawn rejects on stderr #65

Closed damiankoper closed 3 years ago

damiankoper commented 3 years ago

I wonder why change in commit https://github.com/sanketbajoria/ssh2-promise/commit/2fc2ce2715d393ad6d891434d1d9b86e5cac9091 was made to src/sshConnection.ts file. My command writes to stderr within the first 500ms, so the whole spawned process fails, which should not happen.

I think that lines:

.stderr.on('data', function (data) {
    reject(data.toString());
});
setTimeout(() => {
    resolve(stream);
}, 500);

should be reversed and eventual hopping error (as the commit name suggests) should be handled differently. What do you think?

sanketbajoria commented 3 years ago

@damiankoper hmm.. actually it was done so that, when we do connection hopping, we can propagate error properly. image

But, it make sense, if we handle connection hopping logic separately, and i should resolve stream as soon as possible, so now, it will caller responsibility to do error handling from resolved stream object.
Will do the changes by today

sanketbajoria commented 3 years ago

@damiankoper please refer to this commit https://github.com/sanketbajoria/ssh2-promise/commit/b8aebe5cf5abbe4101bee674214041aef46eac9c

Can you please confirm, if it fixes your problem.

damiankoper commented 3 years ago

Thank you :heart:. My case is fixed. Close this issue after making a release, please.

sanketbajoria commented 3 years ago

@damiankoper ssh2-promise@1.0.1 has been published.