2
1
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2023-12-13 21:00:40 +01:00

Change error message response

closes #2643
- added error type
- added error property for validations
- wrapped errors in an array
- returns multiple errors for validation
- updated tests and admin
This commit is contained in:
Sebastian Gierlinger 2014-05-05 15:51:21 +02:00
parent acd71d423e
commit 39e654e9c3
19 changed files with 168 additions and 93 deletions

View file

@ -96,7 +96,14 @@
if (request.status !== 200) {
try {
// Try to parse out the error, or default to "Unknown"
message = request.responseJSON.error || "Unknown Error";
if (request.responseJSON.errors && _.isArray(request.responseJSON.errors)) {
message = '';
_.each(request.responseJSON.errors, function (errorItem) {
message += '<br/>' + errorItem.message;
});
} else {
message = request.responseJSON.error || "Unknown Error";
}
} catch (e) {
msgDetail = request.status ? request.status + " - " + request.statusText : "Server was not available";
message = "The server returned an error (" + msgDetail + ").";

View file

@ -53,7 +53,7 @@
error: function (response) {
$('#startupload').text('Import');
var responseJSON = response.responseJSON,
message = responseJSON && responseJSON.error ? responseJSON.error : 'unknown';
message = responseJSON && responseJSON.errors[0].message ? responseJSON.errors[0].message : 'unknown';
Ghost.notifications.addItem({
type: 'error',
message: ['A problem was encountered while importing new content to your blog. Error: ', message].join(''),
@ -123,7 +123,7 @@
},
error: function onError(response) {
var responseText = JSON.parse(response.responseText),
message = responseText && responseText.error ? responseText.error : 'unknown';
message = responseText && responseText.errors[0].message ? responseText.errors[0].message : 'unknown';
Ghost.notifications.addItem({
type: 'error',
message: ['A problem was encountered while deleting content from your blog. Error: ', message].join(''),
@ -175,7 +175,7 @@
},
error: function onError(response) {
var responseText = JSON.parse(response.responseText),
message = responseText && responseText.error ? responseText.error : 'unknown';
message = responseText && responseText.errors[0].message ? responseText.errors[0].message : 'unknown';
Ghost.notifications.addItem({
type: 'error',
message: ['A problem was encountered while sending the test email: ', message].join(''),

View file

@ -21,10 +21,10 @@ db = {
// Export data, otherwise send error 500
return canThis(self.user).exportContent.db().then(function () {
return dataExport().otherwise(function (error) {
return when.reject({errorCode: 500, message: error.message || error});
return when.reject({type: 'InternalServerError', message: error.message || error});
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to export data. (no rights)'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to export data. (no rights)'});
});
},
'importContent': function (options) {
@ -41,7 +41,7 @@ db = {
* - If there is no path
* - If the name doesn't have json in it
*/
return when.reject({code: 500, message: 'Please select a .json file to import.'});
return when.reject({type: 'InternalServerError', message: 'Please select a .json file to import.'});
}
return api.settings.read({ key: 'databaseVersion' }).then(function (response) {
@ -92,13 +92,13 @@ db = {
}).then(function () {
return when.resolve({message: 'Posts, tags and other data successfully imported'});
}).otherwise(function importFailure(error) {
return when.reject({code: 500, message: error.message || error});
return when.reject({type: 'InternalServerError', message: error.message || error});
}).finally(function () {
// Unlink the file after import
return nodefn.call(fs.unlink, options.importfile.path);
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to export data. (no rights)'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to export data. (no rights)'});
});
},
'deleteAllContent': function () {
@ -109,10 +109,10 @@ db = {
.then(function () {
return when.resolve({message: 'Successfully deleted all content from your blog.'});
}, function (error) {
return when.reject({code: 500, message: error.message || error});
return when.reject({message: error.message || error});
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to export data. (no rights)'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to export data. (no rights)'});
});
}
};

View file

@ -12,7 +12,34 @@ var _ = require('lodash'),
tags = require('./tags'),
mail = require('./mail'),
requestHandler,
init;
init,
errorTypes = {
BadRequest: {
code: 400
},
Unauthorized: {
code: 401
},
NoPermission: {
code: 403
},
NotFound: {
code: 404
},
RequestEntityTooLarge: {
code: 413
},
ValidationError: {
code: 422
},
EmailError: {
code: 500
},
InternalServerError: {
code: 500
}
};
// ## Request Handlers
@ -112,9 +139,25 @@ requestHandler = function (apiMethod) {
});
});
}, function (error) {
var errorCode = error.code || 500,
errorMsg = {error: _.isString(error) ? error : (_.isObject(error) ? error.message : 'Unknown API Error')};
res.json(errorCode, errorMsg);
var errorCode,
errors = [];
if (!_.isArray(error)) {
error = [].concat(error);
}
_.each(error, function (erroritem) {
var errorContent = {};
//TODO: add logic to set the correct status code
errorCode = errorTypes[erroritem.type].code || 500;
errorContent['message'] = _.isString(erroritem) ? erroritem : (_.isObject(erroritem) ? erroritem.message : 'Unknown API Error');
errorContent['type'] = erroritem.type || 'InternalServerError';
errors.push(errorContent);
});
res.json(errorCode, {errors: errors});
});
};
};

View file

@ -19,10 +19,10 @@ mail = {
// **returns:** a promise from the mailer with the number of successfully sent emails
return mailer.send(message)
.then(function (data) {
return when.resolve({ code: 200, message: data.message });
return when.resolve({message: data.message });
})
.otherwise(function (error) {
return when.reject({ code: 500, message: error.message });
return when.reject({type: 'EmailError', message: error.message });
});
},

View file

@ -9,7 +9,7 @@ var when = require('when'),
function checkPostData(postData) {
if (_.isEmpty(postData) || _.isEmpty(postData.posts) || _.isEmpty(postData.posts[0])) {
return when.reject({code: 400, message: 'No root key (\'posts\') provided.'});
return when.reject({type: 'BadRequest', message: 'No root key (\'posts\') provided.'});
}
return when.resolve(postData);
}
@ -86,7 +86,7 @@ posts = {
return { posts: [ omitted ]};
}
return when.reject({code: 404, message: 'Post not found'});
return when.reject({type: 'NotFound', message: 'Post not found.'});
});
},
@ -119,10 +119,10 @@ posts = {
return { posts: [ omitted ]};
}
return when.reject({code: 404, message: 'Post not found'});
return when.reject({type: 'NotFound', message: 'Post not found.'});
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to edit this post.'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to edit this post.'});
});
},
@ -152,7 +152,7 @@ posts = {
return { posts: [ omitted ]};
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to add posts.'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to add posts.'});
});
},
@ -177,7 +177,7 @@ posts = {
});
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to remove posts.'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to remove posts.'});
});
},
@ -190,10 +190,10 @@ posts = {
if (slug) {
return slug;
}
return when.reject({code: 500, message: 'Could not generate slug'});
return when.reject({type: 'InternalServerError', message: 'Could not generate slug'});
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission.'});
return when.reject({type: 'NoPermission', message: 'You do not have permission.'});
});
}

View file

@ -170,7 +170,7 @@ settings = {
result = {};
if (!setting) {
return when.reject({code: 404, message: 'Unable to find setting: ' + options.key});
return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + options.key});
}
result[options.key] = setting;
@ -205,16 +205,16 @@ settings = {
}).otherwise(function (error) {
return dataProvider.Settings.read(key.key).then(function (result) {
if (!result) {
return when.reject({code: 404, message: 'Unable to find setting: ' + key});
return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + key + '.'});
}
return when.reject({message: error.message, stack: error.stack});
return when.reject({type: 'InternalServerError', message: error.message});
});
});
}
return dataProvider.Settings.read(key).then(function (setting) {
if (!setting) {
return when.reject({code: 404, message: 'Unable to find setting: ' + key});
return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + key + '.'});
}
if (!_.isString(value)) {
value = JSON.stringify(value);

View file

@ -37,7 +37,7 @@ users = {
return { users: omitted };
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to browse users.'});
return when.reject({type: 'NotFound', message: 'You do not have permission to browse users.'});
});
},
@ -55,7 +55,7 @@ users = {
return { users: [omitted] };
}
return when.reject({code: 404, message: 'User not found'});
return when.reject({type: 'NotFound', message: 'User not found.'});
});
},
@ -72,10 +72,10 @@ users = {
var omitted = _.omit(result.toJSON(), filteredAttributes);
return { users: [omitted]};
}
return when.reject({code: 404, message: 'User not found'});
return when.reject({type: 'NotFound', message: 'User not found.'});
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to edit this user.'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to edit this users.'});
});
},
@ -99,7 +99,7 @@ users = {
}
});
}, function () {
return when.reject({code: 403, message: 'You do not have permission to add a users.'});
return when.reject({type: 'NoPermission', message: 'You do not have permission to add a users.'});
});
},

View file

@ -160,9 +160,8 @@ frontendControllers = {
if (permalink.match(path) === false) {
// If there are still no matches then return.
if (staticPostPermalink.match(path) === false) {
// Throw specific error
// to break out of the promise chain.
throw new Error('no match');
// Reject promise chain with type 'NotFound'
return when.reject({type: 'NotFound'});
}
permalink = staticPostPermalink;
@ -192,8 +191,8 @@ frontendControllers = {
if (params.edit === 'edit') {
return res.redirect(config().paths.subdir + '/ghost/editor/' + post.id + '/');
} else if (params.edit !== undefined) {
// Use throw 'no match' to show 404.
throw new Error('no match');
// reject with type: 'NotFound'
return when.reject({type: 'NotFound'});
}
setReqCtx(req, post);
@ -249,13 +248,13 @@ frontendControllers = {
return next();
}
render();
return render();
}).otherwise(function (err) {
// If we've thrown an error message
// of 'no match' then we found
// of type: 'NotFound' then we found
// no path match.
if (err.message === 'no match') {
if (err.type === 'NotFound') {
return next();
}

View file

@ -232,30 +232,25 @@ Importer000.prototype.basicImport = function (data) {
// Write changes to DB, if successful commit, otherwise rollback
// when.all() does not work as expected, when.settle() does.
when.settle(ops).then(function (descriptors) {
var rej = false,
error = '';
var errors = [];
descriptors.forEach(function (d) {
if (d.state === 'rejected') {
error += _.isEmpty(error) ? '' : '</br>';
if (!_.isEmpty(d.reason.clientError)) {
error += d.reason.clientError;
} else if (!_.isEmpty(d.reason.message)) {
error += d.reason.message;
}
rej = true;
errors = errors.concat(d.reason);
}
});
if (!rej) {
if (errors.length === 0) {
t.commit();
} else {
t.rollback(error);
t.rollback(errors);
}
});
}).then(function () {
//TODO: could return statistics of imported items
return when.resolve();
}, function (error) {
return when.reject("Error importing data: " + error);
return when.reject(error);
});
};

View file

@ -1,6 +1,7 @@
var schema = require('../schema').tables,
_ = require('lodash'),
validator = require('validator'),
when = require('when'),
validateSchema,
validateSettings,
@ -20,14 +21,17 @@ validator.extend('notContains', function (str, badString) {
// values are checked against the validation objects
// form schema.js
validateSchema = function (tableName, model) {
var columns = _.keys(schema[tableName]);
var columns = _.keys(schema[tableName]),
errors = [];
_.each(columns, function (columnKey) {
var message = '';
// check nullable
if (model.hasOwnProperty(columnKey) && schema[tableName][columnKey].hasOwnProperty('nullable')
&& schema[tableName][columnKey].nullable !== true) {
if (validator.isNull(model[columnKey]) || validator.empty(model[columnKey])) {
throw new Error('Value in [' + tableName + '.' + columnKey + '] cannot be blank.');
message = 'Value in [' + tableName + '.' + columnKey + '] cannot be blank.';
errors.push({type: 'ValidationError', property: tableName + '.' + columnKey, message: message});
}
}
// TODO: check if mandatory values should be enforced
@ -35,24 +39,30 @@ validateSchema = function (tableName, model) {
// check length
if (schema[tableName][columnKey].hasOwnProperty('maxlength')) {
if (!validator.isLength(model[columnKey], 0, schema[tableName][columnKey].maxlength)) {
throw new Error('Value in [' + tableName + '.' + columnKey +
'] exceeds maximum length of ' + schema[tableName][columnKey].maxlength + ' characters.');
message = 'Value in [' + tableName + '.' + columnKey + '] exceeds maximum length of '
+ schema[tableName][columnKey].maxlength + ' characters.';
errors.push({type: 'ValidationError', property: tableName + '.' + columnKey, message: message});
}
}
//check validations objects
if (schema[tableName][columnKey].hasOwnProperty('validations')) {
validate(model[columnKey], columnKey, schema[tableName][columnKey].validations);
errors.concat(validate(model[columnKey], columnKey, schema[tableName][columnKey].validations));
}
//check type
if (schema[tableName][columnKey].hasOwnProperty('type')) {
if (schema[tableName][columnKey].type === 'integer' && !validator.isInt(model[columnKey])) {
throw new Error('Value in [' + tableName + '.' + columnKey + '] is no valid integer.');
message = 'Value in [' + tableName + '.' + columnKey + '] is no valid integer.';
errors.push({type: 'ValidationError', property: tableName + '.' + columnKey, message: message});
}
}
}
});
if (errors.length !== 0) {
return when.reject(errors);
}
};
// Validation for settings
@ -63,7 +73,7 @@ validateSettings = function (defaultSettings, model) {
matchingDefault = defaultSettings[values.key];
if (matchingDefault && matchingDefault.validations) {
validate(values.value, values.key, matchingDefault.validations);
return validate(values.value, values.key, matchingDefault.validations);
}
};
@ -85,6 +95,7 @@ validateSettings = function (defaultSettings, model) {
//
// available validators: https://github.com/chriso/validator.js#validators
validate = function (value, key, validations) {
var errors = [];
_.each(validations, function (validationOptions, validationName) {
var goodResult = true;
@ -99,11 +110,15 @@ validate = function (value, key, validations) {
// equivalent of validator.isSomething(option1, option2)
if (validator[validationName].apply(validator, validationOptions) !== goodResult) {
throw new Error('Settings validation (' + validationName + ') failed for ' + key);
errors.push({type: 'ValidationError', property: key, message: 'Settings validation (' + validationName + ') failed for ' + key});
}
validationOptions.shift();
}, this);
if (errors.length !== 0) {
return when.reject(errors);
}
};
module.exports = {

View file

@ -55,7 +55,7 @@ function ghostBusBoy(req, res, next) {
busboy.on('limit', function () {
hasError = true;
res.send(413, {code: 413, message: 'File size limit breached.'});
res.send(413, {type: 'RequestEntityTooLarge', message: 'File size limit breached.'});
});
busboy.on('error', function (error) {

View file

@ -50,7 +50,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
},
validate: function () {
validation.validateSchema(this.tableName, this.toJSON());
return validation.validateSchema(this.tableName, this.toJSON());
},
creating: function (newObj, attr, options) {

View file

@ -41,8 +41,10 @@ Settings = ghostBookshelf.Model.extend({
},
validate: function () {
validation.validateSchema(this.tableName, this.toJSON());
validation.validateSettings(defaultSettings, this);
var self = this;
return when(validation.validateSchema(self.tableName, self.toJSON())).then(function () {
return validation.validateSettings(defaultSettings, self);
});
},
saving: function () {
@ -74,7 +76,7 @@ Settings = ghostBookshelf.Model.extend({
// Accept an array of models as input
if (item.toJSON) { item = item.toJSON(); }
if (!(_.isString(item.key) && item.key.length > 0)) {
return when.reject(new Error('Setting key cannot be empty.'));
return when.reject({type: 'ValidationError', message: 'Setting key cannot be empty.'});
}
return Settings.forge({ key: item.key }).fetch(options).then(function (setting) {
@ -82,7 +84,7 @@ Settings = ghostBookshelf.Model.extend({
return setting.save({value: item.value}, options);
}
return when.reject(new Error('Unable to find setting to update: ' + item.key));
return when.reject({type: 'NotFound', message: 'Unable to find setting to update: ' + item.key});
}, errors.logAndThrowError);
});

View file

@ -273,7 +273,8 @@ describe('Post API', function () {
res.should.be.json;
var jsonResponse = res.body;
jsonResponse.should.exist;
testUtils.API.checkResponseValue(jsonResponse, ['error']);
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});
@ -290,7 +291,8 @@ describe('Post API', function () {
res.should.be.json;
var jsonResponse = res.body;
jsonResponse.should.exist;
testUtils.API.checkResponseValue(jsonResponse, ['error']);
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});
@ -307,7 +309,8 @@ describe('Post API', function () {
res.should.be.json;
var jsonResponse = res.body;
jsonResponse.should.exist;
testUtils.API.checkResponseValue(jsonResponse, ['error']);
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});
@ -597,7 +600,9 @@ describe('Post API', function () {
var putBody = res.body;
_.has(res.headers, 'x-cache-invalidate').should.equal(false);
res.should.be.json;
testUtils.API.checkResponseValue(putBody, ['error']);
jsonResponse = res.body;
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});
@ -637,7 +642,8 @@ describe('Post API', function () {
res.should.be.json;
var jsonResponse = res.body;
jsonResponse.should.exist;
testUtils.API.checkResponseValue(jsonResponse, ['error']);
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});

View file

@ -61,7 +61,6 @@ describe('Settings API', function () {
if (err) {
return done(err);
}
// console.log('/ghost/', err, res);
csrfToken = res.text.match(pattern_meta)[1];
done();
});
@ -133,7 +132,8 @@ describe('Settings API', function () {
res.should.be.json;
var jsonResponse = res.body;
jsonResponse.should.exist;
testUtils.API.checkResponseValue(jsonResponse, ['error']);
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});
@ -222,10 +222,11 @@ describe('Settings API', function () {
return done(err);
}
var putBody = res.body;
jsonResponse = res.body;
should.not.exist(res.headers['x-cache-invalidate']);
res.should.be.json;
testUtils.API.checkResponseValue(putBody, ['error']);
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});

View file

@ -59,7 +59,6 @@ describe('User API', function () {
if (err) {
return done(err);
}
// console.log('/ghost/', err, res);
csrfToken = res.text.match(pattern_meta)[1];
done();
});
@ -131,8 +130,8 @@ describe('User API', function () {
res.should.be.json;
var jsonResponse = res.body;
jsonResponse.should.exist;
testUtils.API.checkResponseValue(jsonResponse, ['error']);
jsonResponse.errors.should.exist;
testUtils.API.checkResponseValue(jsonResponse.errors[0], ['message', 'type']);
done();
});
});

View file

@ -64,17 +64,17 @@ describe('DB API', function () {
}).then(function (){
done(new Error("Delete all content is not denied for editor."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
return dbAPI.deleteAllContent.call({user: 3});
}).then(function (){
done(new Error("Delete all content is not denied for author."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
return dbAPI.deleteAllContent();
}).then(function (){
done(new Error("Delete all content is not denied without authentication."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
done();
});
});
@ -85,17 +85,17 @@ describe('DB API', function () {
}).then(function (){
done(new Error("Export content is not denied for editor."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
return dbAPI.exportContent.call({user: 3});
}).then(function (){
done(new Error("Export content is not denied for author."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
return dbAPI.exportContent();
}).then(function (){
done(new Error("Export content is not denied without authentication."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
done();
});
});
@ -106,17 +106,17 @@ describe('DB API', function () {
}).then(function (result){
done(new Error("Import content is not denied for editor."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
return dbAPI.importContent.call({user: 3});
}).then(function (result){
done(new Error("Import content is not denied for author."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
return dbAPI.importContent();
}).then(function (result){
done(new Error("Import content is not denied without authentication."));
}, function (error) {
error.code.should.eql(403);
error.type.should.eql('NoPermission');
done();
});
});

View file

@ -257,7 +257,9 @@ describe("Import", function () {
}).then(function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}, function (error) {
error.should.eql('Error importing data: Value in [posts.title] exceeds maximum length of 150 characters.');
error[0].message.should.eql('Value in [posts.title] exceeds maximum length of 150 characters.');
error[0].type.should.eql('ValidationError');
when.all([
knex("users").select(),
@ -303,7 +305,9 @@ describe("Import", function () {
}).then(function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}, function (error) {
error.should.eql('Error importing data: Setting key cannot be empty.');
error[0].message.should.eql('Setting key cannot be empty.');
error[0].type.should.eql('ValidationError');
when.all([
knex("users").select(),
@ -440,7 +444,9 @@ describe("Import", function () {
}).then(function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}, function (error) {
error.should.eql('Error importing data: Value in [posts.title] exceeds maximum length of 150 characters.');
error[0].message.should.eql('Value in [posts.title] exceeds maximum length of 150 characters.');
error[0].type.should.eql('ValidationError');
when.all([
knex("users").select(),
@ -486,7 +492,9 @@ describe("Import", function () {
}).then(function () {
(1).should.eql(0, 'Data import should not resolve promise.');
}, function (error) {
error.should.eql('Error importing data: Setting key cannot be empty.');
error[0].message.should.eql('Setting key cannot be empty.');
error[0].type.should.eql('ValidationError');
when.all([
knex("users").select(),