FTP.ftp_RETR() should pass FTPCmdError errors back to the client

[exarkun]

This is like #4909. Any error encountered while reading a file and sending it to a client, to respond to a RETR command, gets logged and a CNX_CLOSED_TXFR_ABORTED is sent back to the client. If the failure is an FTPCmdError though, it can be presented to the client to give them more information about what went wrong.

[adiroiban]

I have attached a patch based on the implementation from #4909

Please let me know if it requires changes.

Cheers, Adi

[spiv]

This looks acceptable to me. The code for ftp_RETR is alarmingly similar to ftp_STOR, and this leads to annoying similar tests, unsurprisingly… but that's not the fault of this patch, and in fact this patch makes the situation slightly better by making ftp_RETR more similar to ftp_STOR. So if buildbot is happy with this patch I'll land it for you.

These two methods almost certainly would be better off refactored into a common helper (perhaps involving an actual object with state rather than a bunch of nested functions) that can be unit tested thoroughly (once), but that will be a separate ticket.

[spiv]

(In [34782]) Branch to ftp-retr-errors-4913.

[spiv]

(In [34785]) Merge ftp-retr-errors-4913: Pass FTPCmdErrors during FTP.ftp_RETR back to the client.

Author: adiroiban Reviewer: spiv Fixes: #4913

[adiroiban]

Thanks for the review and sorry for the late reply.

I agree that there is some code repetition. I plan to fix that as part of #4180

Cheers, Adi