From 4000b13edbe2430d7ffafbf6c838ecc69e3d6ea4 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 29 Jun 2021 12:15:46 -0700 Subject: [PATCH 1/4] [data.search] Handle warnings inside of headers --- .../search_examples/public/search/app.tsx | 77 ++++++++++++++++++- src/plugins/data/common/search/types.ts | 5 ++ src/plugins/data/common/search/utils.ts | 7 ++ .../public/search/fetch/handle_response.tsx | 13 +++- .../ese_search/ese_search_strategy.ts | 6 +- .../strategies/ese_search/response_utils.ts | 3 +- 6 files changed, 105 insertions(+), 6 deletions(-) diff --git a/examples/search_examples/public/search/app.tsx b/examples/search_examples/public/search/app.tsx index 06f9426b4965c..200a1d514e33f 100644 --- a/examples/search_examples/public/search/app.tsx +++ b/examples/search_examples/public/search/app.tsx @@ -47,6 +47,7 @@ import { isErrorResponse, } from '../../../../src/plugins/data/public'; import { IMyStrategyResponse } from '../../common/types'; +import { isWarningResponse } from '../../../../src/plugins/data/common'; interface SearchExamplesAppDeps { notifications: CoreStart['notifications']; @@ -131,12 +132,46 @@ export const SearchExamplesApp = ({ setSelectedNumericField(fields?.length ? getNumeric(fields)[0] : null); }, [fields]); - const doAsyncSearch = async (strategy?: string, sessionId?: string) => { + const doAsyncSearch = async ( + strategy?: string, + sessionId?: string, + addWarning: boolean = false, + addError: boolean = false + ) => { if (!indexPattern || !selectedNumericField) return; // Construct the query portion of the search request const query = data.query.getEsQuery(indexPattern); + if (addWarning) { + query.bool.must.push({ + // @ts-ignore + error_query: { + indices: [ + { + name: indexPattern.title, + error_type: 'warning', + message: 'Watch out!', + }, + ], + }, + }); + } + if (addError) { + query.bool.must.push({ + // @ts-ignore + error_query: { + indices: [ + { + name: indexPattern.title, + error_type: 'exception', + message: 'Watch out!', + }, + ], + }, + }); + } + // Construct the aggregations portion of the search request by using the `data.search.aggs` service. const aggs = [{ type: 'avg', params: { field: selectedNumericField!.name } }]; const aggsDsl = data.search.aggs.createAggConfigs(indexPattern, aggs).toDsl(); @@ -193,9 +228,15 @@ export const SearchExamplesApp = ({ } ); searchSubscription$.unsubscribe(); + if (isWarningResponse(res)) { + notifications.toasts.addWarning({ + title: 'Warning', + text: mountReactNode(res.warning), + }); + } } else if (isErrorResponse(res)) { // TODO: Make response error status clearer - notifications.toasts.addWarning('An error has occurred'); + notifications.toasts.addDanger('An error has occurred'); searchSubscription$.unsubscribe(); } }, @@ -270,6 +311,14 @@ export const SearchExamplesApp = ({ doAsyncSearch('myStrategy'); }; + const onWarningSearchClickHandler = () => { + doAsyncSearch(undefined, undefined, true); + }; + + const onErrorSearchClickHandler = () => { + doAsyncSearch(undefined, undefined, false, true); + }; + const onPartialResultsClickHandler = () => { setSelectedTab(1); const req = { @@ -530,6 +579,30 @@ export const SearchExamplesApp = ({ + +

Handling errors & warnings

+ + + When fetching data from Elasticsearch, there are several different ways warnings and + errors may be returned. In general, it is recommended to use the utility functions + isWarningResponse and isErrorResponse when + handling responses. + + + + + + + + + +

Handling partial results

 diff --git a/src/plugins/data/common/search/types.ts b/src/plugins/data/common/search/types.ts index c5cf3f9f09e6c..1c9ecd3b37059 100644 --- a/src/plugins/data/common/search/types.ts +++ b/src/plugins/data/common/search/types.ts @@ -70,6 +70,11 @@ export interface IKibanaSearchResponse { */ isRestored?: boolean; + /** + * Optional warnings that should be surfaced to the end user + */ + warning?: string; + /** * The raw response returned by the internal search method (usually the raw ES response) */ diff --git a/src/plugins/data/common/search/utils.ts b/src/plugins/data/common/search/utils.ts index e11957c6fa9fc..155aed937eaf8 100644 --- a/src/plugins/data/common/search/utils.ts +++ b/src/plugins/data/common/search/utils.ts @@ -28,3 +28,10 @@ export const isCompleteResponse = (response?: IKibanaSearchResponse) => { export const isPartialResponse = (response?: IKibanaSearchResponse) => { return Boolean(response && response.isRunning && response.isPartial); }; + +/** + * @returns true if request has warnings that should be surfaced to the end user + */ +export const isWarningResponse = (response?: IKibanaSearchResponse) => { + return Boolean(response && response.warning); +}; diff --git a/src/plugins/data/public/search/fetch/handle_response.tsx b/src/plugins/data/public/search/fetch/handle_response.tsx index 58e4da6b95fae..f4acebfb36060 100644 --- a/src/plugins/data/public/search/fetch/handle_response.tsx +++ b/src/plugins/data/public/search/fetch/handle_response.tsx @@ -16,7 +16,18 @@ import { getNotifications } from '../../services'; import { SearchRequest } from '..'; export function handleResponse(request: SearchRequest, response: IKibanaSearchResponse) { - const { rawResponse } = response; + const { rawResponse, warning } = response; + if (warning) { + getNotifications().toasts.addWarning({ + title: i18n.translate('data.search.searchSource.fetch.warningMessage', { + defaultMessage: 'Warning: {warning}', + values: { + warning, + }, + }), + }); + } + if (rawResponse.timed_out) { getNotifications().toasts.addWarning({ title: i18n.translate('data.search.searchSource.fetch.requestTimedOutNotificationMessage', { diff --git a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts index d6af00ada80fa..ae9a100a09794 100644 --- a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts +++ b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts @@ -71,12 +71,14 @@ export const enhancedEsSearchStrategyProvider = ( const promise = id ? client.asyncSearch.get({ ...params, id }) : client.asyncSearch.submit(params); - const { body } = await shimAbortSignal(promise, options.abortSignal); + const { body, headers } = await shimAbortSignal(promise, options.abortSignal); + const response = shimHitsTotal(body.response, options); return toAsyncKibanaSearchResponse( // @ts-expect-error @elastic/elasticsearch start_time_in_millis expected to be number - { ...body, response } + { ...body, response }, + headers?.warning ); }; diff --git a/src/plugins/data/server/search/strategies/ese_search/response_utils.ts b/src/plugins/data/server/search/strategies/ese_search/response_utils.ts index ae3d258e2205d..0a92c95dac615 100644 --- a/src/plugins/data/server/search/strategies/ese_search/response_utils.ts +++ b/src/plugins/data/server/search/strategies/ese_search/response_utils.ts @@ -12,12 +12,13 @@ import { getTotalLoaded } from '../es_search'; /** * Get the Kibana representation of an async search response (see `IKibanaSearchResponse`). */ -export function toAsyncKibanaSearchResponse(response: AsyncSearchResponse) { +export function toAsyncKibanaSearchResponse(response: AsyncSearchResponse, warning?: string) { return { id: response.id, rawResponse: response.response, isPartial: response.is_partial, isRunning: response.is_running, + ...(warning ? { warning } : {}), ...getTotalLoaded(response.response), }; } From 1436872b4468d52ad622eeaad9207de5952cba0e Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 1 Jul 2021 12:48:08 -0700 Subject: [PATCH 2/4] Update docs --- ...gin-plugins-data-public.ikibanasearchresponse.md | 1 + ...ins-data-public.ikibanasearchresponse.warning.md | 13 +++++++++++++ src/plugins/data/public/public.api.md | 1 + 3 files changed, 15 insertions(+) create mode 100644 docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.warning.md diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.md index c7046902dac72..73261cd49d6d2 100644 --- a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.md +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.md @@ -21,4 +21,5 @@ export interface IKibanaSearchResponse | [loaded](./kibana-plugin-plugins-data-public.ikibanasearchresponse.loaded.md) | number | If relevant to the search strategy, return a loaded number that represents how progress is indicated. | | [rawResponse](./kibana-plugin-plugins-data-public.ikibanasearchresponse.rawresponse.md) | RawResponse | The raw response returned by the internal search method (usually the raw ES response) | | [total](./kibana-plugin-plugins-data-public.ikibanasearchresponse.total.md) | number | If relevant to the search strategy, return a total number that represents how progress is indicated. | +| [warning](./kibana-plugin-plugins-data-public.ikibanasearchresponse.warning.md) | string | Optional warnings that should be surfaced to the end user | diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.warning.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.warning.md new file mode 100644 index 0000000000000..cc0b8e2bea56e --- /dev/null +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ikibanasearchresponse.warning.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-plugins-data-public](./kibana-plugin-plugins-data-public.md) > [IKibanaSearchResponse](./kibana-plugin-plugins-data-public.ikibanasearchresponse.md) > [warning](./kibana-plugin-plugins-data-public.ikibanasearchresponse.warning.md) + +## IKibanaSearchResponse.warning property + +Optional warnings that should be surfaced to the end user + +Signature: + +```typescript +warning?: string; +``` diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 35094fac1cc0f..a766db3e25786 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -1344,6 +1344,7 @@ export interface IKibanaSearchResponse { loaded?: number; rawResponse: RawResponse; total?: number; + warning?: string; } // Warning: (ae-forgotten-export) The symbol "MetricAggType" needs to be exported by the entry point index.d.ts From a98e7c6b4d443b09c078cdd7c7d6e5c6ab28a970 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 6 Jul 2021 20:14:49 -0700 Subject: [PATCH 3/4] Add tests --- .../search_examples/public/search/app.tsx | 32 ++++++++++++++----- .../search_examples/search_example.ts | 21 ++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/examples/search_examples/public/search/app.tsx b/examples/search_examples/public/search/app.tsx index 200a1d514e33f..8ffe23e0267f2 100644 --- a/examples/search_examples/public/search/app.tsx +++ b/examples/search_examples/public/search/app.tsx @@ -240,8 +240,11 @@ export const SearchExamplesApp = ({ searchSubscription$.unsubscribe(); } }, - error: () => { - notifications.toasts.addDanger('Failed to run search'); + error: (e) => { + notifications.toasts.addDanger({ + title: 'Failed to run search', + text: e.message, + }); }, }); }; @@ -348,8 +351,11 @@ export const SearchExamplesApp = ({ searchSubscription$.unsubscribe(); } }, - error: () => { - notifications.toasts.addDanger('Failed to run search'); + error: (e) => { + notifications.toasts.addDanger({ + title: 'Failed to run search', + text: e.message, + }); }, }); }; @@ -588,16 +594,26 @@ export const SearchExamplesApp = ({ isWarningResponse and isErrorResponse when handling responses. - + - + diff --git a/x-pack/test/examples/search_examples/search_example.ts b/x-pack/test/examples/search_examples/search_example.ts index c841b595ed119..fb3cef4055e33 100644 --- a/x-pack/test/examples/search_examples/search_example.ts +++ b/x-pack/test/examples/search_examples/search_example.ts @@ -5,6 +5,7 @@ * 2.0. */ +import expect from '@kbn/expect'; import { FtrProviderContext } from '../../functional/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -13,6 +14,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const PageObjects = getPageObjects(['common', 'timePicker']); const retry = getService('retry'); const comboBox = getService('comboBox'); + const toasts = getService('toasts'); describe('Search session example', () => { const appId = 'searchExamples'; @@ -28,6 +30,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ); }); + beforeEach(async () => { + await toasts.dismissAllToasts(); + await retry.waitFor('toasts gone', async () => { + return (await toasts.getToastCount()) === 0; + }); + }); + it('should have an other bucket', async () => { await testSubjects.click('searchSourceWithOther'); await testSubjects.click('responseTab'); @@ -53,5 +62,17 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { return buckets.length === 2; }); }); + + it('should handle warnings', async () => { + await testSubjects.click('searchWithWarning'); + await retry.waitFor('', async () => { + const toastCount = await toasts.getToastCount(); + return toastCount > 1; + }); + const warningToast = await toasts.getToastElement(2); + const textEl = await warningToast.findByClassName('euiToastBody'); + const text: string = await textEl.getVisibleText(); + expect(text).to.contain('Watch out!'); + }); }); } From 898db1593c911445b647cc867af3baae464dacef Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 26 Jul 2021 14:01:48 -0700 Subject: [PATCH 4/4] Remove isWarningResponse --- examples/search_examples/public/search/app.tsx | 7 ++----- src/plugins/data/common/search/utils.ts | 7 ------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/examples/search_examples/public/search/app.tsx b/examples/search_examples/public/search/app.tsx index 8ffe23e0267f2..bfb41160ae963 100644 --- a/examples/search_examples/public/search/app.tsx +++ b/examples/search_examples/public/search/app.tsx @@ -47,7 +47,6 @@ import { isErrorResponse, } from '../../../../src/plugins/data/public'; import { IMyStrategyResponse } from '../../common/types'; -import { isWarningResponse } from '../../../../src/plugins/data/common'; interface SearchExamplesAppDeps { notifications: CoreStart['notifications']; @@ -228,7 +227,7 @@ export const SearchExamplesApp = ({ } ); searchSubscription$.unsubscribe(); - if (isWarningResponse(res)) { + if (res.warning) { notifications.toasts.addWarning({ title: 'Warning', text: mountReactNode(res.warning), @@ -590,9 +589,7 @@ export const SearchExamplesApp = ({ When fetching data from Elasticsearch, there are several different ways warnings and - errors may be returned. In general, it is recommended to use the utility functions - isWarningResponse and isErrorResponse when - handling responses. + errors may be returned. In general, it is recommended to surface these in the UX. { export const isPartialResponse = (response?: IKibanaSearchResponse) => { return Boolean(response && response.isRunning && response.isPartial); }; - -/** - * @returns true if request has warnings that should be surfaced to the end user - */ -export const isWarningResponse = (response?: IKibanaSearchResponse) => { - return Boolean(response && response.warning); -};