mscdex / node-ftp

An FTP client module for node.js
MIT License
1.13k stars 243 forks source link

Keepalive noop not working #139

Closed hnrch02 closed 8 years ago

hnrch02 commented 8 years ago

The FTP server I'm working with apparently doesn't respect the no-ops that are being sent and returns

[connection] > NOOP
[connection] < '421 Timeout (no operation for 1800 seconds)\r\n'
[parser] < '421 Timeout (no operation for 1800 seconds)\r\n'
[parser] Response: code=421, buffer='Timeout (no operation for 1800 seconds)'

after half an hour. Any ideas how I could work around that?

mscdex commented 8 years ago

Did you verify that it's sending NOOP sooner than 30 minutes after the last command? By default it should start sending NOOP every 10 seconds.

hnrch02 commented 8 years ago

Yes, the log was filled with this:

[connection] > NOOP
[connection] < '200 Zzz...\r\n'
[parser] < '200 Zzz...\r\n'
[parser] Response: code=200, buffer='Zzz...'
hnrch02 commented 8 years ago

I'm currently testing whether sending STAT every five minutes prevents this.

mscdex commented 8 years ago

Yeah some server implementations are more strict than others when it comes to things like this. You may have to manually use some other command on an interval as an alternative to NOOP for these kinds of servers.

hnrch02 commented 8 years ago

Okay, that seems to do the trick.

hnrch02 commented 8 years ago

Maybe you could add an option to configure the no-op command?

mscdex commented 8 years ago

I'm not sure that making that configurable would be useful to most people. I think these kinds of servers are in the minority and there is nothing to stop a server from implementing some other kind of "inactivity" detection (e.g. a server checking if the last n issued commands are exactly the same over a period of time).

hnrch02 commented 8 years ago

I agree that the general usefulness would be rather limited but then I wouldn't have to deal with that interval since all that logic is already implemented anyway.

hnrch02 commented 8 years ago

Anyway, I found my solution. The config option would be handy since I wouldn't have to duplicate functionality but I can see where you're coming from.