Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] [Security Solutions][Detection Engine] Fixes critical bug with error reporting that was doing a throw (#81549) #81728

Merged
merged 1 commit into from
Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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