Handle errors, improve logging and cleanup on shutdown.

[stevenh]

Rework error handling so that all write errors are processed instead of being ignored. This changes the flow of most handle functions to fail fast, with the natural flow being the none error case making it much easier to follow. This also removes the need for panic in handlePASV.

go-kit's logger requires a high number of additional vendor packages just to provide structured logging, so swap this out for logrus which is the most widely used logging package.

Ensure that client connections and signal handler are cleaned up on server shutdown.

Also:

• Improved segregation between server and client.
• Fixed nill driver check in handlePASS.
• Stripped \r\n from command lines to improve logging.
• Enabled keepalives on all connections to prevent deadlock on reads.
• Enabled dial timeouts for active connections.
• Various style cleanups.
• Improved localization of variable calculations.
• Corrected some incorrect comments.
• Removed needless conversions in SetDeadlne call.
• Converted to static commandsMap instead of init().
• Fixed variable shadowing.
• Remove unnecessary testing package.
• Fixed all issues identified by gometalinter.
• Eliminated unneeded buffered writer, which was only doing line buffering, which is very similar to the default anyway; eliminating the need for constant Flush calls.
• Enabled gometalinter.v2 checks under travis, removing redundant golint checks.
• Switched from hardcoded ints to constants for FTP messages, correcting some during the process.
• Renamed client daddy parameter to server to better reflect its real type.

[fclairamb]

Hi @stevenh,

Thank you very much for your PR. I'm sorry for the late answer I'm a bit busy at the time.

Although I'm very thankful for the time you invested on this project, I don't really like having to choose between all and nothing. So I'll probably pick some parts of your PR and leave some others behind.

Here is my first feedback on your PR:

Like (and will be integrated)

• Clean shutdown of server and the associated clients
• Map initialization
• Dial timeout for active connections
• Comments fixes
• SetDeadLine improvement
• Variable shadowing
• gometalinter.v2
• constants
• daddy --> server renaming

About the gometalinter.v2, I think it reveals an issue that is reported in #66. It's not really problematic for files we read but for file we read it's a big one.

Mixed-up feeling (still ensure)

• I'm not sure I understand your motivations behind the keep-alive: "Enable TCP KeepAlives to ensure we don't block forever when waiting for commands.". We already have an IDLE timeout that will take care of silent connection (dead or alive).
• Returning an error every time is more go-compliant but also make the code harder to read. The errors also represent the transmission
• The removal of buffered writing
• Where did you improve segregation?
• I'm not really convinced for now that we need to check for error everytime we write to the control stream. I know it's wrong but it adds a lot of pretty useless code.

Don't like (probably won't be integrated)

• logrus/log15 are too complex. I prefer to keep things simple, that's why I switched from log15 to go-kit/log
• You reintroduced mutexes that I removed. I'm trying as much as possible to avoid them
• Forcing keep-alive (close to the first keep-alive point but worse because you impose them)
• Nil driver checks, it can be done at server construction

I'm not closing the discussion. I might have missed / overlooked some aspects.

[fclairamb]

A lot of the proposed changes have been implemented in #85.