From ae9f863f598bc09ef642f4688b453711d6ccc52c Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 24 Jan 2020 17:03:20 +0100 Subject: [PATCH] [SearchProfiler] Fix handling of bad profile data and update tab behaviour (#55806) (#55844) * Fix searchprofiler's ability to handle badly formed profile data Also fix tab changing upon subsequent requests * Fix comment typo --- .../__tests__/profile_tree.test.tsx | 12 ++++++++++ .../components/profile_tree/profile_tree.tsx | 11 +++++++-- .../application/containers/main/main.tsx | 12 ++-------- .../public/application/store/reducer.ts | 23 +++++++++++++++++-- .../application/utils/has_aggregations.ts | 13 +++++++++++ .../public/application/utils/has_searches.ts | 13 +++++++++++ .../public/application/utils/index.ts | 3 ++- 7 files changed, 72 insertions(+), 15 deletions(-) create mode 100644 x-pack/plugins/searchprofiler/public/application/utils/has_aggregations.ts create mode 100644 x-pack/plugins/searchprofiler/public/application/utils/has_searches.ts diff --git a/x-pack/plugins/searchprofiler/public/application/components/profile_tree/__tests__/profile_tree.test.tsx b/x-pack/plugins/searchprofiler/public/application/components/profile_tree/__tests__/profile_tree.test.tsx index dbd30f584850c3..1286f30d69c262 100644 --- a/x-pack/plugins/searchprofiler/public/application/components/profile_tree/__tests__/profile_tree.test.tsx +++ b/x-pack/plugins/searchprofiler/public/application/components/profile_tree/__tests__/profile_tree.test.tsx @@ -18,4 +18,16 @@ describe('ProfileTree', () => { const init = registerTestBed(ProfileTree); await init(props); }); + + it('does not throw despite bad profile data', async () => { + // For now, ignore the console.error that gets logged. + const props: Props = { + onHighlight: () => {}, + target: 'searches', + data: [{}] as any, + }; + + const init = registerTestBed(ProfileTree); + await init(props); + }); }); diff --git a/x-pack/plugins/searchprofiler/public/application/components/profile_tree/profile_tree.tsx b/x-pack/plugins/searchprofiler/public/application/components/profile_tree/profile_tree.tsx index ea24abbdb56db2..87a73cdefba313 100644 --- a/x-pack/plugins/searchprofiler/public/application/components/profile_tree/profile_tree.tsx +++ b/x-pack/plugins/searchprofiler/public/application/components/profile_tree/profile_tree.tsx @@ -10,7 +10,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui'; import { IndexDetails } from './index_details'; import { ShardDetails } from './shard_details'; import { initDataFor } from './init_data'; -import { Targets, ShardSerialized } from '../../types'; +import { Targets, ShardSerialized, Index } from '../../types'; import { HighlightContextProvider, OnHighlightChangeArgs } from './highlight_context'; export interface Props { @@ -24,7 +24,14 @@ export const ProfileTree = memo(({ data, target, onHighlight }: Props) => { return null; } - const sortedIndices = initDataFor(target)(data); + let sortedIndices: Index[]; + try { + sortedIndices = initDataFor(target)(data); + } catch (e) { + // eslint-disable-next-line no-console + console.error(e); + return null; + } return ( diff --git a/x-pack/plugins/searchprofiler/public/application/containers/main/main.tsx b/x-pack/plugins/searchprofiler/public/application/containers/main/main.tsx index 06bcfb9dcb9c10..90617ba1c51676 100644 --- a/x-pack/plugins/searchprofiler/public/application/containers/main/main.tsx +++ b/x-pack/plugins/searchprofiler/public/application/containers/main/main.tsx @@ -28,18 +28,10 @@ import { import { useAppContext } from '../../contexts/app_context'; import { EmptyTreePlaceHolder, ProfileLoadingPlaceholder } from './components'; -import { Targets, ShardSerialized } from '../../types'; +import { Targets } from '../../types'; import { useProfilerActionContext, useProfilerReadContext } from '../../contexts/profiler_context'; -function hasSearch(profileResponse: ShardSerialized[]) { - const aggs = _.get(profileResponse, '[0].searches', []); - return aggs.length > 0; -} - -function hasAggregations(profileResponse: ShardSerialized[]) { - const aggs = _.get(profileResponse, '[0].aggregations', []); - return aggs.length > 0; -} +import { hasAggregations, hasSearch } from '../../utils'; export const Main = () => { const { getLicenseStatus } = useAppContext(); diff --git a/x-pack/plugins/searchprofiler/public/application/store/reducer.ts b/x-pack/plugins/searchprofiler/public/application/store/reducer.ts index 394110ac495246..5366dcec3fcf02 100644 --- a/x-pack/plugins/searchprofiler/public/application/store/reducer.ts +++ b/x-pack/plugins/searchprofiler/public/application/store/reducer.ts @@ -9,6 +9,7 @@ import { State } from './store'; import { OnHighlightChangeArgs } from '../components/profile_tree'; import { ShardSerialized, Targets } from '../types'; +import { hasSearch, hasAggregations } from '../utils'; export type Action = | { type: 'setProfiling'; value: boolean } @@ -54,8 +55,26 @@ export const reducer: Reducer = (state, action) => { if (action.type === 'setCurrentResponse') { nextState.currentResponse = action.value; if (nextState.currentResponse) { - // Default to the searches tab - nextState.activeTab = 'searches'; + const currentResponseHasAggregations = hasAggregations(nextState.currentResponse); + const currentResponseHasSearch = hasSearch(nextState.currentResponse); + if ( + nextState.activeTab === 'searches' && + !currentResponseHasSearch && + currentResponseHasAggregations + ) { + nextState.activeTab = 'aggregations'; + } else if ( + nextState.activeTab === 'aggregations' && + !currentResponseHasAggregations && + currentResponseHasSearch + ) { + nextState.activeTab = 'searches'; + } else if (!nextState.activeTab) { + // Default to searches tab + nextState.activeTab = 'searches'; + } + } else { + nextState.activeTab = null; } return nextState; } diff --git a/x-pack/plugins/searchprofiler/public/application/utils/has_aggregations.ts b/x-pack/plugins/searchprofiler/public/application/utils/has_aggregations.ts new file mode 100644 index 00000000000000..6d2803be185dc2 --- /dev/null +++ b/x-pack/plugins/searchprofiler/public/application/utils/has_aggregations.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { get } from 'lodash'; +import { ShardSerialized } from '../types'; + +export function hasAggregations(profileResponse: ShardSerialized[]) { + const aggs = get(profileResponse, '[0].aggregations', []); + return aggs.length > 0; +} diff --git a/x-pack/plugins/searchprofiler/public/application/utils/has_searches.ts b/x-pack/plugins/searchprofiler/public/application/utils/has_searches.ts new file mode 100644 index 00000000000000..0247e7a19584ba --- /dev/null +++ b/x-pack/plugins/searchprofiler/public/application/utils/has_searches.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { get } from 'lodash'; +import { ShardSerialized } from '../types'; + +export function hasSearch(profileResponse: ShardSerialized[]) { + const aggs = get(profileResponse, '[0].searches', []); + return aggs.length > 0; +} diff --git a/x-pack/plugins/searchprofiler/public/application/utils/index.ts b/x-pack/plugins/searchprofiler/public/application/utils/index.ts index 556a03fc96fe35..1623ff6fdd51bc 100644 --- a/x-pack/plugins/searchprofiler/public/application/utils/index.ts +++ b/x-pack/plugins/searchprofiler/public/application/utils/index.ts @@ -3,7 +3,8 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ - +export { hasAggregations } from './has_aggregations'; +export { hasSearch } from './has_searches'; export { checkForParseErrors } from './check_for_json_errors'; export { msToPretty } from './ms_to_pretty'; export { nsToPretty } from './ns_to_pretty';