Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00
Code Issues Activity

Improve error handling during bootstrap process

Browse source 
Refs #2986
-More thorough promise handling in bootstrap.js
-Catch rejected promises from the bootstrap module and force
 a Grunt failure instead of an erroneous success
-Adjust the bootstrap unit tests

fix
This commit is contained in:
Jason Williams 2014-06-18 00:25:19 +00:00
parent 6390c6bdc8
commit 258a3cdb40
4 changed files with 49 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Gruntfile.js
View file

@ -699,6 +699,8 @@ var path = require('path'),
var done = this.async(); var done = this.async();
bootstrap().then(function () { bootstrap().then(function () {
done(); done();
}).catch(function (err) {
grunt.fail.fatal(err.stack);
}); });
}); });

47
core/bootstrap.js vendored
View file

@ -13,7 +13,6 @@ var fs = require('fs'),
appRoot = config().paths.appRoot, appRoot = config().paths.appRoot,
configExample = config().paths.configExample, configExample = config().paths.configExample,
rejectMessage = 'Unable to load config',
configFile; configFile;
function readConfigFile(envVal) { function readConfigFile(envVal) {
@ -27,25 +26,32 @@ function writeConfigFile() {
if one doesn't exist. After that, start the server. */ if one doesn't exist. After that, start the server. */
fs.exists(configExample, function checkTemplate(templateExists) { fs.exists(configExample, function checkTemplate(templateExists) {
var read, var read,
write; write,
error;
if (!templateExists) { if (!templateExists) {
return errors.logError(new Error('Could not locate a configuration file.'), appRoot, 'Please check your deployment for config.js or config.example.js.'); error = new Error('Could not locate a configuration file.');
error.context = appRoot;
error.help = 'Please check your deployment for config.js or config.example.js.';
return written.reject(error);
} }
// Copy config.example.js => config.js // Copy config.example.js => config.js
read = fs.createReadStream(configExample); read = fs.createReadStream(configExample);
read.on('error', function (err) { read.on('error', function (err) {
/*jshint unused:false*/ errors.logError(new Error('Could not open config.example.js for read.'), appRoot, 'Please check your deployment for config.js or config.example.js.');
return errors.logError(new Error('Could not open config.example.js for read.'), appRoot, 'Please check your deployment for config.js or config.example.js.');
return written.reject(err);
}); });
read.on('end', written.resolve);
write = fs.createWriteStream(configFile); write = fs.createWriteStream(configFile);
write.on('error', function (err) { write.on('error', function (err) {
/*jshint unused:false*/ errors.logError(new Error('Could not open config.js for write.'), appRoot, 'Please check your deployment for config.js or config.example.js.');
return errors.logError(new Error('Could not open config.js for write.'), appRoot, 'Please check your deployment for config.js or config.example.js.');
return written.reject(err);
}); });
write.on('finish', written.resolve);
read.pipe(write); read.pipe(write);
}); });
@ -62,34 +68,39 @@ function validateConfigEnvironment() {
try { try {
config = readConfigFile(envVal); config = readConfigFile(envVal);
} catch (ignore) { }
catch (e) {
return when.reject(e);
} }
// Check if we don't even have a config // Check if we don't even have a config
if (!config) { if (!config) {
errors.logError(new Error('Cannot find the configuration for the current NODE_ENV'), "NODE_ENV=" + envVal, errors.logError(new Error('Cannot find the configuration for the current NODE_ENV'), 'NODE_ENV=' + envVal,
'Ensure your config.js has a section for the current NODE_ENV value and is formatted properly.'); 'Ensure your config.js has a section for the current NODE_ENV value and is formatted properly.');
return when.reject(rejectMessage);
return when.reject(new Error('Unable to load config for NODE_ENV=' + envVal));
} }
// Check that our url is valid // Check that our url is valid
if (!validator.isURL(config.url, { protocols: ['http', 'https'], require_protocol: true })) { if (!validator.isURL(config.url, { protocols: ['http', 'https'], require_protocol: true })) {
errors.logError(new Error('Your site url in config.js is invalid.'), config.url, 'Please make sure this is a valid url before restarting'); errors.logError(new Error('Your site url in config.js is invalid.'), config.url, 'Please make sure this is a valid url before restarting');
return when.reject(rejectMessage);
return when.reject(new Error('invalid site url'));
} }
parsedUrl = url.parse(config.url || 'invalid', false, true); parsedUrl = url.parse(config.url || 'invalid', false, true);
if (/\/ghost(\/|$)/.test(parsedUrl.pathname)) { if (/\/ghost(\/|$)/.test(parsedUrl.pathname)) {
errors.logError(new Error('Your site url in config.js cannot contain a subdirectory called ghost.'), config.url, 'Please rename the subdirectory before restarting'); errors.logError(new Error('Your site url in config.js cannot contain a subdirectory called ghost.'), config.url, 'Please rename the subdirectory before restarting');
return when.reject(rejectMessage);
return when.reject(new Error('ghost subdirectory not allowed'));
} }
// Check that we have database values // Check that we have database values
if (!config.database || !config.database.client) { if (!config.database || !config.database.client) {
errors.logError(new Error('Your database configuration in config.js is invalid.'), JSON.stringify(config.database), 'Please make sure this is a valid Bookshelf database configuration'); errors.logError(new Error('Your database configuration in config.js is invalid.'), JSON.stringify(config.database), 'Please make sure this is a valid Bookshelf database configuration');
return when.reject(rejectMessage);
return when.reject(new Error('invalid database configuration'));
} }
hasHostAndPort = config.server && !!config.server.host && !!config.server.port; hasHostAndPort = config.server && !!config.server.host && !!config.server.port;
@ -98,7 +109,8 @@ function validateConfigEnvironment() {
// Check for valid server host and port values // Check for valid server host and port values
if (!config.server || !(hasHostAndPort || hasSocket)) { if (!config.server || !(hasHostAndPort || hasSocket)) {
errors.logError(new Error('Your server values (socket, or host and port) in config.js are invalid.'), JSON.stringify(config.server), 'Please provide them before restarting.'); errors.logError(new Error('Your server values (socket, or host and port) in config.js are invalid.'), JSON.stringify(config.server), 'Please provide them before restarting.');
return when.reject(rejectMessage);
return when.reject(new Error('invalid server configuration'));
} }
return when.resolve(config); return when.resolve(config);
@ -118,9 +130,10 @@ function loadConfig(configFilePath) {
if (!configExists) { if (!configExists) {
pendingConfig = writeConfigFile(); pendingConfig = writeConfigFile();
} }
when(pendingConfig).then(validateConfigEnvironment).then(function (rawConfig) { when(pendingConfig).then(validateConfigEnvironment).then(function (rawConfig) {
return config.init(rawConfig).then(loaded.resolve); return config.init(rawConfig).then(loaded.resolve);
}).otherwise(loaded.reject); }).catch(loaded.reject);
}); });
return loaded.promise; return loaded.promise;

6
core/index.js
View file

@ -21,16 +21,16 @@ function startGhost(options) {
var ghost = require('./server'); var ghost = require('./server');
return ghost(options.app) return ghost(options.app)
.then(deferred.resolve) .then(deferred.resolve)
.otherwise(function (e) { .catch(function (err) {
// We don't return the rejected promise to stop // We don't return the rejected promise to stop
// the propagation of the rejection and just // the propagation of the rejection and just
// allow the user to manage what to do. // allow the user to manage what to do.
deferred.reject(e); deferred.reject(err);
}); });
} catch (e) { } catch (e) {
deferred.reject(e); deferred.reject(e);
} }
}); }).catch(deferred.reject);
return deferred.promise; return deferred.promise;
} }

30
core/test/unit/bootstrap_spec.js vendored
View file

@ -15,7 +15,6 @@ var should = require('should'),
describe('Bootstrap', function () { describe('Bootstrap', function () {
var sandbox, var sandbox,
rejectMessage = bootstrap.__get__('rejectMessage'),
overrideConfig = function (newConfig) { overrideConfig = function (newConfig) {
bootstrap.__set__('readConfigFile', sandbox.stub().returns( bootstrap.__set__('readConfigFile', sandbox.stub().returns(
_.extend({}, defaultConfig, newConfig) _.extend({}, defaultConfig, newConfig)
@ -30,7 +29,6 @@ describe('Bootstrap', function () {
afterEach(function () { afterEach(function () {
bootstrap = rewire('../../bootstrap'); bootstrap = rewire('../../bootstrap');
sandbox.restore(); sandbox.restore();
}); });
it('loads the config file if one exists', function (done) { it('loads the config file if one exists', function (done) {
@ -120,7 +118,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -134,7 +132,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -148,7 +146,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -162,7 +160,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -176,7 +174,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -189,7 +187,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -202,7 +200,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -215,7 +213,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -229,7 +227,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -243,7 +241,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -258,7 +256,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -294,7 +292,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -307,7 +305,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);
@ -320,7 +318,7 @@ describe('Bootstrap', function () {
done(expectedError); done(expectedError);
}).catch(function (err) { }).catch(function (err) {
should.exist(err); should.exist(err);
err.should.contain(rejectMessage); err.should.be.an.Error;
done(); done();
}).catch(done); }).catch(done);