Refactored server announce functions to be clearer

- the announce functions exist for the purpose of communicating with Ghost CLI
- the functions were called announceServerStart and announceServerStopped, which implies they tell Ghost CLI when the server starts and stops
- however, the true intention / purpose of these functions is to:
    - either tell Ghost CLI when Ghost has successfully booted (e.g. is ready to serve requests)
    - or tell Ghost CLI when the server failed to boot, and report the error so that Ghost CLI can communicate it to the user
- therefore, I've refactored the old functions into 1 function to make it clearer they do the same job, but with 2 different states
- also added some tests :D
This commit is contained in:
Hannah Wolfe 2020-08-09 16:40:47 +01:00
parent c577007afe
commit 71f02d25e9
5 changed files with 133 additions and 55 deletions

View File

@ -16,7 +16,7 @@ function makeGhost(options) {
err = new errors.GhostError({message: err.message, err: err}); err = new errors.GhostError({message: err.message, err: err});
} }
return GhostServer.announceServerStopped(err) return GhostServer.announceServerReadiness(err)
.finally(() => { .finally(() => {
throw err; throw err;
}); });

View File

@ -29,13 +29,6 @@ function GhostServer(rootApp) {
this.config = config; this.config = config;
} }
const debugInfo = {
versions: process.versions,
platform: process.platform,
arch: process.arch,
release: process.release
};
/** /**
* ## Public API methods * ## Public API methods
* *
@ -109,7 +102,7 @@ GhostServer.prototype.start = function (externalApp) {
debug('...Started'); debug('...Started');
self.logStartMessages(); self.logStartMessages();
return GhostServer.announceServerStart() return GhostServer.announceServerReadiness()
.finally(() => { .finally(() => {
resolve(self); resolve(self);
}); });
@ -249,65 +242,50 @@ GhostServer.prototype.logShutdownMessages = function () {
module.exports = GhostServer; module.exports = GhostServer;
/** /**
* @NOTE announceServerStartCalled: * We call announce server readiness when the server is ready
* When the server is started, but not ready, it is only able to serve 503s
* *
* - backwards compatible logic, because people complained that not all themes were loaded when using Ghost as NPM module * If the server isn't able to reach readiness, announceServerReadiness is called with an error
* - we told them to call `announceServerStart`, which is not required anymore, because we restructured the code * A status message, any error, and debug info are all passed to managing processes via IPC and the bootstrap socket
*/ */
let announceServerStartCalled = false; let announceServerReadinessCalled = false;
module.exports.announceServerStart = function announceServerStart() {
if (announceServerStartCalled || config.get('maintenance:enabled')) { const debugInfo = {
versions: process.versions,
platform: process.platform,
arch: process.arch,
release: process.release
};
module.exports.announceServerReadiness = function (error = null) {
// If we already announced readiness, we should not do it again
if (announceServerReadinessCalled) {
return Promise.resolve(); return Promise.resolve();
} }
announceServerStartCalled = true;
let socketAddress = config.get('bootstrap-socket'); // Mark this function as called
announceServerReadinessCalled = true;
// Build our message
// - if there's no error then the server is ready
let message = { let message = {
started: true, started: true,
debug: debugInfo debug: debugInfo
}; };
events.emit('server.start'); // - if there's an error then the server is not ready, include the errors
if (error) {
message.started = false;
message.error = error;
}
// CASE: IPC communication to the CLI via child process. // CASE: IPC communication to the CLI for local process manager
if (process.send) { if (process.send) {
process.send(message); process.send(message);
} }
// CASE: Ghost extension - bootstrap sockets // CASE: use bootstrap socket to communicate with CLI for systemd
if (socketAddress) {
return bootstrapSocket.connectAndSend(socketAddress, logging, message);
}
return Promise.resolve();
};
/**
* @NOTE announceServerStopCalled:
*
* - backwards compatible logic, because people complained that not all themes were loaded when using Ghost as NPM module
* - we told them to call `announceServerStart`, which is not required anymore, because we restructured code
*/
let announceServerStopCalled = false;
module.exports.announceServerStopped = function announceServerStopped(error) {
if (announceServerStopCalled) {
return Promise.resolve();
}
announceServerStopCalled = true;
let socketAddress = config.get('bootstrap-socket'); let socketAddress = config.get('bootstrap-socket');
let message = {
started: false,
error: error,
debug: debugInfo
};
// CASE: IPC communication to the CLI via child process.
if (process.send) {
process.send(message);
}
// CASE: Ghost extension - bootstrap sockets
if (socketAddress) { if (socketAddress) {
return bootstrapSocket.connectAndSend(socketAddress, logging, message); return bootstrapSocket.connectAndSend(socketAddress, logging, message);
} }

View File

@ -138,10 +138,10 @@ const minimalRequiredSetupToStartGhost = (dbState) => {
.then(() => { .then(() => {
config.set('maintenance:enabled', false); config.set('maintenance:enabled', false);
logging.info('Blog is out of maintenance mode.'); logging.info('Blog is out of maintenance mode.');
return GhostServer.announceServerStart(); return GhostServer.announceServerReadiness();
}) })
.catch((err) => { .catch((err) => {
return GhostServer.announceServerStopped(err) return GhostServer.announceServerReadiness(err)
.finally(() => { .finally(() => {
logging.error(err); logging.error(err);
setTimeout(() => { setTimeout(() => {

View File

@ -0,0 +1,100 @@
const should = require('should');
const sinon = require('sinon');
const configUtils = require('../../utils/configUtils');
const bootstrapSocket = require('@tryghost/bootstrap-socket');
describe('GhostServer', function () {
describe('announceServerReadiness', function () {
let GhostServer;
let socketStub;
beforeEach(function () {
// Have to re-require each time to clear the internal flag
delete require.cache[require.resolve('../../../core/server/ghost-server')];
GhostServer = require('../../../core/server/ghost-server');
// process.send isn't set for tests, we can safely override;
process.send = sinon.stub();
// stub socket connectAndSend method
socketStub = sinon.stub(bootstrapSocket, 'connectAndSend');
});
afterEach(function () {
process.send = undefined;
configUtils.restore();
socketStub.restore();
});
it('it resolves a promise', function () {
GhostServer.announceServerReadiness().should.be.fulfilled();
});
it('it communicates with IPC correctly on success', function () {
GhostServer.announceServerReadiness();
process.send.calledOnce.should.be.true();
let message = process.send.firstCall.args[0];
message.should.be.an.Object().with.properties('started', 'debug');
message.should.not.have.property('error');
message.started.should.be.true();
});
it('communicates with IPC correctly on failure', function () {
GhostServer.announceServerReadiness(new Error('something went wrong'));
process.send.calledOnce.should.be.true();
let message = process.send.firstCall.args[0];
message.should.be.an.Object().with.properties('started', 'debug', 'error');
message.started.should.be.false();
message.error.should.be.an.Object().with.properties('message');
message.error.message.should.eql('something went wrong');
});
it('communicates via bootstrap socket correctly on success', function () {
configUtils.set('bootstrap-socket', 'testing');
GhostServer.announceServerReadiness();
socketStub.calledOnce.should.be.true();
socketStub.firstCall.args[0].should.eql('testing');
socketStub.firstCall.args[1].should.be.an.Object().with.properties('info', 'warn');
let message = socketStub.firstCall.args[2];
message.should.be.an.Object().with.properties('started', 'debug');
message.should.not.have.property('error');
message.started.should.be.true();
});
it('communicates via bootstrap socket correctly on failure', function () {
configUtils.set('bootstrap-socket', 'testing');
GhostServer.announceServerReadiness(new Error('something went wrong'));
socketStub.calledOnce.should.be.true();
socketStub.firstCall.args[0].should.eql('testing');
socketStub.firstCall.args[1].should.be.an.Object().with.properties('info', 'warn');
let message = socketStub.firstCall.args[2];
message.should.be.an.Object().with.properties('started', 'debug', 'error');
message.started.should.be.false();
message.error.should.be.an.Object().with.properties('message');
message.error.message.should.eql('something went wrong');
});
it('can be called multiple times, but only communicates once', function () {
configUtils.set('bootstrap-socket', 'testing');
GhostServer.announceServerReadiness();
GhostServer.announceServerReadiness(new Error('something went wrong'));
GhostServer.announceServerReadiness();
process.send.calledOnce.should.be.true();
socketStub.calledOnce.should.be.true();
});
});
});

View File

@ -946,7 +946,7 @@ startGhost = function startGhost(options) {
.then(function () { .then(function () {
let timeout; let timeout;
GhostServer.announceServerStart(); GhostServer.announceServerReadiness();
return new Promise(function (resolve) { return new Promise(function (resolve) {
(function retry() { (function retry() {