fix: signal not-ready when shutdown() is used (fixes #25)

gajus · Oct 10, 2020 · f457018 · f457018
1 parent 2f0d23b
commit f457018
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 68 deletions.
diff --git a/src/factories/createLightship.js b/src/factories/createLightship.js
@@ -105,12 +105,6 @@ export default (userConfiguration?: ConfigurationInputType): LightshipType => {
   });
 
   const signalNotReady = () => {
-    if (serverIsShuttingDown) {
-      log.warn('server is already shutting down');
-
-      return;
-    }
-
     if (serverIsReady === false) {
       log.warn('server is already in a SERVER_IS_NOT_READY state');
     }
@@ -132,7 +126,7 @@ export default (userConfiguration?: ConfigurationInputType): LightshipType => {
     serverIsReady = true;
   };
 
-  const shutdown = async () => {
+  const shutdown = async (nextReady: boolean) => {
     if (serverIsShuttingDown) {
       log.warn('server is already shutting down');
 
@@ -155,7 +149,8 @@ export default (userConfiguration?: ConfigurationInputType): LightshipType => {
     }
 
     // @see https://github.com/gajus/lightship/issues/12
-    serverIsReady = true;
+    // @see https://github.com/gajus/lightship/issues/25
+    serverIsReady = nextReady;
     serverIsShuttingDown = true;
 
     if (beacons.length) {
@@ -238,7 +233,7 @@ export default (userConfiguration?: ConfigurationInputType): LightshipType => {
           signal,
         }, 'received a shutdown signal');
 
-        shutdown();
+        shutdown(true);
       });
     }
   }
@@ -277,7 +272,9 @@ export default (userConfiguration?: ConfigurationInputType): LightshipType => {
       shutdownHandlers.push(shutdownHandler);
     },
     server,
-    shutdown,
+    shutdown: () => {
+      return shutdown(false);
+    },
     signalNotReady,
     signalReady,
   };

diff --git a/test/lightship/factories/createLightship.js b/test/lightship/factories/createLightship.js
@@ -158,7 +158,7 @@ test('calling `shutdown` changes server state to SERVER_IS_SHUTTING_DOWN', async
 
   lightship.shutdown();
 
-  t.is(lightship.isServerReady(), true);
+  t.is(lightship.isServerReady(), false);
   t.is(lightship.isServerShuttingDown(), true);
 
   const serviceState = await getServiceState(lightship.server.address().port);
@@ -169,9 +169,8 @@ test('calling `shutdown` changes server state to SERVER_IS_SHUTTING_DOWN', async
   t.is(serviceState.live.status, 500);
   t.is(serviceState.live.message, SERVER_IS_SHUTTING_DOWN);
 
-  // @see https://github.com/gajus/lightship/issues/12
-  t.is(serviceState.ready.status, 200);
-  t.is(serviceState.ready.message, SERVER_IS_READY);
+  t.is(serviceState.ready.status, 500);
+  t.is(serviceState.ready.message, SERVER_IS_NOT_READY);
 
   if (!shutdown) {
     throw new Error('Unexpected state.');
@@ -182,6 +181,23 @@ test('calling `shutdown` changes server state to SERVER_IS_SHUTTING_DOWN', async
   t.is(terminate.called, false);
 });
 
+test('invoking `shutdown` using a signal causes SERVER_IS_READY', (t) => {
+  const terminate = sinon.stub();
+
+  const lightship = createLightship({
+    detectKubernetes: false,
+    signals: [
+      'LIGHTSHIP_TEST',
+    ],
+    terminate,
+  });
+
+  process.emit('LIGHTSHIP_TEST');
+
+  t.is(lightship.isServerReady(), true);
+  t.is(lightship.isServerShuttingDown(), true);
+});
+
 test('error thrown from within a shutdown handler does not interrupt the shutdown sequence', async (t) => {
   const terminate = sinon.stub();
 
@@ -256,60 +272,6 @@ test('calling `shutdown` multiple times results in shutdown handlers called once
   t.is(terminate.called, false);
 });
 
-test('calling `signalNotReady` after `shutdown` does not have effect on server state', async (t) => {
-  const terminate = sinon.stub();
-
-  const lightship = createLightship({
-    terminate,
-  });
-
-  let shutdown;
-
-  lightship.registerShutdownHandler(() => {
-    return new Promise((resolve) => {
-      shutdown = resolve;
-    });
-  });
-
-  lightship.signalReady();
-
-  t.is(lightship.isServerReady(), true);
-  t.is(lightship.isServerShuttingDown(), false);
-
-  const serviceState0 = await getServiceState(lightship.server.address().port);
-
-  t.is(serviceState0.health.status, 200);
-  t.is(serviceState0.health.message, SERVER_IS_READY);
-
-  lightship.shutdown();
-
-  t.is(lightship.isServerReady(), true);
-  t.is(lightship.isServerShuttingDown(), true);
-
-  const serviceState1 = await getServiceState(lightship.server.address().port);
-
-  t.is(serviceState1.health.status, 500);
-  t.is(serviceState1.health.message, SERVER_IS_SHUTTING_DOWN);
-
-  lightship.signalNotReady();
-
-  t.is(lightship.isServerReady(), true);
-  t.is(lightship.isServerShuttingDown(), true);
-
-  const serviceState2 = await getServiceState(lightship.server.address().port);
-
-  t.is(serviceState2.health.status, 500);
-  t.is(serviceState2.health.message, SERVER_IS_SHUTTING_DOWN);
-
-  if (!shutdown) {
-    throw new Error('Unexpected state.');
-  }
-
-  await shutdown();
-
-  t.is(terminate.called, false);
-});
-
 test('presence of live beacons suspend the shutdown routine', async (t) => {
   const terminate = sinon.stub();