Skip to content

Commit 90c78f8

Browse files
[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: <img width="2246" alt="Screen Shot 2020-10-22 at 1 04 35 PM" src="https://user-images.githubusercontent.com/1151048/96940994-7d98a780-148e-11eb-93bd-77e4adf42896.png"> After: <img width="2229" alt="Screen Shot 2020-10-22 at 5 45 31 PM" src="https://user-images.githubusercontent.com/1151048/96941005-84bfb580-148e-11eb-989f-1dae6892e641.png"> - [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
1 parent 20cfa16 commit 90c78f8

File tree

4 files changed

+80
-11
lines changed

4 files changed

+80
-11
lines changed

x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_search_after.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,9 @@ describe('singleSearchAfter', () => {
105105
timestampOverride: undefined,
106106
buildRuleMessage,
107107
});
108-
expect(searchErrors).toEqual(['reason: some reason, type: some type, caused by: some reason']);
108+
expect(searchErrors).toEqual([
109+
'reason: "some reason" type: "some type" caused by reason: "some reason" caused by type: "some type"',
110+
]);
109111
});
110112
test('if singleSearchAfter works with a given sort id', async () => {
111113
const searchAfterSortId = '1234567891111';

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,7 @@ describe('utils', () => {
878878
];
879879
const createdErrors = createErrorsFromShard({ errors });
880880
expect(createdErrors).toEqual([
881-
'reason: some reason, type: some type, caused by: some reason',
881+
'reason: "some reason" type: "some type" caused by reason: "some reason" caused by type: "some type"',
882882
]);
883883
});
884884

@@ -917,8 +917,54 @@ describe('utils', () => {
917917
];
918918
const createdErrors = createErrorsFromShard({ errors });
919919
expect(createdErrors).toEqual([
920-
'reason: some reason, type: some type, caused by: some reason',
921-
'reason: some reason 2, type: some type 2, caused by: some reason 2',
920+
'reason: "some reason" type: "some type" caused by reason: "some reason" caused by type: "some type"',
921+
'reason: "some reason 2" type: "some type 2" caused by reason: "some reason 2" caused by type: "some type 2"',
922+
]);
923+
});
924+
925+
test('You can have missing values for the shard errors and get the expected output of an empty string', () => {
926+
const errors: ShardError[] = [
927+
{
928+
shard: 1,
929+
index: 'index-123',
930+
node: 'node-123',
931+
reason: {},
932+
},
933+
];
934+
const createdErrors = createErrorsFromShard({ errors });
935+
expect(createdErrors).toEqual(['']);
936+
});
937+
938+
test('You can have a single value for the shard errors and get expected output without extra spaces anywhere', () => {
939+
const errors: ShardError[] = [
940+
{
941+
shard: 1,
942+
index: 'index-123',
943+
node: 'node-123',
944+
reason: {
945+
reason: 'some reason something went wrong',
946+
},
947+
},
948+
];
949+
const createdErrors = createErrorsFromShard({ errors });
950+
expect(createdErrors).toEqual(['reason: "some reason something went wrong"']);
951+
});
952+
953+
test('You can have two values for the shard errors and get expected output with one space exactly between the two values', () => {
954+
const errors: ShardError[] = [
955+
{
956+
shard: 1,
957+
index: 'index-123',
958+
node: 'node-123',
959+
reason: {
960+
reason: 'some reason something went wrong',
961+
caused_by: { type: 'some type' },
962+
},
963+
},
964+
];
965+
const createdErrors = createErrorsFromShard({ errors });
966+
expect(createdErrors).toEqual([
967+
'reason: "some reason something went wrong" caused by type: "some type"',
922968
]);
923969
});
924970
});

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,23 @@ export const getSignalTimeTuples = ({
511511
*/
512512
export const createErrorsFromShard = ({ errors }: { errors: ShardError[] }): string[] => {
513513
return errors.map((error) => {
514-
return `reason: ${error.reason.reason}, type: ${error.reason.caused_by.type}, caused by: ${error.reason.caused_by.reason}`;
514+
const {
515+
reason: {
516+
reason,
517+
type,
518+
caused_by: { reason: causedByReason, type: causedByType } = {
519+
reason: undefined,
520+
type: undefined,
521+
},
522+
} = {},
523+
} = error;
524+
525+
return [
526+
...(reason != null ? [`reason: "${reason}"`] : []),
527+
...(type != null ? [`type: "${type}"`] : []),
528+
...(causedByReason != null ? [`caused by reason: "${causedByReason}"`] : []),
529+
...(causedByType != null ? [`caused by type: "${causedByType}"`] : []),
530+
].join(' ');
515531
});
516532
};
517533

x-pack/plugins/security_solution/server/lib/types.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,26 @@ export interface ShardsResponse {
4848
failures?: ShardError[];
4949
}
5050

51-
export interface ShardError {
51+
/**
52+
* This type is being very conservative with the partials to not expect anything to
53+
* be guaranteed on the type as we don't have regular and proper types of ShardError.
54+
* Once we do, remove this type for the regular ShardError type from the elastic library.
55+
*/
56+
export type ShardError = Partial<{
5257
shard: number;
5358
index: string;
5459
node: string;
55-
reason: {
60+
reason: Partial<{
5661
type: string;
5762
reason: string;
5863
index_uuid: string;
5964
index: string;
60-
caused_by: {
65+
caused_by: Partial<{
6166
type: string;
6267
reason: string;
63-
};
64-
};
65-
}
68+
}>;
69+
}>;
70+
}>;
6671

6772
export interface SearchResponse<T> {
6873
took: number;

0 commit comments

Comments
 (0)