Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00
Code Issues Activity

Disambiguate between error code & status code

Browse source 
refs #6526

- Change our errors to use `statusCode` for the status code (like res.statusCode)
- Use statusCode for anything that's supposed to be the statusCode, rather than an error idenfier/code
- Update all the tests that check the key
- Route tests don't need fixing as the status codes are still returned correctly
This commit is contained in:
Hannah Wolfe 2016-02-17 14:51:57 +00:00
parent 39b326aa6d
commit 02199c6b02
20 changed files with 79 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
core/server/errors/bad-request-error.js
View file

@ -4,7 +4,7 @@
function BadRequestError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 400;
this.statusCode = 400;
this.errorType = this.name;
}

2
core/server/errors/data-import-error.js
View file

@ -4,7 +4,7 @@
function DataImportError(message, offendingProperty, value) {
this.message = message;
this.stack = new Error().stack;
this.code = 500;
this.statusCode = 500;
this.errorType = this.name;
this.property = offendingProperty || undefined;
this.value = value || undefined;

2
core/server/errors/email-error.js
View file

@ -4,7 +4,7 @@
function EmailError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 500;
this.statusCode = 500;
this.errorType = this.name;
}

58
core/server/errors/index.js
View file

@ -34,6 +34,29 @@ function getConfigModule() {
return config;
}
function isValidErrorStatus(status) {
return _.isNumber(status) && status >= 400 && status < 600;
}
function getStatusCode(error) {
if (error.statusCode) {
return error.statusCode;
}
if (error.status && isValidErrorStatus(error.status)) {
error.statusCode = error.status;
return error.statusCode;
}
if (error.code && isValidErrorStatus(error.code)) {
error.statusCode = error.code;
return error.statusCode;
}
error.statusCode = 500;
return error.statusCode;
}
/**
* Basic error handling helpers
*/
@ -197,7 +220,7 @@ errors = {
var errorContent = {};
// TODO: add logic to set the correct status code
statusCode = errorItem.code || 500;
statusCode = getStatusCode(errorItem);
errorContent.message = _.isString(errorItem) ? errorItem :
(_.isObject(errorItem) ? errorItem.message : i18n.t('errors.errors.unknownApiError'));
@ -227,7 +250,7 @@ errors = {
if (error.code && (error.errno || error.detail)) {
error.db_error_code = error.code;
error.errorType = 'DatabaseError';
error.code = 500;
error.statusCode = 500;
return this.rejectError(error);
}
@ -243,7 +266,7 @@ errors = {
res.status(httpErrors.statusCode).json({errors: httpErrors.errors});
},
renderErrorPage: function (code, err, req, res, next) {
renderErrorPage: function (statusCode, err, req, res, next) {
/*jshint unused:false*/
var self = this,
defaultErrorTemplatePath = path.resolve(getConfigModule().paths.adminViews, 'user-error.hbs');
@ -281,17 +304,22 @@ errors = {
function renderErrorInt(errorView) {
var stack = null;
if (code !== 404 && process.env.NODE_ENV !== 'production' && err.stack) {
if (statusCode !== 404 && process.env.NODE_ENV !== 'production' && err.stack) {
stack = parseStack(err.stack);
}
res.status(code).render((errorView || 'error'), {
res.status(statusCode).render((errorView || 'error'), {
message: err.message || err,
code: code,
// We have to use code here, as it's the variable passed to the template
// And error templates can be customised... therefore this constitutes API
// In future I recommend we make this be used for a combo-version of statusCode & errorCode
code: statusCode,
// Adding this as being distinctly, the status code, as opposed to any other code see #6526
statusCode: statusCode,
stack: stack
}, function (templateErr, html) {
if (!templateErr) {
return res.status(code).send(html);
return res.status(statusCode).send(html);
}
// There was an error trying to render the error page, output the error
self.logError(templateErr, i18n.t('errors.errors.errorWhilstRenderingError'), i18n.t('errors.errors.errorTemplateHasError'));
@ -303,12 +331,12 @@ errors = {
'<p>' + i18n.t('errors.errors.encounteredError') + '</p>' +
'<pre>' + hbs.handlebars.Utils.escapeExpression(templateErr.message || templateErr) + '</pre>' +
'<br ><p>' + i18n.t('errors.errors.whilstTryingToRender') + '</p>' +
code + ' ' + '<pre>' + hbs.handlebars.Utils.escapeExpression(err.message || err) + '</pre>'
statusCode + ' ' + '<pre>' + hbs.handlebars.Utils.escapeExpression(err.message || err) + '</pre>'
);
});
}
if (code >= 500) {
if (statusCode >= 500) {
this.logError(err, i18n.t('errors.errors.renderingErrorPage'), i18n.t('errors.errors.caughtProcessingError'));
}
@ -334,10 +362,13 @@ errors = {
},
error500: function (err, req, res, next) {
var statusCode = getStatusCode(err),
returnErrors = [];
// 500 errors should never be cached
res.set({'Cache-Control': 'no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0'});
if (err.status === 404 || err.code === 404) {
if (statusCode === 404) {
return this.error404(req, res, next);
}
@ -345,11 +376,8 @@ errors = {
if (!err || !(err instanceof Error)) {
next();
}
errors.renderErrorPage(err.status || err.code || 500, err, req, res, next);
errors.renderErrorPage(statusCode, err, req, res, next);
} else {
var statusCode = 500,
returnErrors = [];
if (!_.isArray(err)) {
err = [].concat(err);
}
@ -357,8 +385,6 @@ errors = {
_.each(err, function (errorItem) {
var errorContent = {};
statusCode = errorItem.code || 500;
errorContent.message = _.isString(errorItem) ? errorItem :
(_.isObject(errorItem) ? errorItem.message : i18n.t('errors.errors.unknownError'));
errorContent.errorType = errorItem.errorType || 'InternalServerError';

2
core/server/errors/internal-server-error.js
View file

@ -4,7 +4,7 @@
function InternalServerError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 500;
this.statusCode = 500;
this.errorType = this.name;
}

2
core/server/errors/method-not-allowed-error.js
View file

@ -4,7 +4,7 @@
function MethodNotAllowedError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 405;
this.statusCode = 405;
this.errorType = this.name;
}

2
core/server/errors/no-permission-error.js
View file

@ -4,7 +4,7 @@
function NoPermissionError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 403;
this.statusCode = 403;
this.errorType = this.name;
}

2
core/server/errors/not-found-error.js
View file

@ -4,7 +4,7 @@
function NotFoundError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 404;
this.statusCode = 404;
this.errorType = this.name;
}

2
core/server/errors/request-too-large-error.js
View file

@ -4,7 +4,7 @@
function RequestEntityTooLargeError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 413;
this.statusCode = 413;
this.errorType = this.name;
}

2
core/server/errors/too-many-requests-error.js
View file

@ -4,7 +4,7 @@
function TooManyRequestsError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 429;
this.statusCode = 429;
this.errorType = this.name;
}

2
core/server/errors/unauthorized-error.js
View file

@ -4,7 +4,7 @@
function UnauthorizedError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 401;
this.statusCode = 401;
this.errorType = this.name;
}

2
core/server/errors/unsupported-media-type-error.js
View file

@ -4,7 +4,7 @@
function UnsupportedMediaTypeError(message) {
this.message = message;
this.stack = new Error().stack;
this.code = 415;
this.statusCode = 415;
this.errorType = this.name;
}

2
core/server/errors/validation-error.js
View file

@ -4,7 +4,7 @@
function ValidationError(message, offendingProperty) {
this.message = message;
this.stack = new Error().stack;
this.code = 422;
this.statusCode = 422;
if (offendingProperty) {
this.property = offendingProperty;
}

2
core/server/helpers/ghost_head.js
View file

@ -73,7 +73,7 @@ function getAjaxHelper(clientId, clientSecret) {
function ghost_head(options) {
// if error page do nothing
if (this.code >= 400) {
if (this.statusCode >= 400) {
return;
}
var metaData = getMetaData(this, options.data.root),

16
core/test/integration/api/api_authentication_spec.js
View file

@ -97,7 +97,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('NoPermissionError');
err.code.should.equal(403);
err.statusCode.should.equal(403);
done();
});
@ -110,7 +110,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('NoPermissionError');
err.code.should.equal(403);
err.statusCode.should.equal(403);
done();
});
@ -123,7 +123,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('NoPermissionError');
err.code.should.equal(403);
err.statusCode.should.equal(403);
done();
});
@ -156,7 +156,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('NoPermissionError');
err.code.should.equal(403);
err.statusCode.should.equal(403);
done();
});
@ -169,7 +169,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('UnauthorizedError');
err.code.should.equal(401);
err.statusCode.should.equal(401);
err.message.should.equal('Invalid token structure');
done();
});
@ -191,7 +191,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('UnauthorizedError');
err.code.should.equal(401);
err.statusCode.should.equal(401);
err.message.should.equal('Invalid token structure');
done();
});
@ -226,7 +226,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('NoPermissionError');
err.code.should.equal(403);
err.statusCode.should.equal(403);
done();
});
@ -259,7 +259,7 @@ describe('Authentication API', function () {
should.exist(err);
err.name.should.equal('NoPermissionError');
err.code.should.equal(403);
err.statusCode.should.equal(403);
done();
});

4
core/test/integration/api/api_posts_spec.js
View file

@ -263,7 +263,7 @@ describe('Post API', function () {
}).catch(function (err) {
should.exist(err);
err.message.should.eql('Validation (isSlug) failed for tag');
err.code.should.eql(422);
err.statusCode.should.eql(422);
done();
});
});
@ -274,7 +274,7 @@ describe('Post API', function () {
}).catch(function (err) {
should.exist(err);
err.message.should.eql('Validation (isSlug) failed for author');
err.code.should.eql(422);
err.statusCode.should.eql(422);
done();
});
});

6
core/test/integration/api/api_upload_spec.js
View file

@ -42,7 +42,7 @@ describe('Upload API', function () {
UploadAPI.add({uploadimage: uploadimage}).then(function () {
done(new Error('Upload suceeded with invalid file.'));
}, function (result) {
result.code.should.equal(415);
result.statusCode.should.equal(415);
result.errorType.should.equal('UnsupportedMediaTypeError');
done();
});
@ -59,7 +59,7 @@ describe('Upload API', function () {
UploadAPI.add({uploadimage: uploadimage}).then(function () {
done(new Error('Upload suceeded with invalid file.'));
}, function (result) {
result.code.should.equal(415);
result.statusCode.should.equal(415);
result.errorType.should.equal('UnsupportedMediaTypeError');
done();
});
@ -88,7 +88,7 @@ describe('Upload API', function () {
UploadAPI.add({uploadimage: uploadimage}).then(function () {
done(new Error('Upload suceeded with invalid file.'));
}, function (result) {
result.code.should.equal(415);
result.statusCode.should.equal(415);
result.errorType.should.equal('UnsupportedMediaTypeError');
done();
});

2
core/test/unit/controllers/frontend/channels_spec.js
View file

@ -97,7 +97,7 @@ describe('Channels', function () {
function testChannel404(props, done) {
testChannelError(props, function (error) {
error.errorType.should.eql('NotFoundError');
error.code.should.eql(404);
error.statusCode.should.eql(404);
}, done);
}

12
core/test/unit/error_handling_spec.js
View file

@ -358,6 +358,7 @@ describe('Error handling', function () {
// Test that the message is correct
options.message.should.equal('Page not found');
// Template variable
options.code.should.equal(404);
this.statusCode.should.equal(404);
@ -389,6 +390,7 @@ describe('Error handling', function () {
// Test that the message is correct
options.message.should.equal('Page not found');
// Template variable
options.code.should.equal(404);
this.statusCode.should.equal(404);
@ -421,6 +423,7 @@ describe('Error handling', function () {
// Test that the message is correct
options.message.should.equal('I am a big bad error');
// Template variable
options.code.should.equal(500);
this.statusCode.should.equal(500);
@ -452,6 +455,7 @@ describe('Error handling', function () {
// Test that the message is correct
options.message.should.equal('I am a big bad error');
// Template variable
options.code.should.equal(500);
this.statusCode.should.equal(500);
@ -469,18 +473,18 @@ describe('Error handling', function () {
return res;
});
err.code = 500;
err.statusCode = 500;
errors.error500(err, req, res, null);
});
it('Renders custom error template if one exists', function (done) {
var code = 404,
var statusCode = 404,
error = {message: 'Custom view test'},
req = {
session: null
},
res = {
status: function (code) {
status: function (statusCode) {
/*jshint unused:false*/
return this;
},
@ -493,7 +497,7 @@ describe('Error handling', function () {
},
next = null;
errors.updateActiveTheme('theme-with-error');
errors.renderErrorPage(code, error, req, res, next);
errors.renderErrorPage(statusCode, error, req, res, next);
});
});
});

4
core/test/unit/rss_spec.js
View file

@ -417,7 +417,7 @@ describe('RSS', function () {
rss(req, res, function (err) {
should.exist(err);
err.code.should.eql(404);
err.statusCode.should.eql(404);
res.redirect.called.should.be.false();
res.render.called.should.be.false();
done();
@ -434,7 +434,7 @@ describe('RSS', function () {
rss(req, res, function (err) {
should.exist(err);
err.code.should.eql(404);
err.statusCode.should.eql(404);
res.redirect.called.should.be.false();
res.render.called.should.be.false();
done();