Skip to content

Commit

Permalink
[Security Solutions][Detection Engine] Fixes critical bug with error …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
FrankHassanabad authored Oct 27, 2020
1 parent 20cfa16 commit 90c78f8
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"',
]);
});

Expand Down Expand Up @@ -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"',
]);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(' ');
});
};

Expand Down
17 changes: 11 additions & 6 deletions x-pack/plugins/security_solution/server/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
took: number;
Expand Down

0 comments on commit 90c78f8

Please sign in to comment.