Skip to content

Commit

Permalink
feat: add multiple error reporting
Browse files Browse the repository at this point in the history
src: throw error on any predicate only if there are any

chore: make default message empty
Doesn't matter much as it's overwritten immediately after

chore: drop redundant nullish coalescing

fix: declare the validationErrors prop as readonly
Gives a TS compilation error if you try to reassign it

chore: generate error message only if there are errors

chore: fix bug that caused validators to error if validator func throws
Noticeable with a function like
```js
	ow(
		null,
		ow.any(
			ow.string.minLength(12),
			ow.string.includes('owo')
		)
	);
```

chore: apply code style suggestion

Co-authored-by: Sindre Sorhus <[email protected]>

chore: fix stack generation and comment

chore: move from t.assert to t.is

src: add more tests to increase coverage

chore: correct test messages

chore: fix rebase
  • Loading branch information
vladfrangu committed Jan 7, 2021
1 parent b4a2ada commit 1a91170
Show file tree
Hide file tree
Showing 14 changed files with 418 additions and 46 deletions.
13 changes: 9 additions & 4 deletions source/argument-error.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
const wrapStackTrace = (error: ArgumentError, stack: string) => `${error.name}: ${error.message}\n${stack}`;

/**
@hidden
*/
export class ArgumentError extends Error {
constructor(message: string, context: Function) {
readonly validationErrors: ReadonlyMap<string, string[]>;

constructor(message: string, context: Function, stack: string, errors = new Map<string, string[]>()) {
super(message);

this.name = 'ArgumentError';

if (Error.captureStackTrace) {
// TODO: Node.js does not preserve the error name in output when using the below, why?
Error.captureStackTrace(this, context);
} else {
this.stack = (new Error()).stack;
this.stack = wrapStackTrace(this, stack);
}

this.name = 'ArgumentError';
this.validationErrors = errors;
}
}
15 changes: 10 additions & 5 deletions source/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import {BasePredicate, isPredicate} from './predicates/base-predicate';
import modifiers, {Modifiers} from './modifiers';
import predicates, {Predicates} from './predicates';
import test from './test';
import {generateStackTrace} from './utils/generate-stack';

/**
@hidden
*/
export type Main = <T>(value: T, label: string | Function, predicate: BasePredicate<T>) => void;
export type Main = <T>(value: T, label: string | Function, predicate: BasePredicate<T>, stack: string) => void;

// Extends is only necessary for the generated documentation to be cleaner. The loaders below infer the correct type.
export interface Ow extends Modifiers, Predicates {
Expand Down Expand Up @@ -61,6 +62,8 @@ export interface ReusableValidator<T> {
}

const ow = <T>(value: T, labelOrPredicate: unknown, predicate?: BasePredicate<T>) => {
const stack = generateStackTrace();

if (!isPredicate(labelOrPredicate) && typeof labelOrPredicate !== 'string') {
throw new TypeError(`Expected second argument to be a predicate or a string, got \`${typeof labelOrPredicate}\``);
}
Expand All @@ -69,12 +72,12 @@ const ow = <T>(value: T, labelOrPredicate: unknown, predicate?: BasePredicate<T>
// If the second argument is a predicate, infer the label
const stackFrames = callsites();

test(value, () => inferLabel(stackFrames), labelOrPredicate);
test(value, () => inferLabel(stackFrames), labelOrPredicate, stack);

return;
}

test(value, labelOrPredicate, predicate as BasePredicate<T>);
test(value, labelOrPredicate, predicate as BasePredicate<T>, stack);
};

Object.defineProperties(ow, {
Expand All @@ -90,15 +93,17 @@ Object.defineProperties(ow, {
},
create: {
value: <T>(labelOrPredicate: BasePredicate<T> | string | undefined, predicate?: BasePredicate<T>) => (value: T, label?: string) => {
const stack = generateStackTrace();

if (isPredicate(labelOrPredicate)) {
const stackFrames = callsites();

test(value, label ?? (() => inferLabel(stackFrames)), labelOrPredicate);
test(value, label ?? (() => inferLabel(stackFrames)), labelOrPredicate, stack);

return;
}

test(value, label ?? (labelOrPredicate as string), predicate as BasePredicate<T>);
test(value, label ?? (labelOrPredicate as string), predicate as BasePredicate<T>, stack);
}
}
});
Expand Down
38 changes: 31 additions & 7 deletions source/predicates/any.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {ArgumentError} from '../argument-error';
import {BasePredicate, testSymbol} from './base-predicate';
import {PredicateOptions} from './predicate';
import {Main} from '..';
import {generateArgumentErrorMessage} from '../utils/generate-argument-error-message';

/**
@hidden
Expand All @@ -12,24 +13,47 @@ export class AnyPredicate<T = unknown> implements BasePredicate<T> {
private readonly options: PredicateOptions = {}
) {}

[testSymbol](value: T, main: Main, label: string | Function): asserts value {
const errors = [
'Any predicate failed with the following errors:'
];
[testSymbol](value: T, main: Main, label: string | Function, stack: string): asserts value {
const errors = new Map<string, string[]>();

for (const predicate of this.predicates) {
try {
main(value, label, predicate);
main(value, label, predicate, stack);
return;
} catch (error: unknown) {
if (value === undefined && this.options.optional === true) {
return;
}

errors.push(`- ${(error as Error).message}`);
// If we received an ArgumentError, then..
if (error instanceof ArgumentError) {
// Iterate through every error reported.
for (const [key, value] of error.validationErrors.entries()) {
// Get the current errors set, if any.
const alreadyPresent = errors.get(key);

// If they are present already, create a unique set with both current and new values.
if (alreadyPresent) {
errors.set(key, [...new Set([...alreadyPresent, ...value])]);
} else {
// Add the errors found as is to the map.
errors.set(key, value);
}
}
}
}
}

throw new ArgumentError(errors.join('\n'), main);
if (errors.size > 0) {
// Generate the `error.message` property.
const message = generateArgumentErrorMessage(errors, true);

throw new ArgumentError(
`Any predicate failed with the following errors:\n${message}`,
main,
stack,
errors
);
}
}
}
2 changes: 1 addition & 1 deletion source/predicates/base-predicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ export const isPredicate = (value: unknown): value is BasePredicate => Boolean((
@hidden
*/
export interface BasePredicate<T = unknown> {
[testSymbol](value: T, main: Main, label: string | Function): void;
[testSymbol](value: T, main: Main, label: string | Function, stack: string): void;
}
50 changes: 40 additions & 10 deletions source/predicates/predicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {ArgumentError} from '../argument-error';
import {not} from '../operators/not';
import {BasePredicate, testSymbol} from './base-predicate';
import {Main} from '..';
import {generateArgumentErrorMessage} from '../utils/generate-argument-error-message';

/**
Function executed when the provided validation fails.
Expand Down Expand Up @@ -95,30 +96,59 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
/**
@hidden
*/
[testSymbol](value: T, main: Main, label: string | Function): asserts value is T {
[testSymbol](value: T, main: Main, label: string | Function, stack: string): asserts value is T {
// Create a map of labels -> received errors.
const errors = new Map<string, string[]>();

for (const {validator, message} of this.context.validators) {
if (this.options.optional === true && value === undefined) {
continue;
}

const result = validator(value);
let result: unknown;

try {
result = validator(value);
} catch (error: unknown) {
// Any errors caught means validators couldn't process the input.
result = error;
}

if (result === true) {
continue;
}

let label2 = label;
const label2 = is.function_(label) ? label() : label;

if (typeof label === 'function') {
label2 = label();
}

label2 = label2 ?
const label_ = label2 ?
`${this.type} \`${label2}\`` :
this.type;

// TODO: Modify the stack output to show the original `ow()` call instead of this `throw` statement
throw new ArgumentError(message(value, label2, result), main);
const mapKey = label2 || this.type;

// Get the current errors encountered for this label.
const currentErrors = errors.get(mapKey);
// Pre-generate the error message that will be reported to the user.
const errorMessage = message(value, label_, result);

// If we already have any errors for this label.
if (currentErrors) {
// If we don't already have this error logged, add it.
if (!currentErrors.includes(errorMessage)) {
currentErrors.push(errorMessage);
}
} else {
// Set this label and error in the full map.
errors.set(mapKey, [errorMessage]);
}
}

// If we have any errors to report, throw.
if (errors.size > 0) {
// Generate the `error.message` property.
const message = generateArgumentErrorMessage(errors);

throw new ArgumentError(message, main, stack, errors);
}
}

Expand Down
4 changes: 2 additions & 2 deletions source/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ Validate the value against the provided predicate.
@param label - Label which should be used in error messages.
@param predicate - Predicate to test to value against.
*/
export default function test<T>(value: T, label: string | Function, predicate: BasePredicate<T>) {
predicate[testSymbol](value, test, label);
export default function test<T>(value: T, label: string | Function, predicate: BasePredicate<T>, stack: string) {
predicate[testSymbol](value, test, label, stack);
}
44 changes: 44 additions & 0 deletions source/utils/generate-argument-error-message.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
Generates a complete message from all errors generated by predicates.
@param errors - The errors generated by the predicates.
@param isAny - If this function is called from the any argument.
@hidden
*/
export const generateArgumentErrorMessage = (errors: Map<string, string[]>, isAny = false) => {
const message = [];

const errorArray = [...errors.values()];

const anyErrorWithoutOneItemOnly = errorArray.some(array => array.length !== 1);

// If only one error "key" is present, enumerate all of those errors only.
if (errors.size === 1) {
const returnedErrors = errorArray[0]!;

if (!isAny && returnedErrors.length === 1) {
return returnedErrors[0]!;
}

for (const entry of returnedErrors) {
message.push(`${isAny ? ' - ' : ''}${entry}`);
}

return message.join('\n');
}

// If every predicate returns just one error, enumerate them as is.
if (!anyErrorWithoutOneItemOnly) {
return errorArray.map(([item]) => ` - ${item}`).join('\n');
}

// Else, iterate through all the errors and enumerate them.
for (const [key, value] of errors) {
message.push(`Errors from the "${key}" predicate:`);
for (const entry of value) {
message.push(` - ${entry}`);
}
}

return message.join('\n');
};
10 changes: 10 additions & 0 deletions source/utils/generate-stack.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
Generates a useful stacktrace that points to the user's code where the error happened on platforms without the `Error.captureStackTrace()` method.
@hidden
*/
export const generateStackTrace = () => {
const stack = new RangeError('INTERNAL_OW_ERROR').stack!;

return stack;
};
7 changes: 5 additions & 2 deletions source/utils/match-shape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import is from '@sindresorhus/is';
import test from '../test';
import {isPredicate} from '../predicates/base-predicate';
import {BasePredicate} from '..';
import {generateStackTrace} from './generate-stack';

// eslint-disable-next-line @typescript-eslint/consistent-indexed-object-style
export interface Shape {
Expand Down Expand Up @@ -44,12 +45,13 @@ Test if the `object` matches the `shape` partially.
@param parent - Name of the parent property.
*/
export function partial(object: Record<string, any>, shape: Shape, parent?: string): boolean | string {
const stack = generateStackTrace();
try {
for (const key of Object.keys(shape)) {
const label = parent ? `${parent}.${key}` : key;

if (isPredicate(shape[key])) {
test(object[key], label, shape[key] as BasePredicate);
test(object[key], label, shape[key] as BasePredicate, stack);
} else if (is.plainObject(shape[key])) {
const result = partial(object[key], shape[key] as Shape, label);

Expand All @@ -75,6 +77,7 @@ Test if the `object` matches the `shape` exactly.
@param parent - Name of the parent property.
*/
export function exact(object: Record<string, any>, shape: Shape, parent?: string): boolean | string {
const stack = generateStackTrace();
try {
const objectKeys = new Set<string>(Object.keys(object));

Expand All @@ -84,7 +87,7 @@ export function exact(object: Record<string, any>, shape: Shape, parent?: string
const label = parent ? `${parent}.${key}` : key;

if (isPredicate(shape[key])) {
test(object[key], label, shape[key] as BasePredicate);
test(object[key], label, shape[key] as BasePredicate, stack);
} else if (is.plainObject(shape[key])) {
if (!Object.prototype.hasOwnProperty.call(object, key)) {
return `Expected \`${label}\` to exist`;
Expand Down
Loading

0 comments on commit 1a91170

Please sign in to comment.