From 176433e30745f23cbaccc565aa34678deed6eac8 Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Fri, 19 Feb 2021 20:32:37 +0000
Subject: [PATCH] Refactored notify to send started + ready

- In the old boot the server wasn't started til we were ready
- In new boot, we start the server immediately and send the old started event
- Then, when we are ready to accept some traffic, we send a ready event
- At the moment, ready isn't quite sent at the right time:
   - It _should_ be when we're ready to serve real traffic, not just send 503s
   - This is after the URL generation has finished
   - But this requires more refactoring work :(
   - So for now we send when everything else is ready
- This really needs some tests
---
 core/boot.js                | 15 +++++++--------
 core/server/ghost-server.js |  8 ++++++--
 core/server/notify.js       | 37 ++++++++++++++++++++++++-------------
 test/utils/index.js         |  3 ---
 4 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/core/boot.js b/core/boot.js
index 7abf5667e3..c8881f95a5 100644
--- a/core/boot.js
+++ b/core/boot.js
@@ -187,16 +187,15 @@ function mountGhost(rootApp, ghostApp) {
     debug('End: mountGhost');
 }
 
-// @TODO: make this notification different
-function notifyReadiness(error) {
+function notifyServerReady(error) {
     const notify = require('./server/notify');
 
     if (error) {
-        debug('Notifying readiness (error)');
-        notify.notifyServerStarted(error);
+        debug('Notifying server ready (error)');
+        notify.notifyServerReady(error);
     } else {
-        debug('Notifying readiness (success)');
-        notify.notifyServerStarted();
+        debug('Notifying server ready (success)');
+        notify.notifyServerReady();
     }
 }
 
@@ -269,7 +268,7 @@ async function bootGhost() {
         // We are technically done here
         bootLogger.log('booted');
 
-        notifyReadiness();
+        notifyServerReady();
 
         // Init our background jobs, we don't wait for this to finish
         initRecurringJobs({config});
@@ -289,10 +288,10 @@ async function bootGhost() {
         }
 
         logging.error(serverStartError);
-        notifyReadiness(serverStartError);
 
         // If ghost was started and something else went wrong, we shut it down
         if (ghostServer) {
+            notifyServerReady(serverStartError);
             ghostServer.shutdown(2);
         } else {
             // Ghost server failed to start, set a timeout to give logging a chance to flush
diff --git a/core/server/ghost-server.js b/core/server/ghost-server.js
index 6a451a99ff..52c696cd1f 100644
--- a/core/server/ghost-server.js
+++ b/core/server/ghost-server.js
@@ -96,7 +96,11 @@ class GhostServer {
                     });
                 }
 
-                reject(ghostError);
+                debug('Notifying server started (error)');
+                return notify.notifyServerStarted()
+                    .finally(() => {
+                        reject(ghostError);
+                    });
             });
 
             self.httpServer.on('listening', function () {
@@ -108,7 +112,7 @@ class GhostServer {
                     self._startTestMode();
                 }
 
-                debug('server notifying started');
+                debug('Notifying server ready (success)');
                 return notify.notifyServerStarted()
                     .finally(() => {
                         resolve(self);
diff --git a/core/server/notify.js b/core/server/notify.js
index d488e5cd87..7c89717137 100644
--- a/core/server/notify.js
+++ b/core/server/notify.js
@@ -10,7 +10,10 @@
 const config = require('../shared/config');
 const logging = require('../shared/logging');
 
-let notifyServerStartedCalled = false;
+let notified = {
+    started: false,
+    ready: false
+};
 
 const debugInfo = {
     versions: process.versions,
@@ -19,27 +22,27 @@ const debugInfo = {
     release: process.release
 };
 
-module.exports.notifyServerStarted = function (error = null) {
-    // If we already sent a ready notification, we should not do it again
-    if (notifyServerStartedCalled) {
-        return Promise.resolve();
+async function notify(type, error = null) {
+    // If we already sent this notification, we should not do it again
+    if (notified[type]) {
+        return;
     }
 
     // Mark this function as called
-    notifyServerStartedCalled = true;
+    notified[type] = true;
 
     // Build our message
-    // - if there's no error then the server is ready
-    let message = {
-        started: true,
-        debug: debugInfo
-    };
-
     // - if there's an error then the server is not ready, include the errors
+    // - if there's no error then the server has started
+    let message = {};
     if (error) {
-        message.started = false;
+        message[type] = false;
         message.error = error;
+    } else {
+        message[type] = true;
     }
+    // Add debug info to the message
+    message.debug = debugInfo;
 
     // CASE: IPC communication to the CLI for local process manager
     if (process.send) {
@@ -54,4 +57,12 @@ module.exports.notifyServerStarted = function (error = null) {
     }
 
     return Promise.resolve();
+}
+
+module.exports.notifyServerStarted = async function (error = null) {
+    return await notify('started', error);
+};
+
+module.exports.notifyServerReady = async function (error = null) {
+    return await notify('ready', error);
 };
diff --git a/test/utils/index.js b/test/utils/index.js
index ee1406b7d6..b7f63aa9cb 100644
--- a/test/utils/index.js
+++ b/test/utils/index.js
@@ -251,9 +251,6 @@ const freshModeGhostStart = async (options) => {
     // Actually boot Ghost
     await bootGhost(options);
 
-    // Ensure notify was called (this is idempotent)
-    notify.notifyServerStarted();
-
     // Wait for the URL service to be ready, which happens after boot, but don't re-trigger db.ready
     await urlServiceUtils.isFinished({disableDbReadyEvent: true});
 };