Check all errors on joi.assert and joi.attempt

[jsardev]

Describe the problem you are trying to fix (provide as much context as possible)

I would like joi.assert and joi.attempt to return all the errors found in the validating object and not throwing just the first one noticed.

Which API (or modification of the current API) do you suggest to solve that problem ?

joi.validate has this exact option via abortEarly: false. Unfortunately, I don't like the API of this method as I can't just do a try/catch or anything similar and I always need to do if(valid.error === null) check and throw the error manually which feels very cumbersome. joi.assert and joi.attempt methods could have the same option to make the API consistent.

Are you ready to work on a pull request if your suggestion is accepted ?

Unfortunately, I'm in the middle of a big project and already missing some deadlines and I can't work on it right now 😢 Although I could be available in 2 months to take a look on it.

[WesTyler]

Currently both attempt and assert throw the ValidationError, while validate is able to build a collection of ValidationErrors to return.

How would you envision attempt/assert throwing the collection? I think that following the "collection" strategy of validate would require changes to the Error Shape.

[jsardev]

@WesTyler I'm not familiar with joi implementation but looking at it fast right now:

joi/lib/index.js

Lines 170 to 200 in bc09c65

	root.assert = function (value, schema, message) {
	this.attempt(value, schema, message);
	};
	root.attempt = function (value, schema, message) {
	const result = this.validate(value, schema);
	const error = result.error;
	if (error) {
	if (!message) {
	if (typeof error.annotate === 'function') {
	error.message = error.annotate();
	}
	throw error;
	}
	if (!(message instanceof Error)) {
	if (typeof error.annotate === 'function') {
	error.message = `${message} ${error.annotate()}`;
	}
	throw error;
	}
	throw message;
	}
	return result.value;
	};

I see that assert and attempt actually use validate. Couldn't we just allow the abortEarly option in both functions? Wouldn't that work right away?

I'm sorry if this is a lame suggestion but I didn't dive deep into the code yet 😄

[WesTyler]

Sorry, I could have been more clear.

Validate processes errors into an array of error objects that are added to the result object, while assert/attempt throw the singular error instead. I think that allowing abortEarly: false for assert/attempt would require modifying them to instead either:

• throw an updated ValidationError with the collected error objects (change error shape)
• throw a new error with the collected error objects (introduce new error)

I'm just trying to get a sense for what your expectation would be as the requester :)

[jsardev]

Validate processes errors into an array of error objects that are added to the result object, while assert/attempt throw the singular error instead

I think this is wrong. Validate processes errors into an array of error objects, but the result objects get always a single one, the array is being flattened to a single error. So the change would neither require to change the error shape, nor introduce a new error.

joi/lib/types/any/index.js

Lines 756 to 790 in bc09c65

	_validateWithOptions(value, options, callback) {
	if (options) {
	this.checkOptions(options);
	}
	const settings = Settings.concat(internals.defaults, options);
	const result = this._validate(value, null, settings);
	const errors = Errors.process(result.errors, value);
	if (callback) {
	return callback(errors, result.value);
	}
	return {
	error: errors,
	value: result.value,
	then(resolve, reject) {
	if (errors) {
	return Promise.reject(errors).catch(reject);
	}
	return Promise.resolve(result.value).then(resolve);
	},
	catch(reject) {
	if (errors) {
	return Promise.reject(errors).catch(reject);
	}
	return Promise.resolve(result.value);
	}
	};
	}

joi/lib/errors.js

Lines 147 to 210 in bc09c65

	exports.process = function (errors, object) {
	if (!errors) {
	return null;
	}
	// Construct error
	let message = '';
	const details = [];
	const processErrors = function (localErrors, parent, overrideMessage) {
	for (let i = 0; i < localErrors.length; ++i) {
	const item = localErrors[i];
	if (item instanceof Error) {
	return item;
	}
	if (item.flags.error && typeof item.flags.error !== 'function') {
	if (!item.flags.selfError || !item.context.reason) {
	return item.flags.error;
	}
	}
	let itemMessage;
	if (parent === undefined) {
	itemMessage = item.toString();
	message = message + (message ? '. ' : '') + itemMessage;
	}
	// Do not push intermediate errors, we're only interested in leafs
	if (item.context.reason) {
	const override = processErrors(item.context.reason, item.path, item.type === 'override' ? item.message : null);
	if (override) {
	return override;
	}
	}
	else {
	details.push({
	message: overrideMessage || itemMessage || item.toString(),
	path: item.path,
	type: item.type,
	context: item.context
	});
	}
	}
	};
	const override = processErrors(errors);
	if (override) {
	return override;
	}
	const error = new Error(message);
	error.isJoi = true;
	error.name = 'ValidationError';
	error.details = details;
	error._object = object;
	error.annotate = internals.annotate;
	return error;
	};

That's why I suggested to just allow assert and attempt accept an abortEarly. The error in result is never an array of errors and the message of the flattened array of errors while doing abortEarly: false is clear enough IMO: ValidationError: child "p1" fails because ["p1" is required]. child "p2 " fails because ["p2" is required]

Am I right or am I missing something? :)

[WesTyler]

Oh geez, I'm sorry, you're totally right. I was thinking about something else 😆

[jsardev]

Haha, no problem 😄 So is this request still valid for you now when you get my point? :)

[WesTyler]

Absolutely 👍

[jsardev]

Awesome! I will pick it up in the next few months if nobody will outrun me 😃