From f294613e34e261b212d905a947c276dacb0cde4f Mon Sep 17 00:00:00 2001 From: tedeschia Date: Wed, 6 Mar 2024 11:37:11 -0300 Subject: [PATCH] #2043 introduce ValidationErrorNoStack class to improve error creation performance (#2142) * #2043 add ValidationErrorNoStack to improve error creation performance * Consolidate * #2043 remove ValidationErrorNoStack references --------- Co-authored-by: jquense --- src/ValidationError.ts | 97 +++++++++++++++++++++++++++--------- src/util/createValidation.ts | 8 ++- test/ValidationError.ts | 7 +++ test/array.ts | 13 +++++ test/mixed.ts | 19 +++++-- 5 files changed, 110 insertions(+), 34 deletions(-) diff --git a/src/ValidationError.ts b/src/ValidationError.ts index 9fce36f3a..20dee52e6 100644 --- a/src/ValidationError.ts +++ b/src/ValidationError.ts @@ -5,15 +5,59 @@ let strReg = /\$\{\s*(\w+)\s*\}/g; type Params = Record; -export default class ValidationError extends Error { +class ValidationErrorNoStack implements ValidationError { + name: string; + message: string; + value: any; path?: string; type?: string; + params?: Params; + errors: string[]; + inner: ValidationError[]; + + constructor( + errorOrErrors: string | ValidationError | readonly ValidationError[], + value?: any, + field?: string, + type?: string, + ) { + this.name = 'ValidationError'; + this.value = value; + this.path = field; + this.type = type; + + this.errors = []; + this.inner = []; + + toArray(errorOrErrors).forEach((err) => { + if (ValidationError.isError(err)) { + this.errors.push(...err.errors); + const innerErrors = err.inner.length ? err.inner : [err]; + this.inner.push(...innerErrors); + } else { + this.errors.push(err); + } + }); + + this.message = + this.errors.length > 1 + ? `${this.errors.length} errors occurred` + : this.errors[0]; + } + [Symbol.toStringTag] = 'Error'; +} + +export default class ValidationError extends Error { + value: any; + path?: string; + type?: string; params?: Params; - inner: ValidationError[]; + errors: string[] = []; + inner: ValidationError[] = []; static formatError( message: string | ((params: Params) => string) | unknown, @@ -40,33 +84,38 @@ export default class ValidationError extends Error { type?: string, disableStack?: boolean, ) { - super(); - - this.name = 'ValidationError'; - this.value = value; - this.path = field; - this.type = type; + const errorNoStack = new ValidationErrorNoStack( + errorOrErrors, + value, + field, + type, + ); - this.errors = []; - this.inner = []; + if (disableStack) { + return errorNoStack; + } - toArray(errorOrErrors).forEach((err) => { - if (ValidationError.isError(err)) { - this.errors.push(...err.errors); - const innerErrors = err.inner.length ? err.inner : [err]; - this.inner.push(...innerErrors); - } else { - this.errors.push(err); - } - }); + super(); - this.message = - this.errors.length > 1 - ? `${this.errors.length} errors occurred` - : this.errors[0]; + this.name = errorNoStack.name; + this.message = errorNoStack.message; + this.type = errorNoStack.type; + this.value = errorNoStack.value; + this.path = errorNoStack.path; + this.errors = errorNoStack.errors; + this.inner = errorNoStack.inner; - if (!disableStack && Error.captureStackTrace) + if (Error.captureStackTrace) { Error.captureStackTrace(this, ValidationError); + } + } + + static [Symbol.hasInstance](inst: any) { + return ( + ValidationErrorNoStack[Symbol.hasInstance](inst) || + super[Symbol.hasInstance](inst) + ); } + [Symbol.toStringTag] = 'Error'; } diff --git a/src/util/createValidation.ts b/src/util/createValidation.ts index 036fbfdb5..cb55c8fb4 100644 --- a/src/util/createValidation.ts +++ b/src/util/createValidation.ts @@ -98,6 +98,7 @@ export default function createValidation(config: { label: schema.spec.label, path: overrides.path || path, spec: schema.spec, + disableStackTrace: overrides.disableStackTrace || disableStackTrace, ...params, ...overrides.params, }; @@ -111,7 +112,7 @@ export default function createValidation(config: { value, nextParams.path, overrides.type || name, - overrides.disableStackTrace ?? disableStackTrace, + nextParams.disableStackTrace, ); error.params = nextParams; return error; @@ -158,10 +159,7 @@ export default function createValidation(config: { `This test will finish after the validate call has returned`, ); } - return Promise.resolve(result).then( - handleResult, - handleError, - ); + return Promise.resolve(result).then(handleResult, handleError); } } catch (err: any) { handleError(err); diff --git a/test/ValidationError.ts b/test/ValidationError.ts index 6084b5015..8d7020000 100644 --- a/test/ValidationError.ts +++ b/test/ValidationError.ts @@ -49,4 +49,11 @@ describe('ValidationError', () => { expect(str).toContain('0'); }); }); + + it('should disable stacks', () => { + const disabled = new ValidationError('error', 1, 'field', 'type', true); + + expect(disabled.constructor.name).toEqual('ValidationErrorNoStack'); + expect(disabled).toBeInstanceOf(ValidationError); + }); }); diff --git a/test/array.ts b/test/array.ts index b01b5b1d5..0771deee2 100644 --- a/test/array.ts +++ b/test/array.ts @@ -158,6 +158,19 @@ describe('Array types', () => { ); }); + it('should respect disableStackTrace', async () => { + let inst = array().of(object({ str: string().required() })); + + const data = [{ str: undefined }, { str: undefined }]; + return Promise.all([ + expect(inst.strict().validate(data)).rejects.toHaveProperty('stack'), + + expect( + inst.strict().validate(data, { disableStackTrace: true }), + ).rejects.not.toHaveProperty('stack'), + ]); + }); + it('should compact arrays', () => { let arr = ['', 1, 0, 4, false, null], inst = array(); diff --git a/test/mixed.ts b/test/mixed.ts index 687476b9f..6cb64e322 100644 --- a/test/mixed.ts +++ b/test/mixed.ts @@ -338,6 +338,16 @@ describe('Mixed Types ', () => { ]); }); + it('should respect disableStackTrace', () => { + // let inst = string().trim(); + // return Promise.all([ + // expect(inst.strict().validate(' hi ')).rejects.toHaveProperty('stack'), + // expect( + // inst.strict().validate(' hi ', { disableStackTrace: true }), + // ).not.toHaveProperty('stack'), + // ]); + }); + it('should overload test()', () => { let inst = mixed().test('test', noop); @@ -967,11 +977,10 @@ describe('Mixed Types ', () => { then: (s) => s.defined(), }), baz: tuple([string(), number()]), - }) - .when(['dummy'], (_, s) => { + }).when(['dummy'], (_, s) => { return s.shape({ - when: string() - }) + when: string(), + }); }); }); @@ -1201,7 +1210,7 @@ describe('Mixed Types ', () => { oneOf: [], optional: true, tests: [], - } + }, }, }); });