[Security Solution][Tech Debt] - Cleans up error formatter to not ret…

…urn duplicate error messages (#74600) ## Summary Using the `formatErrors` util would result in duplicate error messages sometimes. Was noticing this in particular when using union types, where the type validation would check every item in a union and report an error for each one. This resulted in large, repeating errors. Used `uniq` to filter out duplicates. Updated unit tests.
elastic · Aug 6, 2020 · 979bdaa · 979bdaa
1 parent ccf8e2b
commit 979bdaa
Show file tree

Hide file tree

Showing 13 changed files with 22 additions and 47 deletions.
diff --git a/x-pack/plugins/lists/common/schemas/types/comment.test.ts b/x-pack/plugins/lists/common/schemas/types/comment.test.ts
@@ -60,7 +60,6 @@ describe('Comment', () => {
 
       expect(getPaths(left(message.errors))).toEqual([
         'Invalid value "undefined" supplied to "({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)"',
-        'Invalid value "undefined" supplied to "({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)"',
       ]);
       expect(message.schema).toEqual({});
     });
@@ -200,7 +199,6 @@ describe('Comment', () => {
 
       expect(getPaths(left(message.errors))).toEqual([
         'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
-        'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
       ]);
       expect(message.schema).toEqual({});
     });
@@ -232,7 +230,6 @@ describe('Comment', () => {
 
       expect(getPaths(left(message.errors))).toEqual([
         'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
-        'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
       ]);
       expect(message.schema).toEqual({});
     });

diff --git a/x-pack/plugins/lists/common/schemas/types/default_comments_array.test.ts b/x-pack/plugins/lists/common/schemas/types/default_comments_array.test.ts
@@ -39,7 +39,6 @@ describe('default_comments_array', () => {
 
     expect(getPaths(left(message.errors))).toEqual([
       'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
-      'Invalid value "1" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
     ]);
     expect(message.schema).toEqual({});
   });
@@ -51,7 +50,6 @@ describe('default_comments_array', () => {
 
     expect(getPaths(left(message.errors))).toEqual([
       'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
-      'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString, created_at: string, created_by: string, id: NonEmptyString |} & Partial<{| updated_at: string, updated_by: string |}>)>"',
     ]);
     expect(message.schema).toEqual({});
   });

diff --git a/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.test.ts b/x-pack/plugins/lists/common/schemas/types/default_update_comments_array.test.ts
@@ -39,7 +39,6 @@ describe('default_update_comments_array', () => {
 
     expect(getPaths(left(message.errors))).toEqual([
       'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
-      'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
     ]);
     expect(message.schema).toEqual({});
   });
@@ -51,7 +50,6 @@ describe('default_update_comments_array', () => {
 
     expect(getPaths(left(message.errors))).toEqual([
       'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
-      'Invalid value "some string" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
     ]);
     expect(message.schema).toEqual({});
   });

diff --git a/x-pack/plugins/lists/common/schemas/types/entries.test.ts b/x-pack/plugins/lists/common/schemas/types/entries.test.ts
@@ -61,17 +61,10 @@ describe('Entries', () => {
       const message = pipe(decoded, foldLeftRight);
 
       expect(getPaths(left(message.errors))).toEqual([
-        'Invalid value "undefined" supplied to "operator"',
-        'Invalid value "nested" supplied to "type"',
-        'Invalid value "undefined" supplied to "value"',
         'Invalid value "undefined" supplied to "operator"',
         'Invalid value "nested" supplied to "type"',
         'Invalid value "undefined" supplied to "value"',
         'Invalid value "undefined" supplied to "list"',
-        'Invalid value "undefined" supplied to "operator"',
-        'Invalid value "nested" supplied to "type"',
-        'Invalid value "undefined" supplied to "operator"',
-        'Invalid value "nested" supplied to "type"',
       ]);
       expect(message.schema).toEqual({});
     });

diff --git a/x-pack/plugins/lists/common/schemas/types/non_empty_entries_array.test.ts b/x-pack/plugins/lists/common/schemas/types/non_empty_entries_array.test.ts
@@ -125,10 +125,6 @@ describe('non_empty_entries_array', () => {
 
     expect(getPaths(left(message.errors))).toEqual([
       'Invalid value "1" supplied to "NonEmptyEntriesArray"',
-      'Invalid value "1" supplied to "NonEmptyEntriesArray"',
-      'Invalid value "1" supplied to "NonEmptyEntriesArray"',
-      'Invalid value "1" supplied to "NonEmptyEntriesArray"',
-      'Invalid value "1" supplied to "NonEmptyEntriesArray"',
     ]);
     expect(message.schema).toEqual({});
   });

diff --git a/x-pack/plugins/lists/common/schemas/types/non_empty_nested_entries_array.test.ts b/x-pack/plugins/lists/common/schemas/types/non_empty_nested_entries_array.test.ts
@@ -86,19 +86,6 @@ describe('non_empty_nested_entries_array', () => {
       'Invalid value "undefined" supplied to "operator"',
       'Invalid value "nested" supplied to "type"',
       'Invalid value "undefined" supplied to "value"',
-      'Invalid value "undefined" supplied to "operator"',
-      'Invalid value "nested" supplied to "type"',
-      'Invalid value "undefined" supplied to "value"',
-      'Invalid value "undefined" supplied to "operator"',
-      'Invalid value "nested" supplied to "type"',
-      'Invalid value "undefined" supplied to "operator"',
-      'Invalid value "nested" supplied to "type"',
-      'Invalid value "undefined" supplied to "value"',
-      'Invalid value "undefined" supplied to "operator"',
-      'Invalid value "nested" supplied to "type"',
-      'Invalid value "undefined" supplied to "value"',
-      'Invalid value "undefined" supplied to "operator"',
-      'Invalid value "nested" supplied to "type"',
     ]);
     expect(message.schema).toEqual({});
   });
@@ -123,8 +110,6 @@ describe('non_empty_nested_entries_array', () => {
 
     expect(getPaths(left(message.errors))).toEqual([
       'Invalid value "1" supplied to "NonEmptyNestedEntriesArray"',
-      'Invalid value "1" supplied to "NonEmptyNestedEntriesArray"',
-      'Invalid value "1" supplied to "NonEmptyNestedEntriesArray"',
     ]);
     expect(message.schema).toEqual({});
   });

diff --git a/x-pack/plugins/lists/common/schemas/types/update_comment.test.ts b/x-pack/plugins/lists/common/schemas/types/update_comment.test.ts
@@ -110,7 +110,6 @@ describe('CommentsUpdate', () => {
 
       expect(getPaths(left(message.errors))).toEqual([
         'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
-        'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
       ]);
       expect(message.schema).toEqual({});
     });
@@ -142,7 +141,6 @@ describe('CommentsUpdate', () => {
 
       expect(getPaths(left(message.errors))).toEqual([
         'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
-        'Invalid value "1" supplied to "Array<({| comment: NonEmptyString |} & Partial<{| id: NonEmptyString |}>)>"',
       ]);
       expect(message.schema).toEqual({});
     });

diff --git a/...ecurity_solution/common/detection_engine/schemas/request/create_rules_bulk_schema.test.ts b/...ecurity_solution/common/detection_engine/schemas/request/create_rules_bulk_schema.test.ts
@@ -127,7 +127,6 @@ describe('create_rules_bulk_schema', () => {
     const output = foldLeftRight(checked);
     expect(formatErrors(output.errors)).toEqual([
       'Invalid value "undefined" supplied to "risk_score"',
-      'Invalid value "undefined" supplied to "risk_score"',
     ]);
     expect(output.schema).toEqual({});
   });

diff --git a/...ecurity_solution/common/detection_engine/schemas/request/update_rules_bulk_schema.test.ts b/...ecurity_solution/common/detection_engine/schemas/request/update_rules_bulk_schema.test.ts
@@ -123,7 +123,6 @@ describe('update_rules_bulk_schema', () => {
     const output = foldLeftRight(checked);
     expect(formatErrors(output.errors)).toEqual([
       'Invalid value "undefined" supplied to "risk_score"',
-      'Invalid value "undefined" supplied to "risk_score"',
     ]);
     expect(output.schema).toEqual({});
   });

diff --git a/x-pack/plugins/security_solution/common/format_errors.test.ts b/x-pack/plugins/security_solution/common/format_errors.test.ts
@@ -41,6 +41,22 @@ describe('utils', () => {
     expect(output).toEqual(['some error 1', 'some error 2']);
   });
 
+  test('it filters out duplicate error messages', () => {
+    const validationError1: t.ValidationError = {
+      value: 'Some existing error 1',
+      context: [],
+      message: 'some error 1',
+    };
+    const validationError2: t.ValidationError = {
+      value: 'Some existing error 1',
+      context: [],
+      message: 'some error 1',
+    };
+    const errors: t.Errors = [validationError1, validationError2];
+    const output = formatErrors(errors);
+    expect(output).toEqual(['some error 1']);
+  });
+
   test('will use message before context if it is set', () => {
     const context: t.Context = ([{ key: 'some string key' }] as unknown) as t.Context;
     const validationError1: t.ValidationError = {

diff --git a/x-pack/plugins/security_solution/common/format_errors.ts b/x-pack/plugins/security_solution/common/format_errors.ts
@@ -8,7 +8,7 @@ import * as t from 'io-ts';
 import { isObject } from 'lodash/fp';
 
 export const formatErrors = (errors: t.Errors): string[] => {
-  return errors.map((error) => {
+  const err = errors.map((error) => {
     if (error.message != null) {
       return error.message;
     } else {
@@ -26,4 +26,6 @@ export const formatErrors = (errors: t.Errors): string[] => {
       return `Invalid value "${value}" supplied to "${suppliedValue}"`;
     }
   });
+
+  return [...new Set(err)];
 };
diff --git a/x-pack/plugins/security_solution/server/lib/timeline/routes/export_timelines_route.test.ts b/x-pack/plugins/security_solution/server/lib/timeline/routes/export_timelines_route.test.ts
@@ -110,7 +110,7 @@ describe('export timelines', () => {
       const result = server.validate(request);
 
       expect(result.badRequest.mock.calls[0][0]).toEqual(
-        'Invalid value "someId" supplied to "ids",Invalid value "someId" supplied to "ids",Invalid value "{"ids":"someId"}" supplied to "(Partial<{ ids: (Array<string> | null) }> | null)"'
+        'Invalid value "someId" supplied to "ids",Invalid value "{"ids":"someId"}" supplied to "(Partial<{ ids: (Array<string> | null) }> | null)"'
       );
     });
   });

diff --git a/x-pack/plugins/security_solution/server/lib/timeline/routes/import_timelines_route.test.ts b/x-pack/plugins/security_solution/server/lib/timeline/routes/import_timelines_route.test.ts
@@ -494,10 +494,7 @@ describe('import timelines', () => {
       const result = server.validate(request);
 
       expect(result.badRequest).toHaveBeenCalledWith(
-        [
-          'Invalid value "undefined" supplied to "file"',
-          'Invalid value "undefined" supplied to "file"',
-        ].join(',')
+        'Invalid value "undefined" supplied to "file"'
       );
     });
   });
@@ -923,10 +920,7 @@ describe('import timeline templates', () => {
       const result = server.validate(request);
 
       expect(result.badRequest).toHaveBeenCalledWith(
-        [
-          'Invalid value "undefined" supplied to "file"',
-          'Invalid value "undefined" supplied to "file"',
-        ].join(',')
+        'Invalid value "undefined" supplied to "file"'
       );
     });
   });