From c20a440d46f6cbd7bb2d9df303ae764ffb1da497 Mon Sep 17 00:00:00 2001
From: Julian Simioni <julian@simioni.org>
Date: Thu, 8 Aug 2019 16:57:58 -0400
Subject: [PATCH] feat(http): Improve signal handling and logging

This PR improves the signal handling of Placeholder so that it exits
gracefully in all cases, even when running under Docker.

There appear to have been several interconnected issues:

- SIGINT was not handled, which is the signal sent by `ctrl-C` on the
command line and by docker

- the signal handler called `app.close()`, which is not defined. Instead
`.close()` must be called on the http server returned from
`app.listen()`

Because of the `cluster` module, there's a few different paths through
the code: either with multiple CPUs or with a single CPU. In the case of
multiple CPUs the signal handler has to work for both a worker and
master process.

Along the way, logging has been a bit streamlined and now uses
`pelias-logger`.
---
 server/http.js | 47 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/server/http.js b/server/http.js
index f40b4843..3735dfd7 100644
--- a/server/http.js
+++ b/server/http.js
@@ -35,6 +35,12 @@ var PORT = process.env.PORT || 3000;
 var HOST = process.env.HOST || undefined;
 var app = express();
 
+// store the express http server so it can be terminated gracefully later
+let server;
+
+//record whether the service is terminating to control what events are worth logging
+let terminating = false;
+
 function log() {
   morgan.token('url', (req, res) => {
     // if there's a DNT header, just return '/' as the URL
@@ -58,7 +64,6 @@ function log() {
 app.use(log());
 
 // init placeholder
-console.error( 'loading data' );
 var ph = new Placeholder({ readonly: true });
 ph.load();
 
@@ -95,25 +100,41 @@ app.get( '/parser/tokenize', require( './routes/tokenize' ) );
 app.use('/demo', express.static( __dirname + '/demo' ));
 app.use('/', (req, res) => { res.redirect('/demo#eng'); });
 
-// handle SIGTERM (required for fast docker restarts)
-process.on('SIGTERM', () => {
+// handle SIGINT and SIGTERM (required for fast docker restarts)
+function handler() {
   ph.close();
-  app.close();
-});
+
+  terminating = true;
+  if (cluster.isMaster) {
+    logger.info('Placeholder service shutting down');
+    for (const id in cluster.workers) {
+      cluster.workers[id].kill('SIGINT');
+    }
+  }
+
+  if (server) {
+    server.close();
+  }
+}
+
+process.on('SIGINT', handler);
+process.on('SIGTERM', handler);
 
 // start multi-threaded server
 if( cpus > 1 ){
   if( cluster.isMaster ){
-    console.error('[master] using %d cpus', cpus);
+    logger.info('[master] using %d cpus', cpus);
 
     // worker exit event
     cluster.on('exit', (worker, code, signal) => {
-      console.error('[master] worker died', worker.process.pid);
+      if (!terminating) {
+        logger.error('[master] worker died', worker.process.pid);
+      }
     });
 
     // worker fork event
     cluster.on('fork', (worker, code, signal) => {
-      console.error('[master] worker forked', worker.process.pid);
+      logger.info('[master] worker forked', worker.process.pid);
     });
 
     // fork workers
@@ -122,17 +143,17 @@ if( cpus > 1 ){
     }
 
   } else {
-    app.listen( PORT, HOST, () => {
-      console.error('[worker %d] listening on %s:%s', process.pid, HOST||'0.0.0.0', PORT);
+    server = app.listen( PORT, HOST, () => {
+      logger.info('[worker %d] listening on %s:%s', process.pid, HOST||'0.0.0.0', PORT);
     });
   }
 }
 
 // start single-threaded server
 else {
-  console.error('[master] using %d cpus', cpus);
+  logger.info('[master] using %d cpus', cpus);
 
-  app.listen( PORT, HOST, () => {
-    console.log('[master] listening on %s:%s', HOST||'0.0.0.0', PORT);
+  server = app.listen( PORT, HOST, () => {
+    logger.info('[master] listening on %s:%s', HOST||'0.0.0.0', PORT);
   });
 }