#2043 introduce ValidationErrorNoStack class to improve error creatio…

…n performance (#2142) * #2043 add ValidationErrorNoStack to improve error creation performance * Consolidate * #2043 remove ValidationErrorNoStack references --------- Co-authored-by: jquense <jquense@ramp.com>
jquense · Mar 6, 2024 · f294613 · f294613
1 parent 2a9e060
commit f294613
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 34 deletions.
diff --git a/src/ValidationError.ts b/src/ValidationError.ts
@@ -5,15 +5,59 @@ let strReg = /\$\{\s*(\w+)\s*\}/g;
 
 type Params = Record<string, unknown>;
 
-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
@@ -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
@@ -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
@@ -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
@@ -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: [],
-          }
+          },
         },
       });
     });