[Issue #7] Add support for graceful shutdowns.

[sean]

With this change hitting Control+C or sending kill -{2,15} (SIGINT or SIGTERM) will cause the server to shutdown gracefully draining any connections. Clients which connect will see this message:

Server is in the process of restarting.

If the connections are drained successfully prior to the timeout, stdout will display:

Received kill signal, shutting down gracefully.
Closed out remaining connections.

and the exit code will be 0. Otherwise you'll see:

Received kill signal, shutting down gracefully.
Could not close connections in time, forcefully shutting down.

and the exit code will be 1. The timeout is hardcoded at 30s, perhaps this should be configurable.

[totherik]

Although I don't have time to review this right this second, I'm not ashamed to say I want to hug you for contributing this, and so quickly. Thanks @sean!

[jeffharrell]

Awesome 👍

Shouldn't the server return a 503 instead of a 502 in this case? Also, either as part of this or as a subsequent commit we should enable a template to be loaded from middleware:errorPages

[lensam69]

Agree with the team.
Thanks!
Just one very minor nitpick:
Should say "shutting down" instead of restarting.

[sean]

Good catch, that was a typo, I've corrected it in the most recent patch. Now it sets a status of 503 (Service Unavailable) and renders the errorPages['503'] template if defined.

[lensam69]

Agree with the team.
Thanks!
Just a minor nitpick on this line: Should it say "shutting down" instead of "restarting"?

[totherik]

Thanks @sean. Just a few nitpicks/thoughts.

In index.js can you please either 1) move the httpServer variable declaration to the top of the function to match coding style, or 2) inline the call: app.set('kraken:server', server.listen(port, host));?

I see there's mixed single and double quotes in some places. The coding style for kraken-js is single quotes everywhere.

Should the kraken config object support more generic states? e.g kraken:state could be set to disconnected, listening, disconnecting, etc. Would there be value in that over a single binary state or is it not worth the complexity of transitioning states?

I know we don't (yet) have logging integrated into kraken, but can you please remove the console.log statements? I'm thinking that in addition to logging we should support debuglog, and maybe these statements could use that in the interim. Either way, I'm doing my best to resist the temptation to have a module write unwanted stuff to folk's stdout/stderr.

Can we make the shutdown-timeout configurable with 30 seconds as a default?

[sean]

@totherik I've incorporated all of the review feedback and the pull request has been updated. Thanks!

[jeffharrell]

Two more nitpicks and then I think we're good:

1. +1 to Erik's idea about using kraken:state=disconnecting rather than kraken:shuttingDown=true. The former is a tad more flexible.
2. The default config value should be populated and taken from https://github.com/sean/kraken-js/blob/master/config/webcore.json . Also, it should be camelCase and not use underscores. The user config is merged with this file on startup, so the defaults are implied if nothing overrides them.

[lmarkus]

I like it 👍

[jeffharrell]

Thanks for the work, @sean. I filed #60 to track that we should add additonal values to the kraken:state value, but otherwise this is good to go.

[sean]

Thanks @jeffharrell!