FTPS Keep Alive

[SamKirkland]

First of thanks for making this library @patrickjuchli - really awesome.

Describe the bug
I'm looking for a enhancement to resolve the existing known "Error: Timeout (control socket)" issue. See issue #123

I use this library as a dependency for a few of my libraries see FTP-Deploy-Action and @samkirkland/ftp-deploy. This is a ongoing issue and I'd love to work with you on a resolution.

Example code
Please see tickets SamKirkland/FTP-Deploy-Action#144 and SamKirkland/FTP-Deploy-Action#106

Sadly the recreation is very difficult due to it being dependent on routers between the client/server. I am able to recreate a similar error (426 Failure reading network stream) but I believe its the same cause because it only appears with the following conditions:

• Must be using FTPS
• Must be running the deployment code on github servers and connecting to a server in Germany (router dependent)
• Must be uploading a relatively large file, In my recreation I successfully upload a dozen files <1MB and then it fails consistently on a file >1MB

Console output

Error from user reports

Note: Ignore line numbers, i've pushed out a new version. New line number is 3759 in my codebase

 Error: Timeout (control socket)
    at Socket.<anonymous> (/home/runner/work/_actions/SamKirkland/FTP-Deploy-Action/beta-v4/dist/index.js:3701:58)
    at Object.onceWrapper (events.js:299:28)
    at Socket.emit (events.js:210:5)
    at Socket._onTimeout (net.js:469:8)
    at listOnTimeout (internal/timers.js:531:17)
    at processTimers (internal/timers.js:475:7)

Error I am able to recreate

uploading ".htaccess"
uploading "About.php"
uploading "api/.htaccess"
uploading "api/API.class.php"
uploading "api/api.php"
uploading "api/concrete.php"
uploading "api/error_log"
uploading "cist2910/create.php"
uploading "cist2910/css/style.css"
uploading "cist2910/img/drink.svg"
uploading "cist2910/img/exercise.svg"
uploading "cist2910/img/food.svg"
uploading "cist2910/img/music.svg"
uploading "cist2910/index.php"
uploading "cist2910/insert.php"
uploading "cist2910/js/jquery-2.1.4.min.js"
400 level error from server when performing action - retrying...
FTPError: 426 Failure reading network stream.
    at FTPContext._onControlSocketData (/home/runner/work/_actions/SamKirkland/FTP-Deploy-Action/4.0.0/dist/index.js:3759:39)
    at TLSSocket.<anonymous> (/home/runner/work/_actions/SamKirkland/FTP-Deploy-Action/4.0.0/dist/index.js:3600:44)
    at TLSSocket.emit (events.js:210:5)
    at addChunk (_stream_readable.js:309:12)
    at readableAddChunk (_stream_readable.js:286:13)
    at TLSSocket.Readable.push (_stream_readable.js:224:10)
    at TLSWrap.onStreamRead (internal/stream_base_commons.js:182:23) {
  name: 'FTPError',
  code: 426
}

Which version of Node.js are you using?
Node v14.15.1

Additional context
Based on some reading on node keep alive the default interval can be quite long.

See Stackoverflow post one of the recommendations is to use net-keepalive but it doesn't support windows 😞

It appears other clients such as FileZilla and WinSCP are doing it the way you suggested in issue #123

I would love to implement this on my side but looking through the API I don't see how it'll be possible. I can't send commands on the open control socket while the connection is already open.

My current idea is to create a interval to send a NOOP over the control socket every . Internal could be set within handle and cleared when the promise is resolved/rejected. A new keepAlive interval number option would be added to the client constructor next to timeout. Default is undefined (non-breaking), if a number is passed we will send a "NOOP" every N ms.

[patrickjuchli]

Here we are again :)

This issue describes one of the central problems of FTPS and one of the reasons you shouldn't use FTP today if you have a choice. But apparently we still need it now and then.

The problem I have with NOOP while another task is running is that it isn't something supported by the FTP protocol. The protocol is always request-response, possibly multiple responses. But something like requestA, requestB, responseB, responseA is just not supported.

Still, you can try to do it. But then you need to be careful that you can discern the answer to NOOP from the answer to whatever other command is currently running. Not all FTP servers return the same answers, though. Some just always return 200 to whatever you've sent, others might be more specific. There might be FTP servers that don't support NOOP as well.

I have two answers to this problem:

1. If you transfer large files, you have to handle any connection issue and resuming ongoing transfers anyway. This reconnect and resumption is made possible by this library but I've decided to leave implementing the mechanics to the application developers. Not sure about this anymore, see this ticket here Resume file transfers after connection issues #124. If you implement something like this, you automatically handle the problem you describe here as well. This is my preferred solution, you solve two problems at once in a reliable way that works with the FTP protocol.

2. If you want to explore the NOOP route, I'd suggest to maybe limit sending NOOP over the control socket only during a transfer over the data connection. There is already a class called TransferResolver handling transfer state management, so maybe there? Not sure. The main challenge will be discerning responses as described above. By the way, I've also heard client sending something else than NOOP in case the server doesn't like it. I don't know more about this though. I find this solution fiddly to be honest but maybe investigating it a bit might be helpful as well.

[patrickjuchli]

I will close this issue to make it transparent that there are no current plans to address this issue.

Generally, I would advise to follow-up on solution 1 outlined above: Support retries of transfer(s) regardless of the actual reason, be it timeouts, disk issues or else. The scope of this library is not to be a full-fledged application like Filezilla, though, but to provide basic building blocks. From my perspective, transfer resumption is an application issue because desired behaviour might vary depending on context. I understand that there is an argument against that but I want to make clear here that at least I don't have any plans to pursue this any further.

One hint for anyone looking into this in their application: Both uploadFrom and downloadTo support partial uploads/downloads to resume work on already transferred data.