Include property value in the error message

When building a list of errors for `ValidationError.message`, include
the values of invalid properties too.

In order to keep the message reasonably short, the values are truncated
at approx 32 characters.
This commit is contained in:
Miroslav Bajtoš 2015-01-12 14:52:54 +01:00
parent 2270ba4db4
commit 9b759c95ac
4 changed files with 143 additions and 33 deletions

View File

@ -26,7 +26,7 @@ function Validatable() {
} }
/** /**
* Validate presence of one or more specified properties. * Validate presence of one or more specified properties.
* Requires a model to include a property to be considered valid; fails when validated field is blank. * Requires a model to include a property to be considered valid; fails when validated field is blank.
* *
* For example, validate presence of title * For example, validate presence of title
@ -49,7 +49,7 @@ function Validatable() {
Validatable.validatesPresenceOf = getConfigurator('presence'); Validatable.validatesPresenceOf = getConfigurator('presence');
/** /**
* Validate absence of one or more specified properties. * Validate absence of one or more specified properties.
* A model should not include a property to be considered valid; fails when validated field not blank. * A model should not include a property to be considered valid; fails when validated field not blank.
* *
* For example, validate absence of reserved * For example, validate absence of reserved
@ -85,7 +85,7 @@ Validatable.validatesAbsenceOf = getConfigurator('absence');
* User.validatesLengthOf('state', {is: 2, message: {is: 'is not valid state name'}}); * User.validatesLengthOf('state', {is: 2, message: {is: 'is not valid state name'}});
* ``` * ```
* @param {String} propertyName Property name to validate. * @param {String} propertyName Property name to validate.
* @options {Object} Options * @options {Object} Options
* @property {Number} is Value that property must equal to validate. * @property {Number} is Value that property must equal to validate.
* @property {Number} min Value that property must be less than to be valid. * @property {Number} min Value that property must be less than to be valid.
* @property {Number} max Value that property must be less than to be valid. * @property {Number} max Value that property must be less than to be valid.
@ -103,7 +103,7 @@ Validatable.validatesLengthOf = getConfigurator('length');
* ``` * ```
* *
* @param {String} propertyName Property name to validate. * @param {String} propertyName Property name to validate.
* @options {Object} Options * @options {Object} Options
* @property {Boolean} int If true, then property must be an integer to be valid. * @property {Boolean} int If true, then property must be an integer to be valid.
* @property {Object} message Optional object with string properties for 'int' for integer validation. Default error messages: * @property {Object} message Optional object with string properties for 'int' for integer validation. Default error messages:
* - number: is not a number * - number: is not a number
@ -123,7 +123,7 @@ Validatable.validatesNumericalityOf = getConfigurator('numericality');
* ``` * ```
* *
* @param {String} propertyName Property name to validate. * @param {String} propertyName Property name to validate.
* @options {Object} Options * @options {Object} Options
* @property {Array} in Array Property must match one of the values in the array to be valid. * @property {Array} in Array Property must match one of the values in the array to be valid.
* @property {String} message Optional error message if property is not valid. Default error message: "is not included in the list". * @property {String} message Optional error message if property is not valid. Default error message: "is not included in the list".
*/ */
@ -135,7 +135,7 @@ Validatable.validatesInclusionOf = getConfigurator('inclusion');
* Example: `Company.validatesExclusionOf('domain', {in: ['www', 'admin']});` * Example: `Company.validatesExclusionOf('domain', {in: ['www', 'admin']});`
* *
* @param {String} propertyName Property name to validate. * @param {String} propertyName Property name to validate.
* @options {Object} Options * @options {Object} Options
* @property {Array} in Array Property must match one of the values in the array to be valid. * @property {Array} in Array Property must match one of the values in the array to be valid.
* @property {String} message Optional error message if property is not valid. Default error message: "is reserved". * @property {String} message Optional error message if property is not valid. Default error message: "is reserved".
*/ */
@ -146,9 +146,9 @@ Validatable.validatesExclusionOf = getConfigurator('exclusion');
* *
* Require a model to include a property that matches the given format. Example: * Require a model to include a property that matches the given format. Example:
* `User.validatesFormat('name', {with: /\w+/});` * `User.validatesFormat('name', {with: /\w+/});`
* *
* @param {String} propertyName Property name to validate. * @param {String} propertyName Property name to validate.
* @options {Object} Options * @options {Object} Options
* @property {RegExp} with Regular expression to validate format. * @property {RegExp} with Regular expression to validate format.
* @property {String} message Optional error message if property is not valid. Default error message: " is invalid". * @property {String} message Optional error message if property is not valid. Default error message: " is invalid".
*/ */
@ -223,7 +223,7 @@ Validatable.validateAsync = getConfigurator('custom', {async: true});
* ``` * ```
* @param {String} propertyName Property name to validate. * @param {String} propertyName Property name to validate.
* @options {Object} Options * @options {Object} Options
* @property {RegExp} with Regular expression to validate format. * @property {RegExp} with Regular expression to validate format.
* @property {Array.<String>} scopedTo List of properties defining the scope. * @property {Array.<String>} scopedTo List of properties defining the scope.
* @property {String} message Optional error message if property is not valid. Default error message: "is not unique". * @property {String} message Optional error message if property is not valid. Default error message: "is not unique".
@ -437,7 +437,7 @@ Validatable.prototype.isValid = function (callback, data) {
asyncFail = false; asyncFail = false;
var attrs = Object.keys(validations || {}); var attrs = Object.keys(validations || {});
attrs.forEach(function(attr) { attrs.forEach(function(attr) {
var attrValidations = validations[attr] || []; var attrValidations = validations[attr] || [];
attrValidations.forEach(function(v) { attrValidations.forEach(function(v) {
@ -454,7 +454,7 @@ Validatable.prototype.isValid = function (callback, data) {
} }
}); });
}); });
if (!async) { if (!async) {
validationsDone.call(inst, function () { validationsDone.call(inst, function () {
if (valid) cleanErrors(inst); if (valid) cleanErrors(inst);
@ -498,12 +498,12 @@ function cleanErrors(inst) {
function validationFailed(inst, attr, conf, cb) { function validationFailed(inst, attr, conf, cb) {
var opts = conf.options || {}; var opts = conf.options || {};
if (typeof attr !== 'string') return false; if (typeof attr !== 'string') return false;
// here we should check skip validation conditions (if, unless) // here we should check skip validation conditions (if, unless)
// that can be specified in conf // that can be specified in conf
if (skipValidation(inst, conf, 'if') if (skipValidation(inst, conf, 'if')
|| skipValidation(inst, conf, 'unless')) { || skipValidation(inst, conf, 'unless')) {
if (cb) cb(false); if (cb) cb(false);
return false; return false;
@ -715,11 +715,11 @@ function ErrorCodes(messages) {
* for (var key in changes) { * for (var key in changes) {
* model[key] = changes[key]; * model[key] = changes[key];
* } * }
* *
* if (model.isValid()) { * if (model.isValid()) {
* return callback(null, { success: true }); * return callback(null, { success: true });
* } * }
* *
* // This line shows how to create a ValidationError * // This line shows how to create a ValidationError
* err = new ValidationError(model); * err = new ValidationError(model);
* callback(err); * callback(err);
@ -735,7 +735,7 @@ function ValidationError(obj) {
this.message = util.format( this.message = util.format(
'The %s instance is not valid. Details: %s.', 'The %s instance is not valid. Details: %s.',
context ? '`' + context + '`' : 'model', context ? '`' + context + '`' : 'model',
formatErrors(obj.errors) || '(unknown)' formatErrors(obj.errors, obj.toJSON()) || '(unknown)'
); );
this.statusCode = 422; this.statusCode = 422;
@ -761,7 +761,9 @@ util.inherits(ValidationError, Error);
var errorHasStackProperty = !!(new Error).stack; var errorHasStackProperty = !!(new Error).stack;
function formatErrors(errors) { ValidationError.maxPropertyStringLength = 32;
function formatErrors(errors, propertyValues) {
var DELIM = '; '; var DELIM = '; ';
errors = errors || {}; errors = errors || {};
return Object.getOwnPropertyNames(errors) return Object.getOwnPropertyNames(errors)
@ -770,9 +772,52 @@ function formatErrors(errors) {
}) })
.map(function(propertyName) { .map(function(propertyName) {
var messages = errors[propertyName]; var messages = errors[propertyName];
var propertyValue = propertyValues[propertyName];
return messages.map(function(msg) { return messages.map(function(msg) {
return '`' + propertyName + '` ' + msg; return formatPropertyError(propertyName, propertyValue, msg);
}).join(DELIM); }).join(DELIM);
}) })
.join(DELIM); .join(DELIM);
} }
function formatPropertyError(propertyName, propertyValue, errorMessage) {
var formattedValue;
var valueType = typeof propertyValue;
if (valueType === 'string') {
formattedValue = JSON.stringify(truncatePropertyString(propertyValue));
} else if (propertyValue instanceof Date) {
formattedValue = propertyValue.toISOString();
} else if (valueType === 'object') {
// objects and arrays
formattedValue = util.inspect(propertyValue, {
showHidden: false,
color: false,
// show top-level object properties only
depth: Array.isArray(propertyValue) ? 1 : 0
});
formattedValue = truncatePropertyString(formattedValue);
} else {
formattedValue = truncatePropertyString('' + propertyValue);
}
return '`' + propertyName + '` ' + errorMessage +
' (value: ' + formattedValue + ')';
}
function truncatePropertyString(value) {
var len = ValidationError.maxPropertyStringLength;
if (value.length <= len) return value;
// preserve few last characters like `}` or `]`, but no more than 3
// this way the last `} ]` in the array of objects is included in the message
var tail;
var m = value.match(/([ \t})\]]+)$/);
if (m) {
tail = m[1].slice(-3);
len -= tail.length;
} else {
tail = value.slice(-3);
len -= 3;
}
return value.slice(0, len-4) + '...' + tail;
}

View File

@ -66,8 +66,8 @@ describe('manipulation', function () {
it('should not allow user-defined value for the id of object - create', function (done) { it('should not allow user-defined value for the id of object - create', function (done) {
Person.create({id: 123456}, function (err, p) { Person.create({id: 123456}, function (err, p) {
err.should.be.instanceof(ValidationError); err.should.be.instanceof(ValidationError);
err.message.should.equal('The `Person` instance is not valid. Details: `id` can\'t be set.');
err.statusCode.should.equal(422); err.statusCode.should.equal(422);
err.details.messages.id.should.eql(['can\'t be set']);
p.should.be.instanceof(Person); p.should.be.instanceof(Person);
p.id.should.equal(123456); p.id.should.equal(123456);
p.isNewRecord().should.be.true; p.isNewRecord().should.be.true;
@ -80,8 +80,8 @@ describe('manipulation', function () {
p.isNewRecord().should.be.true; p.isNewRecord().should.be.true;
p.save(function(err, inst) { p.save(function(err, inst) {
err.should.be.instanceof(ValidationError); err.should.be.instanceof(ValidationError);
err.message.should.equal('The `Person` instance is not valid. Details: `id` can\'t be set.');
err.statusCode.should.equal(422); err.statusCode.should.equal(422);
err.details.messages.id.should.eql(['can\'t be set']);
inst.id.should.equal(123456); inst.id.should.equal(123456);
inst.isNewRecord().should.be.true; inst.isNewRecord().should.be.true;
done(); done();

View File

@ -2111,9 +2111,7 @@ describe('relations', function () {
p.passportItem.create({}, function(err, passport) { p.passportItem.create({}, function(err, passport) {
should.exist(err); should.exist(err);
err.name.should.equal('ValidationError'); err.name.should.equal('ValidationError');
var msg = 'The `Passport` instance is not valid.'; err.details.messages.name.should.eql(['can\'t be blank']);
msg += ' Details: `name` can\'t be blank.';
err.message.should.equal(msg);
done(); done();
}); });
}); });
@ -2125,9 +2123,8 @@ describe('relations', function () {
p.save(function(err) { p.save(function(err) {
should.exist(err); should.exist(err);
err.name.should.equal('ValidationError'); err.name.should.equal('ValidationError');
var msg = 'The `Person` instance is not valid.'; err.details.messages.passportItem
msg += ' Details: `passportItem` is invalid: `name` can\'t be blank.'; .should.eql(['is invalid: `name` can\'t be blank']);
err.message.should.equal(msg);
done(); done();
}); });
}); });
@ -2568,9 +2565,9 @@ describe('relations', function () {
addresses.push({ id: 'work', street: '' }); addresses.push({ id: 'work', street: '' });
Person.create({ name: 'Wilma', addresses: addresses }, function(err, p) { Person.create({ name: 'Wilma', addresses: addresses }, function(err, p) {
err.name.should.equal('ValidationError'); err.name.should.equal('ValidationError');
var expected = 'The `Person` instance is not valid. '; err.details.messages.addresses.should.eql([
expected += 'Details: `addresses` contains invalid item: `work` (`street` can\'t be blank).'; 'contains invalid item: `work` (`street` can\'t be blank)'
err.message.should.equal(expected); ]);
done(); done();
}); });
}); });
@ -2769,7 +2766,7 @@ describe('relations', function () {
err.name.should.equal('ValidationError'); err.name.should.equal('ValidationError');
err.details.codes.street.should.eql(['presence']); err.details.codes.street.should.eql(['presence']);
var expected = 'The `Address` instance is not valid. '; var expected = 'The `Address` instance is not valid. ';
expected += 'Details: `street` can\'t be blank.'; expected += 'Details: `street` can\'t be blank (value: undefined).';
err.message.should.equal(expected); err.message.should.equal(expected);
done(); done();
}); });
@ -3225,9 +3222,6 @@ describe('relations', function () {
should.exist(err); should.exist(err);
err.name.should.equal('ValidationError'); err.name.should.equal('ValidationError');
err.details.codes.jobs.should.eql(['uniqueness']); err.details.codes.jobs.should.eql(['uniqueness']);
var expected = 'The `Category` instance is not valid. ';
expected += 'Details: `jobs` contains duplicate `Job` instance.';
err.message.should.equal(expected);
done(); done();
}); });
}); });

View File

@ -211,6 +211,16 @@ describe('validations', function () {
}); });
}); });
it('should include property value in err.message', function(done) {
delete User.validations;
User.validatesPresenceOf('name');
User.create(function (e, u) {
should.exist(e);
e.message.should.match(/`name` can't be blank \(value: undefined\)/);
done();
});
});
it('should include model name in err.message', function(done) { it('should include model name in err.message', function(done) {
delete User.validations; delete User.validations;
User.validatesPresenceOf('name'); User.validatesPresenceOf('name');
@ -432,4 +442,65 @@ describe('validations', function () {
})).should.be.false; })).should.be.false;
}); });
}); });
describe('invalid value formatting', function() {
var origMaxLen;
beforeEach(function saveAndSetMaxLen() {
origMaxLen = ValidationError.maxPropertyStringLength;
});
afterEach(function restoreMaxLen() {
ValidationError.maxPropertyStringLength = origMaxLen;
});
it('should truncate long strings', function() {
ValidationError.maxPropertyStringLength = 9;
var err = givenValidationError('prop', '1234567890abc', 'is invalid');
getErrorDetails(err)
.should.equal('`prop` is invalid (value: "12...abc").');
});
it('should truncate long objects', function() {
ValidationError.maxPropertyStringLength = 12;
var err = givenValidationError('prop', { foo: 'bar' }, 'is invalid');
getErrorDetails(err)
.should.equal('`prop` is invalid (value: { foo:... }).');
});
it('should truncate long arrays', function() {
ValidationError.maxPropertyStringLength = 12;
var err = givenValidationError('prop', [{ a: 1, b: 2}], 'is invalid');
getErrorDetails(err)
.should.equal('`prop` is invalid (value: [ { a...} ]).');
});
it('should print only top-level object properties', function() {
var err = givenValidationError('prop', { a: { b: 'c' }}, 'is invalid');
getErrorDetails(err)
.should.equal('`prop` is invalid (value: { a: [Object] }).');
});
it('should print only top-level props of objects in array', function() {
var err = givenValidationError('prop', [{ a: { b: 'c' }}], 'is invalid');
getErrorDetails(err)
.should.equal('`prop` is invalid (value: [ { a: [Object] } ]).');
});
function givenValidationError(propertyName, propertyValue, errorMessage) {
var jsonVal = {};
jsonVal[propertyName] = propertyValue;
var errorVal = {};
errorVal[propertyName] = [errorMessage];
var obj = {
errors: errorVal,
toJSON: function() { return jsonVal; }
};
return new ValidationError(obj);
}
function getErrorDetails(err) {
return err.message.replace(/^.*Details: /, '');
}
});
}); });