theophilusx / ssh2-sftp-client

a client for SSH2 SFTP
Apache License 2.0
798 stars 195 forks source link

It doesnt work with node v18 #477

Closed huseyinozgul closed 1 month ago

huseyinozgul commented 1 year ago

We have used older version (5.x) with node v16, it works properly but the latest version 9.1.0 doesn't work on node v18 What do you suggest? CLIENT[sftp]: ssh2-sftp-client Version: 9.0.4 { "node": "18.15.0", "v8": "10.2.154.26-node.25", "uv": "1.44.2", "zlib": "1.2.13", "brotli": "1.0.9", "ares": "1.18.1", "modules": "108", "nghttp2": "1.51.0", "napi": "8", "llhttp": "6.0.10", "uvwasi": "0.0.15", "acorn": "8.8.2", "simdutf": "3.1.0", "undici": "5.20.0", "openssl": "3.0.8+quic", "cldr": "42.0", "icu": "72.1", "tz": "2022g", "unicode": "15.0", "ngtcp2": "0.8.1", "nghttp3": "0.7.0" } CLIENT[sftp]: connect: Connect attempt 1 Custom crypto binding not available Local ident: 'SSH-2.0-ssh2js1.13.0' Client: Trying staging-ftp.xxxx.com on port 22 ... Socket connected CLIENT[sftp]: connect errorListener - ignoring handled error CLIENT[sftp]: Global error event: Ignoring expected and handled event CLIENT[sftp]: connect errorListener - ignoring handled error CLIENT[sftp]: Global error event: Ignoring expected and handled event Socket ended CLIENT[sftp]: connect errorListener - ignoring handled error CLIENT[sftp]: Global error event: Ignoring expected and handled event CLIENT[sftp]: connect endListener - ignoring handled error CLIENT[sftp]: Global end event: Ignoring expected and handled event Socket closed CLIENT[sftp]: connect closeListener - ignoring handled error CLIENT[sftp]: Global close event: Ignoring expected and handled event CLIENT[sftp]: connect: Connect attempt 2 Custom crypto binding not available Local ident: 'SSH-2.0-ssh2js1.13.0' Client: Trying staging-ftp.dsco.io on port 22 ... Socket connected CLIENT[sftp]: connect errorListener - ignoring handled error CLIENT[sftp]: Global error event: Ignoring expected and handled event CLIENT[sftp]: end: Called when no connection active CLIENT[sftp]: connect: getConnection: Invalid identification string (ERR_GENERIC_CLIENT) CLIENT[sftp]: end: Called when no connection active

theophilusx commented 1 year ago

There is nowhere enough context here for me to suggest anything.

What I can tell you is that I use it with both node 16.19.1, 18.16.0, 19.9.0 and 20.0.0 with no issues.

Suggest you provide a minimal example showing what is failing to provide more context.

huseyinozgul commented 1 year ago

`import SftpClient from 'ssh2-sftp-client'

export const uploadSftp = async ( config, data ) => { const sftp = new SftpClient() try { const sftpConfig = { host: config.host, port: 22, username: config.username, password: config.password, debug: console.log } await sftp.connect(sftpConfig) await sftp.put(data, config.output) } catch (err) { throw new Error('SFTP : ' + JSON.stringify(err)) } finally { await sftp.end() } }`

theophilusx commented 1 year ago

Can you also provide

  1. Details on the actual error you are seeing
  2. A (possibly sanatised) debug log (using the debug property of the connection properties
  3. What platform you are running on (Linux, Windows, macOS, etc).
huseyinozgul commented 1 year ago

This problem occurs on MacOS Ventura or AWS Lambda functions (based on Node v18.12). We also tried node v18.16. However, it throws the same error. If we use ver 9.1.0, it doesn't connect. If we use ver 5.1.3, it connects and uploads but the process hangs like the attached picture. image (2)

theophilusx commented 1 year ago

Ignore v5.1.3. That version is very old and no longer supported.

With v9.1.0, when you say it doesn't connect, do you get any error? Can you create a debug log? Can you also confirm that the sample code you sent actually generates the issue you are seeing (sorry to be so insistent, but I have had multiple cases where people have provided sample code which is not the code they are using to generate the error and after I spend hours tying and failing to reproduce the error, they then tell me "Sorry, our code is slightly different and does XXX". I just want to confirm that you have actually run that code you sent and it fails in both macOS and AWS.

FYI I am not seeing the issues you are seeing with very similar code (i.e. code which connects and uploads data from a buffer) in any node version. The only big difference is I always use commonJS module syntax with node, not ES module syntax. Have you tried with require instead of import? Given that this module has a lot of weekly downloads and your the only one reporeting this issue, I suspect this is something specific to your environment or code.

On Fri, 26 May 2023 at 17:26, Huseyin Ozgul @.***> wrote:

This problem occurs on MacOS Ventura or AWS Lambda functions (based on Node v18.12). We also tried node v18.16. However, it throws the same error. If we use ver 9.1.0, it doesn't connect. If we use ver 5.1.3, it connects and uploads but the process hangs like the attached picture. [image: image (2)] https://user-images.githubusercontent.com/4598366/241148853-51a177ef-ecf1-4e5d-a791-0ae7157d6b65.png

— Reply to this email directly, view it on GitHub https://github.com/theophilusx/ssh2-sftp-client/issues/477#issuecomment-1563928880, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMFIKI6IXPNSOTYVYLXDPLXIBLKLANCNFSM6AAAAAAYOSHZDY . You are receiving this because you commented.Message ID: @.***>

-- regards,

Tim

-- Tim Cross

huseyinozgul commented 1 year ago

This is the log.

CLIENT[sftp]: connect: Debugging turned on CLIENT[sftp]: ssh2-sftp-client Version: 9.0.4 { "node": "18.16.0", "acorn": "8.8.2", "ada": "1.0.4", "ares": "1.19.0", "brotli": "1.0.9", "cldr": "42.0", "icu": "72.1", "llhttp": "6.0.10", "modules": "108", "napi": "8", "nghttp2": "1.52.0", "nghttp3": "0.7.0", "ngtcp2": "0.8.1", "openssl": "3.0.8+quic", "simdutf": "3.2.2", "tz": "2022g", "undici": "5.21.0", "unicode": "15.0", "uv": "1.44.2", "uvwasi": "0.0.15", "v8": "10.2.154.26-node.26", "zlib": "1.2.13" } CLIENT[sftp]: connect: Connect attempt 1 Custom crypto binding available Local ident: 'SSH-2.0-ssh2js1.13.0' Client: Trying staging-ftp.xxxx.io on port 22 ... Socket connected CLIENT[sftp]: connect errorListener - ignoring handled error CLIENT[sftp]: Global error event: Ignoring expected and handled event Socket ended CLIENT[sftp]: connect errorListener - ignoring handled error CLIENT[sftp]: Global error event: Ignoring expected and handled event CLIENT[sftp]: connect endListener - ignoring handled error CLIENT[sftp]: Global end event: Ignoring expected and handled event Socket closed CLIENT[sftp]: connect closeListener - ignoring handled error CLIENT[sftp]: Global close event: Ignoring expected and handled event CLIENT[sftp]: connect: Connect attempt 2 Custom crypto binding available Local ident: 'SSH-2.0-ssh2js1.13.0' Client: Trying staging-ftp.xxxx.io on port 22 ... Socket connected CLIENT[sftp]: connect errorListener - ignoring handled error CLIENT[sftp]: Global error event: Ignoring expected and handled event CLIENT[sftp]: end: Called when no connection active CLIENT[sftp]: connect: getConnection: Invalid identification string (ERR_GENERIC_CLIENT) CLIENT[sftp]: end: Called when no connection active err Error: SFTP : {"code":"ERR_GENERIC_CLIENT","custom":true} at uploadSftp (file:///Users/huseyinozgul/projects/xxxx/test/sftp.mjs:30:11) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) at async uploadInventoryFile (file:///Users/huseyinozgul/projects/xxxx/test/uploadInventoryFile.mjs:11:3) at async uploadInventory (file:///Users/huseyinozgul/projects/xxxx/test/uploadInventory.mjs:52:3) Socket ended CLIENT[sftp]: Global error event: Handling unexpected event CLIENT[sftp]: Global end event: Handling unexpected event Socket closed CLIENT[sftp]: Global close event: Handling unexpected event

huseyinozgul commented 1 year ago

Do we need any shh config?

theophilusx commented 1 year ago

The log indicates the remote sftp server is sending an invalid identifier string. Are you able to upload data using openssh to the same server just as confirmation it isn't a server issue?

On Fri, 26 May 2023 at 18:18, Huseyin Ozgul @.***> wrote:

Do we need any shh config?

— Reply to this email directly, view it on GitHub https://github.com/theophilusx/ssh2-sftp-client/issues/477#issuecomment-1563996633, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMFIKLRBDUGSTIRMEBEMFDXIBRNNANCNFSM6AAAAAAYOSHZDY . You are receiving this because you commented.Message ID: @.***>

-- regards,

Tim

-- Tim Cross

huseyinozgul commented 1 year ago

I tried with sftp, it connects and puts the file.

mcheale-parkit commented 1 year ago

I'm not convinced this is a node 18 issue but the change in behaviour around unexpected ends. I believe an unexpected end during connect no longer throws so reject is never called and because the connection was never ready is never resolves either.

For example:

async function main() {
  const configuration = getConfiguration();

  const connectionOptions: Client.ConnectOptions = {
    host,
    port: 22,
    username: username,
    password: decrypt(configuration, password) as string,
    // debug: console.log
  }

  try {
    console.log('=== 001 ===');
    const ftpClient = new Client();
    const sftp = await ftpClient.connect(connectionOptions);
    console.log('=== 002 ===');

    sftp.fastPut('./test', './upload-test', (e) => {
      console.log('=== 003 ===');
      sftp.end();
      console.log('=== 004 ===');
    });
  } catch(e) {
    console.log('=== 006 ===');
  }
  console.log('=== 007 ===');
}

main()
  .then(() => console.log('YAY'))
  .catch((e) => console.log('BOO', e))
  .finally(() => console.log('The end'));

Only thing that was logged is === 001 ===.

theophilusx commented 1 year ago

I'm still not sure there is even an issue here. I cannot reproduce the behaviour (though it still isn't clear exactly what the issue is). I use this module with all currently supported versions of node with no issue. I will also draw attention to the provided log file which shows the remote sftp server is returning an invalid identifier.

As to the sample script i.e.

async function main() { const configuration = getConfiguration();

const connectionOptions: Client.ConnectOptions = { host, port: 22, username: username, password: decrypt(configuration, password) as string, // debug: console.log }

async function main() { const configuration = getConfiguration();

const connectionOptions: Client.ConnectOptions = { host, port: 22, username: username, password: decrypt(configuration, password) as string, // debug: console.log }

try { console.log('=== 001 ==='); const ftpClient = new Client(); const sftp = await ftpClient.connect(connectionOptions); console.log('=== 002 ===');

sftp.fastPut('./test', './upload-test', (e) => {
  console.log('=== 003 ===');
  sftp.end();
  console.log('=== 004 ===');
});

} catch(e) { console.log('=== 006 ==='); } console.log('=== 007 ==='); }

main() .then(() => console.log('YAY')) .catch((e) => console.log('BOO', e)) .finally(() => console.log('The end')); try { console.log('=== 001 ==='); const ftpClient = new Client(); const sftp = await ftpClient.connect(connectionOptions); console.log('=== 002 ===');

sftp.fastPut('./test', './upload-test', (e) => {
  console.log('=== 003 ===');
  sftp.end();
  console.log('=== 004 ===');
});

} catch(e) { console.log('=== 006 ==='); } console.log('=== 007 ==='); }

main() .then(() => console.log('YAY')) .catch((e) => console.log('BOO', e)) .finally(() => console.log('The end'));

it has two bugs I can see from just a quick read.

  1. There is no 3rd callback parameter to fastPut()

  2. fastPut() returns a promise. The script has no await, so the script will exit before the promise has been fulfilled.

  3. The call to fastPut() should be

    let rslt = ftpClient.fastPut(src, dst)

i.e. should use the object returnd by new, not the internal sftp object returned from connect (see the example directory for sample scripts).

mcheale-parkit commented 1 year ago

That's all one bug, in that it's using the underlying connection, agreed, I was pulling it out of someone else's implementation as a simplified test case. Even with that change I observed two behaviours:

  1. Give it valid credentials and the script would output the YAY and The end.
  2. Give it invalid credentials and the script would not output BOO and The end.

I don't think the symptoms are anything to do with node but the change made in how it handles the underlying ssh2 connection emitting end on an authentication failure. In this scenario getConnection neither resolves or rejects.

theophilusx commented 1 year ago

No, I disagree. As previously reported, I cannot reproduce this issue. For example, using this script

const sftpClient = require("../src/index");

const host = process.argv[2]; const port = Number.parseInt(process.argv[3]); const username = process.argv[4]; const password = process.argv[5]; const src = process.argv[6]; const dst = process.argv[7];

const config = { host, port, username, password, };

console.log("=== 001 start ===");

async function main() { const client = new sftpClient();

try { console.log("=== 002 after new ==="); await client.connect(config); console.log("=== 003 after connect ==="); await client.fastPut(src, dst); console.log("=== 004 after fastput ==="); } catch (e) { console.log(e.message); } finally { await client.end(); } console.log("=== 005 end ==="); }

main() .catch((e) => e.message());

and running it under node 18.16.1, I get the following results

  1. With all good credentials

=== 001 start === === 002 after new === === 003 after connect === === 004 after fastput === === 005 end ===

  1. with bad hostname

=== 001 start === === 002 after new === connect: Address lookup failed for host === 005 end ===

  1. with bad port number

=== 001 start === === 002 after new === connect: getConnection: connect EHOSTUNREACH 192.168.122.87:2233 === 005 end ===

  1. with bad username

=== 001 start === === 002 after new === connect: getConnection: All configured authentication methods failed === 005 end ===

  1. with bad password

=== 001 start === === 002 after new === connect: getConnection: All configured authentication methods failed === 005 end ===

  1. with bad source path

=== 001 start === === 002 after new === === 003 after connect === fastPut: Bad path: ./bogus: not exist === 005 end ===

  1. with bad destination path

=== 001 start === === 002 after new === === 003 after connect === fastPut: No such file Local: ./fastPut.js Remote: /not/here/fred === 005 end ===

So I think this demonstrates your analysis is flawed. Using bad host address, port number, username, password, source path or destination path all result in an error being reported and the script terminating as would be expected. The only minor issue I can see here is that the last error message for bad destination path needs improvement as it doesn't make it clear enough the problem was with the destination path.

As previously stated. I cannot reproduce this issue. I have not problems using this module with node v18 and have successfully tested this module with the following node versions

node/14.21.3
node/16.19.1
node/18.16.1
node/19.9.0
node/20.0.0

I still do not have a clear and concise description of the precise issue the OP has encountered and as previously mentioned, from the supplied debug output, it appears that the remote sftp server is returning an invalid identifier.

mcheale-parkit commented 1 year ago

This is definitely a thing but you need to use an SSH server that hangsup on auth failure. I've copied your example, enabled debugging, and put in the credentials where I see this behaviour:

package.json

{
  "name": "ssh2-sftp-client",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "ssh2-sftp-client": "^9.1.0"
  }
}

output

=== 001 start ===
=== 002 after new ===
CLIENT[sftp]: connect: Debugging turned on
CLIENT[sftp]: ssh2-sftp-client Version: 9.0.4  {
 "node": "18.16.0",
 "acorn": "8.8.2",
 "ada": "1.0.4",
 "ares": "1.19.0",
 "brotli": "1.0.9",
 "cldr": "42.0",
 "icu": "72.1",
 "llhttp": "6.0.10",
 "modules": "108",
 "napi": "8",
 "nghttp2": "1.52.0",
 "nghttp3": "0.7.0",
 "ngtcp2": "0.8.1",
 "openssl": "3.0.8+quic",
 "simdutf": "3.2.2",
 "tz": "2022g",
 "undici": "5.21.0",
 "unicode": "15.0",
 "uv": "1.44.2",
 "uvwasi": "0.0.15",
 "v8": "10.2.154.26-node.26",
 "zlib": "1.2.13"
}
CLIENT[sftp]: connect: Connect attempt 1
Custom crypto binding available
Local ident: 'SSH-2.0-ssh2js1.14.0'
Client: Trying *****.blob.core.windows.net on port 22 ...
Socket connected
Remote ident: 'SSH-2.0-AzureSSH_1.0.0'
Outbound: Sending KEXINIT
Inbound: Handshake in progress
Handshake: (local) KEX method: curve25519-sha256@libssh.org,curve25519-sha256,ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521,diffie-hellman-group-exchange-sha256,diffie-hellman-group14-sha256,diffie-hellman-group15-sha512,diffie-hellman-group16-sha512,diffie-hellman-group17-sha512,diffie-hellman-group18-sha512,ext-info-c
Handshake: (remote) KEX method: ecdh-sha2-nistp384,ecdh-sha2-nistp256,diffie-hellman-group14-sha256,diffie-hellman-group16-sha512,diffie-hellman-group-exchange-sha256,ext-info-s
Handshake: KEX algorithm: ecdh-sha2-nistp256
Handshake: (local) Host key format: ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,rsa-sha2-512,rsa-sha2-256,ssh-rsa
Handshake: (remote) Host key format: rsa-sha2-256,rsa-sha2-512,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384
Handshake: Host key format: ecdsa-sha2-nistp256
Handshake: (local) C->S cipher: aes128-gcm@openssh.com,aes256-gcm@openssh.com,aes128-ctr,aes192-ctr,chacha20-poly1305@openssh.com,aes256-ctr
Handshake: (remote) C->S cipher: aes128-gcm@openssh.com,aes256-gcm@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr
Handshake: C->S Cipher: aes128-gcm@openssh.com
Handshake: (local) S->C cipher: aes128-gcm@openssh.com,aes256-gcm@openssh.com,aes128-ctr,aes192-ctr,chacha20-poly1305@openssh.com,aes256-ctr
Handshake: (remote) S->C cipher: aes128-gcm@openssh.com,aes256-gcm@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr
Handshake: S->C cipher: aes128-gcm@openssh.com
Handshake: (local) C->S MAC: hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-sha1-etm@openssh.com,hmac-sha2-256,hmac-sha2-512,hmac-sha1
Handshake: (remote) C->S MAC: hmac-sha2-256,hmac-sha2-512,hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com
Handshake: C->S MAC: <implicit>
Handshake: (local) S->C MAC: hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-sha1-etm@openssh.com,hmac-sha2-256,hmac-sha2-512,hmac-sha1
Handshake: (remote) S->C MAC: hmac-sha2-256,hmac-sha2-512,hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com
Handshake: S->C MAC: <implicit>
Handshake: (local) C->S compression: none,zlib@openssh.com,zlib
Handshake: (remote) C->S compression: none
Handshake: C->S compression: none
Handshake: (local) S->C compression: none,zlib@openssh.com,zlib
Handshake: (remote) S->C compression: none
Handshake: S->C compression: none
Outbound: Sending KEXECDH_INIT
Received DH Reply
Host accepted by default (no verification)
Host accepted (verified)
Outbound: Sending NEWKEYS
Inbound: NEWKEYS
Verifying signature ...
Verified signature
Handshake completed
Outbound: Sending SERVICE_REQUEST (ssh-userauth)
Inbound: Received EXT_INFO
Inbound: Received SERVICE_ACCEPT (ssh-userauth)
Outbound: Sending USERAUTH_REQUEST (none)
Inbound: Received USERAUTH_FAILURE (publickey,password)
Client: none auth failed
Outbound: Sending USERAUTH_REQUEST (password)
Inbound: Received DISCONNECT (11, "SSH/SFTP are not enabled for this account. - RequestId:2ef2eb9b-e01e-001f-5043-a7820d000000 Time:2023-06-25T08:57:10.9882690Z")
Socket ended
CLIENT[sftp]: getConnection Unexpected end event - ignoring
CLIENT[sftp]: connect endListener - ignoring handled error
CLIENT[sftp]: Global end event: Handling unexpected event
Socket closed
CLIENT[sftp]: getConnection closeListener - ignoring handled error
CLIENT[sftp]: connect closeListener - ignoring handled error
CLIENT[sftp]: Global close event: Handling unexpected event

In this scenario the ssh2 library doesn't emit a ready event, nor does it throw an Error. Which results in the getConnection() execution never hitting the lines that resolve or reject. Previously, this was handled in the handler for the end event, but that has been commented out.

theophilusx commented 1 year ago

If that is the case, it is a problem.

THe previous behaviour was changed because it caused problems with sftp servers which don't just end the connection, but issue an error event after an error and then an end event followed by a close event.

If I restore the previous behaviour, then we will get issues from those sftp servers which IMO are doing the correct thing and issuing an error when there is an error and not just ending the connection.

While I can think of a couple of solutions, they all have disadvantages, primarily wrt performance, for all those servers which are doing the right thing. I'm not going to restore the previous behaviour without careful consideration. I am also not going to be in a position to do anything for at least 3 months. There will be a new bug fix release in a couple of weeks, but fixing this issue would represent significant behaviour change, so won't be pushing that out right before I am away for 3 months.

mcheale-parkit commented 1 year ago

Taking a fresh look this morning the server is sending a valid ssh DISCONNECT message with a reason code of 11 (by application). The underlying ssh2 library does not consider a DISCONNECT by application as an error. It processes that message to just close the underlying socket. To me this feels like a valid timeline of events to be supported.

Appreciate that you're away for the next three months but I'm happy to contribute towards a solution with your steer.

theophilusx commented 1 year ago

I guess the first thing to do would be to see how we can get sufficient information regarding the disconnect reason. If we simply reject on end or close, the user has no idea why. This will result in issues being logged and more support requests. This could be as easy as just reporting very clearly that the connection was closed by the remote server. However, it would be much better if we can provide more details and we need to be certain that teh close/end is due to a valid disconnect by application and not some other cause (reporting the wrong reason is probably worse than not reporting a reason).

The other issue is ensuring that when we have the events error -> close -> end that we see/process them in the right order. Part of the issues encountered before was because the end or close would be seen and the connection terminated with no meaningful information for the client before the error event was seen by node. As you can only call resolve/reject once, you need to ensure when yuou do call it, you have all the relevant information, such as error message or reason for the disconnect. Ensuring these events are processed in the right order without imposing some arbitrary wait/sleep is problematic because it has a general impact on performance and you can never know what is an appropriate time to wait/sleep.

What I'm not sure about is whether we need to add back the end/close listeners generally and somehow ensure that we respond to error events first, then close and finally end events or if we just add close/end events just for the connect stage and to handle this use case. Managing these event listeners, ensuring they are removed when no longer required and ensuring you never exceed the default max of 11 before warnings are issued can be challenging.

We also need to work within the confines of what is available in the oldest supported node version. Currently, this is node 16 (2 months left).

mcheale-parkit commented 1 year ago

From my understanding the error event handler will always be executed before the end event handler. This is the DISCONNECT handler in the ssh2 library:

https://github.com/mscdex/ssh2/blob/7410a11c282fd0f21e8e989906c699c84ca711ed/lib/client.js#L335

I think there's four code paths from the ssh2 library:

Happy path:

  1. ready emitted

Works as is.

Config parse errors:

  1. Error thrown

Works as is and error is fed back.

DISCONNECT not by application:

  1. error emitted
  2. end emitted

Works as is as the error event handler will reject the promise

DISCONNECT by application:

  1. end emitted

Not covered currently

As a DISCONNECT message can be delivered at any time, during connect, while PUTing any active Promises should be rejected at that point. Otherwise there will be unresolved Promises leaking memory. As long as the error event is processed before the end event then a reason should get out. Where I think reasons might get lost is that an end event is processed before the catch happens, which would lose the feed back to the client if the end event rejects the Promise first.

theophilusx commented 1 year ago

One of the reasons the close/end event handlers were changed to not reject/throw was because at times, the close or end event was seen and handled before the error event. This resulted in promises being rejected with no cldar reasons, leading to confusion for developers (the promise would just be rejected with no error message as the error is only in the error handler).

The core issue is that it seems you cannot rely on the order in which events are handled. If we can guarantee that error events will be handled before close or end events, then we probably don't have an issue. However, that was not the case previously.

PedroS11 commented 4 months ago

Heey, any update on this issue? I have a service that i'd like to migrate to node18 and i'm getting time outs too

theophilusx commented 4 months ago

Timeout has nothing to do with this issue. This issue is about promises not resolving because the remote server has closed the connection and has not sent any error message. It is an uncommon situation which is difficult to handle cleanly and unrelated to timeouts). Many people are using this library (and ssh2 more generally) with node v18 , v20 and v21 wqith no problems. I test against all three versions with no problems. The timeout issue is a local configuration issue.

Timeout is typically caused by one of two client configuration problems -

  1. Your client and server are negotiating to use one of the very slow key exchange or cipher algorithms which are taking too long and the client times out. Incrfeasing the timout setting for the client may fix this. Alternatively, you can use the algorithm property of the connect object to specify different key exchange and cipher algorithms to try and force the client and server to negotiate a faster algorithm. Note that htis issue has come up a few times in the ssh2 repository and many of the closed issues in that repository show how to set different algorithms etc.

  2. None of the default key exchange or cipher algorithms used by ssh2 are supported by your server, preventing either the key exchange or cipher negotiation steps completing. The best way to resolve this is to use openSSH's sftp client, adding the -vvv switch to turn up debugging and see what key exchange and cipher algorithms it negotiates and then ensuring those are added to the set of supported key exchange and cipher algorithms by using the algorithm property. Again, see the ssh2 documentation for connect and similar issues in the ssh2 repositoryh.

Make sure you are using ssh2-sftp-client v10.0.3

PedroS11 commented 4 months ago

Timeout has nothing to do with this issue. This issue is about promises not resolving because the remote server has closed the connection and has not sent any error message. It is an uncommon situation which is difficult to handle cleanly and unrelated to timeouts). Many people are using this library (and ssh2 more generally) with node v18 , v20 and v21 wqith no problems. I test against all three versions with no problems. The timeout issue is a local configuration issue.

Timeout is typically caused by one of two client configuration problems -

  1. Your client and server are negotiating to use one of the very slow key exchange or cipher algorithms which are taking too long and the client times out. Incrfeasing the timout setting for the client may fix this. Alternatively, you can use the algorithm property of the connect object to specify different key exchange and cipher algorithms to try and force the client and server to negotiate a faster algorithm. Note that htis issue has come up a few times in the ssh2 repository and many of the closed issues in that repository show how to set different algorithms etc.
  2. None of the default key exchange or cipher algorithms used by ssh2 are supported by your server, preventing either the key exchange or cipher negotiation steps completing. The best way to resolve this is to use openSSH's sftp client, adding the -vvv switch to turn up debugging and see what key exchange and cipher algorithms it negotiates and then ensuring those are added to the set of supported key exchange and cipher algorithms by using the algorithm property. Again, see the ssh2 documentation for connect and similar issues in the ssh2 repositoryh.

Make sure you are using ssh2-sftp-client v10.0.3

Thank you for the answer, i was indeed using an old version of this package, upgrading to the last one, i'm getting some errors building coming from ssh2 and cpu-features, have you seen it? the only thing that changed was the package version

image
theophilusx commented 4 months ago

These are related to optional dependencies and can safely be ignored. Again, comes up frequently in issues on the ssh2 repository, so check there for more details. Depending on your package manager, you can turn off optional dependencies and this error/warning will go away. Alternatively, install the low level tools necessary to build c++ dependencies and your package will add the cpu frequency support.

PedroS11 commented 4 months ago

I'll have a better look but it seems I got it fixed now!! Basically:

Thank you for the help!

vignesh-sowmitri commented 3 months ago

I have the exact same error as shown in this link https://github.com/mscdex/cpu-features/issues/5 I installed the node-loader and but still get the same error. I dont have webpack.config.js since i am using serverless-bundle. Can you help or share your webpack.config.js ?

theophilusx commented 3 months ago

This has nothing to do with ssh2-sftp-client. This is an ssh2 'issue'.

The cpu-features requirements is an optional dependency. Both ssh2 and ssh2-sftp-client will work fine without cpu-freatures. The real problem here is that your package manager is not handling/reporting absent optional dependencies as a warning instead of an error.

There is nothing I can do about this at the ssh2-sftp-client level, so am going to close this issue.

vignesh-sowmitri commented 3 months ago

Hi @theophilusx , Yes i was able to fix the cpu-features as an optional dependency. Its not disturbing the ssh2-sftp-client functionality as you said. But i still have the ssh2 issue. I tried to make ssh2 as optional too. But the functionality breaks. I am using serverless-bundle package

image

Error : image

theophilusx commented 3 months ago

This is an issue with whatever deployment tool your using. It has nothing to do with ssh2-sftp-client.

I have no idea what the "serveless-budnle" package is, but form the error message, it would appear it DOES use webpack 'udner the hood'.

This is an error being caused by your chosen build/deployment chain and outside anything ssh2-sftp-client has control over. There is nothing ssh2-sftp-client can do at this level and is not an ssh2-sftp-client issue.

You might be able to get some guidance from the ssh2 project, but I suspect the answer will be the same.

vignesh-sowmitri commented 3 months ago

Hi I think i was able to fix with serverless-bundle itself where i need to add the ssh2 as an external bundle package and as u said I was able to make cpu-features as an optional dependency. Thanks for the clarification.