Skip to content

Commit

Permalink
Improved error messaging
Browse files Browse the repository at this point in the history 
closes TryGhost#748

- Removed the alpha software warning
- Better error message output for the whole app - can now specify an error, a context, and a help message
- Improved invalid node version, start and stop messaging
- Listens for Ctrl+C and exits nicely
- Minor improvements to handling and errors with old DBs (temporary)
  • Loading branch information
@ErisDS
ErisDS committed Sep 15, 2013
1 parent 9fa659a commit 71a9219
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 66 deletions.
59 changes: 43 additions & 16 deletions core/server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,27 +256,54 @@ when.all([ghost.init(), helpers.loadCoreHelpers(ghost)]).then(function () {
// Tell users if their node version is not supported, and exit
if (!semver.satisfies(process.versions.node, packageInfo.engines.node)) {
console.log(
"\n !!! INVALID NODE VERSION !!!\n".red,
"Ghost requires node version".red,
"\nERROR: Unsupported version of Node".red,
"\nGhost needs Node version".red,
packageInfo.engines.node.yellow,
"as defined in package.json\n".red
"you are using version".red,
process.versions.node.yellow,
"\nPlease go to http://nodejs.org to get the latest version".green
);

process.exit(-1);
process.exit(0);
}

// Alpha warning, reminds users this is not production-ready software (yet)
// Remove once software becomes suitably 'ready'
console.log(
"\n !!! ALPHA SOFTWARE WARNING !!!\n".red,
"Ghost is in the early stages of development.\n".red,
"Expect to see bugs and other issues (but please report them.)\n".red
);

// Startup message
console.log("Express server listening on address:",
ghost.config().server.host + ':'
+ ghost.config().server.port);
// Startup & Shutdown messages
if (process.env.NODE_ENV === 'production') {
console.log(
"Ghost is running...".green,
"\nYour blog is now available on",
ghost.config().url,
"\nCtrl+C to shut down".grey
);

// ensure that Ghost exits correctly on Ctrl+C
process.on('SIGINT', function () {
console.log(
"\nGhost has shut down".red,
"\nYour blog is now offline"
);
process.exit(0);
});
} else {
console.log(
"Ghost is running...".green,
"\nListening on",
ghost.config().server.host + ':' + ghost.config().server.port,
"\nUrl configured as:",
ghost.config().url,
"\nCtrl+C to shut down".grey
);
// ensure that Ghost exits correctly on Ctrl+C
process.on('SIGINT', function () {
console.log(
"\nGhost has shutdown".red,
"\nGhost was running for",
Math.round(process.uptime()),
"seconds"
);
process.exit(0);
});
}

// Let everyone know we have finished loading
loading.resolve();
Expand Down
15 changes: 10 additions & 5 deletions core/server/data/migration/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,18 @@ function getDatabaseVersion() {
.select('value')
.then(function (versions) {
var databaseVersion = _.reduce(versions, function (memo, version) {
if (isNaN(version.value)) {
errors.throwError('Database version is not recognised');
}
return parseInt(version.value, 10) > parseInt(memo, 10) ? version.value : memo;
}, initialVersion);

if (!databaseVersion || databaseVersion.length === 0) {
// we didn't get a response we understood, assume initialVersion
databaseVersion = initialVersion;
}
return when.resolve(databaseVersion);

return databaseVersion;
});
}
return when.reject('Settings table does not exist');
Expand Down Expand Up @@ -88,8 +92,10 @@ module.exports = {
if (databaseVersion > defaultVersion) {
// 3. The database exists but the currentVersion setting does not or cannot be understood
// In this case we don't understand the version because it is too high
errors.logError('Database is not compatible with software version.');
process.exit(-3);
errors.logErrorAndExit(
'Your database is not compatible with this version of Ghost',
'You will need to create a new database'
);
}

}, function (err) {
Expand All @@ -101,8 +107,7 @@ module.exports = {

// 3. The database exists but the currentVersion setting does not or cannot be understood
// In this case the setting was missing or there was some other problem
errors.logError('Database is not recognisable.' + err);
process.exit(-2);
errors.logErrorAndExit('There is a problem with the database', err.message || err);
});
},

Expand Down
40 changes: 25 additions & 15 deletions core/server/errorHandling.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
var _ = require('underscore'),
colors = require("colors"),
errors;

/**
Expand All @@ -17,35 +18,44 @@ errors = {
throw err;
},

logError: function (err) {
err = err || "Unknown";
logError: function (err, context, help) {
err = err.message || err || "Unknown";
// TODO: Logging framework hookup
// Eventually we'll have better logging which will know about envs
if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'staging'
|| process.env.NODE_ENV === 'production') {
console.log("Error occurred: ", err.message || err, err.stack || "");

console.log("\nERROR:".red, err.red, err.stack || "");

if (context) {
console.log(context);
}

if (help) {
console.log(help.green);
}
// add a new line
console.log("");
}
},

logAndThrowError: function (err) {
this.logError(err);

this.throwError(err);
logErrorAndExit: function (err, context, help) {
this.logError(err, context, help);
// Exit with 0 to prevent npm errors as we have our own
process.exit(0);
},

logErrorWithMessage: function (msg) {
var self = this;
logAndThrowError: function (err, context, help) {
this.logError(err, context, help);

return function () {
self.logError(msg);
};
this.throwError(err, context, help);
},

logErrorWithRedirect: function (msg, redirectTo, req, res) {
logErrorWithRedirect: function (msg, context, help, redirectTo, req, res) {
var self = this;

return function () {
self.logError(msg);
self.logError(msg, context, help);

if (_.isFunction(res.redirect)) {
res.redirect(redirectTo);
Expand All @@ -55,6 +65,6 @@ errors = {
};

// Ensure our 'this' context in the functions
_.bindAll(errors, "throwError", "logError", "logAndThrowError", "logErrorWithMessage", "logErrorWithRedirect");
_.bindAll(errors, "throwError", "logError", "logAndThrowError", "logErrorWithRedirect");

module.exports = errors;
5 changes: 5 additions & 0 deletions core/server/models/settings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ Settings = GhostBookshelf.Model.extend({

_.each(defaultSettings, function (defaultSetting, defaultSettingKey) {
var isMissingFromDB = usedKeys.indexOf(defaultSettingKey) === -1;
// Temporary code to deal with old databases with currentVersion settings
// TODO: remove before release
if (defaultSettingKey === 'databaseVersion' && usedKeys.indexOf('currentVersion') !== -1) {
isMissingFromDB = false;
}
if (isMissingFromDB) {
defaultSetting.value = defaultSetting.defaultValue;
insertOperations.push(Settings.forge(defaultSetting).save());
Expand Down
35 changes: 5 additions & 30 deletions core/test/unit/errorHandling_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ var testUtils = require('./testUtils'),
sinon = require('sinon'),

// Stuff we are testing
colors = require("colors"),
errors = require('../../server/errorHandling'),
// storing current environment
currentEnv = process.env.NODE_ENV;



describe("Error handling", function () {

// Just getting rid of jslint unused error
Expand Down Expand Up @@ -51,7 +50,7 @@ describe("Error handling", function () {
errors.logError(err);

// Calls log with message on Error objects
logStub.calledWith("Error occurred: ", err.message).should.equal(true);
logStub.calledWith("\nERROR:".red, err.message.red).should.equal(true);

logStub.reset();

Expand All @@ -60,37 +59,13 @@ describe("Error handling", function () {
errors.logError(err);

// Calls log with string on strings
logStub.calledWith("Error occurred: ", err).should.equal(true);
logStub.calledWith("\nERROR:".red, err.red).should.equal(true);

logStub.restore();
process.env.NODE_ENV = currentEnv;

});

it("logs promise errors with custom messages", function (done) {
var def = when.defer(),
prom = def.promise,
logStub = sinon.stub(console, "log");

// give environment a value that will console log
process.env.NODE_ENV = "development";
prom.then(function () {
throw new Error("Ran success handler");
}, errors.logErrorWithMessage("test1"));

prom.otherwise(function () {
logStub.calledWith("Error occurred: ", "test1").should.equal(true);
logStub.restore();

done();
});
prom.ensure(function () {
// gives the environment the correct value back
process.env.NODE_ENV = currentEnv;
});
def.reject();
});

it("logs promise errors and redirects", function (done) {
var def = when.defer(),
prom = def.promise,
Expand All @@ -107,10 +82,10 @@ describe("Error handling", function () {
process.env.NODE_ENV = "development";
prom.then(function () {
throw new Error("Ran success handler");
}, errors.logErrorWithRedirect("test1", "/testurl", req, res));
}, errors.logErrorWithRedirect("test1", null, null, "/testurl", req, res));

prom.otherwise(function () {
logStub.calledWith("Error occurred: ", "test1").should.equal(true);
logStub.calledWith("\nERROR:".red, "test1".red).should.equal(true);
logStub.restore();

redirectStub.calledWith('/testurl').should.equal(true);
Expand Down

0 comments on commit 71a9219

Please sign in to comment.