From 19852de5cb57c304a5cb5f3b9c594c73aecae19f Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 20 Apr 2022 12:47:25 +0100 Subject: [PATCH] Removed config and bluebird from ghost-server - In the interest of simplifying and cleaning up this code which affects Ghost's boot process - Moved config to DI, injecting only known properties. It's expected that you must restart the server to pick up new config - Even in tests! - Got rid of unused bluebird code, but also reworked some promise-based code to be simpler using util.promisify --- core/boot.js | 2 +- core/server/ghost-server.js | 63 +++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/core/boot.js b/core/boot.js index c17ca5f188..c6564a2d7a 100644 --- a/core/boot.js +++ b/core/boot.js @@ -402,7 +402,7 @@ async function bootGhost({backend = true, frontend = true, server = true} = {}) if (server) { const GhostServer = require('./server/ghost-server'); - ghostServer = new GhostServer({url: config.getSiteUrl()}); + ghostServer = new GhostServer({url: config.getSiteUrl(), env: config.get('env'), serverConfig: config.get('server')}); await ghostServer.start(rootApp); bootLogger.log('server started'); debug('End: load server + minimal app'); diff --git a/core/server/ghost-server.js b/core/server/ghost-server.js index 846c029d8d..d417fc0d0a 100644 --- a/core/server/ghost-server.js +++ b/core/server/ghost-server.js @@ -1,9 +1,6 @@ // # Ghost Server // Handles the creation of an HTTP Server for Ghost const debug = require('@tryghost/debug')('server'); - -const Promise = require('bluebird'); -const config = require('../shared/config'); const errors = require('@tryghost/errors'); const tpl = require('@tryghost/tpl'); const logging = require('@tryghost/logging'); @@ -39,14 +36,25 @@ const messages = { * ## GhostServer */ class GhostServer { - constructor({url}) { + /** + * + * @param {Object} options + * @param {String} options.url + * @param {String} options.env development|production|testing + * @param {Object} options.serverConfig + * @param {String} options.serverConfig.host + * @param {Number} options.serverConfig.port + * @param {Number} options.serverConfig.shutdownTimeout + * @param {Boolean} options.serverConfig.testmode + */ + constructor({url, env, serverConfig}) { this.url = url; + this.env = env; + this.serverConfig = serverConfig; + this.rootApp = null; this.httpServer = null; - // Expose config module for use externally. - this.config = config; - // Tasks that should be run before the server exits this.cleanupTasks = []; } @@ -63,13 +71,15 @@ class GhostServer { */ start(rootApp) { debug('Starting...'); + this.rootApp = rootApp; + + const {host, port, testmode, shutdownTimeout} = this.serverConfig; const self = this; - self.rootApp = rootApp; return new Promise(function (resolve, reject) { self.httpServer = rootApp.listen( - config.get('server').port, - config.get('server').host + port, + host ); self.httpServer.on('error', function (error) { @@ -78,7 +88,7 @@ class GhostServer { if (error.code === 'EADDRINUSE') { ghostError = new errors.InternalServerError({ message: tpl(messages.addressInUse.error), - context: tpl(messages.addressInUse.context, {port: config.get('server').port}), + context: tpl(messages.addressInUse.context, {port}), help: tpl(messages.addressInUse.help) }); } else { @@ -101,7 +111,7 @@ class GhostServer { self._logStartMessages(); // Debug logs output in testmode only - if (config.get('server:testmode')) { + if (testmode) { self._startTestMode(); } @@ -112,7 +122,7 @@ class GhostServer { }); }); - stoppable(self.httpServer, config.get('server:shutdownTimeout')); + stoppable(self.httpServer, shutdownTimeout); // ensure that Ghost exits correctly on Ctrl+C and SIGTERM process @@ -146,7 +156,7 @@ class GhostServer { * Stops the server & handles cleanup, but does not exit the process * Used in tests for quick start/stop actions * Called by shutdown to handle server stop and cleanup before exiting - * @returns {Promise} Resolves once Ghost has stopped + * @returns {Promise} Resolves once Ghost has stopped */ async stop() { try { @@ -190,22 +200,13 @@ class GhostServer { * If server.shutdownTimeout is reached, requests are terminated in-flight */ async _stopServer() { - return new Promise((resolve, reject) => { - this.httpServer - .stop((error, status) => { - if (error) { - return reject(error); - } - - return resolve(status); - }); - }); + const util = require('util'); + return util.promisify(this.httpServer.stop)(); } async _cleanup() { // Wait for all cleanup tasks to finish - await Promise - .all(this.cleanupTasks.map(task => task())); + return Promise.all(this.cleanupTasks.map(task => task())); } /** @@ -250,14 +251,14 @@ class GhostServer { * Log Start Messages */ _logStartMessages() { - logging.info(tpl(messages.ghostIsRunningIn, {env: config.get('env')})); + logging.info(tpl(messages.ghostIsRunningIn, {env: this.env})); - if (config.get('env') === 'production') { + if (this.env === 'production') { logging.info(tpl(messages.yourBlogIsAvailableOn, {url: this.url})); } else { logging.info(tpl(messages.listeningOn, { - host: config.get('server').host, - port: config.get('server').port + host: this.serverConfig.host, + port: this.serverConfig.port })); logging.info(tpl(messages.urlConfiguredAs, {url: this.url})); } @@ -272,7 +273,7 @@ class GhostServer { logging.warn(tpl(messages.ghostHasShutdown)); // Extra clear message for production mode - if (config.get('env') === 'production') { + if (this.env === 'production') { logging.warn(tpl(messages.yourBlogIsNowOffline)); }