From 90c78f882e6ec520eb7d2535ad1e5235146fc815 Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Mon, 26 Oct 2020 22:21:47 -0600 Subject: [PATCH] [Security Solutions][Detection Engine] Fixes critical bug with error reporting that was doing a throw (#81549) ## Summary Fixes an error where it was expecting some data structures on an ES error but there wasn't in some cases. Before: Screen Shot 2020-10-22 at 1 04 35 PM After: Screen Shot 2020-10-22 at 5 45 31 PM - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../signals/single_search_after.test.ts | 4 +- .../detection_engine/signals/utils.test.ts | 52 +++++++++++++++++-- .../lib/detection_engine/signals/utils.ts | 18 ++++++- .../security_solution/server/lib/types.ts | 17 +++--- 4 files changed, 80 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts index 7b7c40f0c4355..c4869f024a977 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts @@ -105,7 +105,9 @@ describe('singleSearchAfter', () => { timestampOverride: undefined, buildRuleMessage, }); - expect(searchErrors).toEqual(['reason: some reason, type: some type, caused by: some reason']); + expect(searchErrors).toEqual([ + 'reason: "some reason" type: "some type" caused by reason: "some reason" caused by type: "some type"', + ]); }); test('if singleSearchAfter works with a given sort id', async () => { const searchAfterSortId = '1234567891111'; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts index 157f741439bd2..894e934ff0247 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts @@ -878,7 +878,7 @@ describe('utils', () => { ]; const createdErrors = createErrorsFromShard({ errors }); expect(createdErrors).toEqual([ - 'reason: some reason, type: some type, caused by: some reason', + 'reason: "some reason" type: "some type" caused by reason: "some reason" caused by type: "some type"', ]); }); @@ -917,8 +917,54 @@ describe('utils', () => { ]; const createdErrors = createErrorsFromShard({ errors }); expect(createdErrors).toEqual([ - 'reason: some reason, type: some type, caused by: some reason', - 'reason: some reason 2, type: some type 2, caused by: some reason 2', + 'reason: "some reason" type: "some type" caused by reason: "some reason" caused by type: "some type"', + 'reason: "some reason 2" type: "some type 2" caused by reason: "some reason 2" caused by type: "some type 2"', + ]); + }); + + test('You can have missing values for the shard errors and get the expected output of an empty string', () => { + const errors: ShardError[] = [ + { + shard: 1, + index: 'index-123', + node: 'node-123', + reason: {}, + }, + ]; + const createdErrors = createErrorsFromShard({ errors }); + expect(createdErrors).toEqual(['']); + }); + + test('You can have a single value for the shard errors and get expected output without extra spaces anywhere', () => { + const errors: ShardError[] = [ + { + shard: 1, + index: 'index-123', + node: 'node-123', + reason: { + reason: 'some reason something went wrong', + }, + }, + ]; + const createdErrors = createErrorsFromShard({ errors }); + expect(createdErrors).toEqual(['reason: "some reason something went wrong"']); + }); + + test('You can have two values for the shard errors and get expected output with one space exactly between the two values', () => { + const errors: ShardError[] = [ + { + shard: 1, + index: 'index-123', + node: 'node-123', + reason: { + reason: 'some reason something went wrong', + caused_by: { type: 'some type' }, + }, + }, + ]; + const createdErrors = createErrorsFromShard({ errors }); + expect(createdErrors).toEqual([ + 'reason: "some reason something went wrong" caused by type: "some type"', ]); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index ac10f5ed9a72d..e2b39b8d0a8c8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -511,7 +511,23 @@ export const getSignalTimeTuples = ({ */ export const createErrorsFromShard = ({ errors }: { errors: ShardError[] }): string[] => { return errors.map((error) => { - return `reason: ${error.reason.reason}, type: ${error.reason.caused_by.type}, caused by: ${error.reason.caused_by.reason}`; + const { + reason: { + reason, + type, + caused_by: { reason: causedByReason, type: causedByType } = { + reason: undefined, + type: undefined, + }, + } = {}, + } = error; + + return [ + ...(reason != null ? [`reason: "${reason}"`] : []), + ...(type != null ? [`type: "${type}"`] : []), + ...(causedByReason != null ? [`caused by reason: "${causedByReason}"`] : []), + ...(causedByType != null ? [`caused by type: "${causedByType}"`] : []), + ].join(' '); }); }; diff --git a/x-pack/plugins/security_solution/server/lib/types.ts b/x-pack/plugins/security_solution/server/lib/types.ts index 29db38bbbea68..c735412aedbf5 100644 --- a/x-pack/plugins/security_solution/server/lib/types.ts +++ b/x-pack/plugins/security_solution/server/lib/types.ts @@ -48,21 +48,26 @@ export interface ShardsResponse { failures?: ShardError[]; } -export interface ShardError { +/** + * This type is being very conservative with the partials to not expect anything to + * be guaranteed on the type as we don't have regular and proper types of ShardError. + * Once we do, remove this type for the regular ShardError type from the elastic library. + */ +export type ShardError = Partial<{ shard: number; index: string; node: string; - reason: { + reason: Partial<{ type: string; reason: string; index_uuid: string; index: string; - caused_by: { + caused_by: Partial<{ type: string; reason: string; - }; - }; -} + }>; + }>; +}>; export interface SearchResponse { took: number;