diff --git a/src/plugins/data/public/search/fetch/handle_warnings.test.ts b/src/plugins/data/public/search/fetch/handle_warnings.test.ts index 8a1bb86b9f71c..0d644d8dc0811 100644 --- a/src/plugins/data/public/search/fetch/handle_warnings.test.ts +++ b/src/plugins/data/public/search/fetch/handle_warnings.test.ts @@ -45,13 +45,17 @@ const warnings: SearchResponseWarning[] = [ }, ]; +const sessionId = 'abcd'; + describe('Filtering and showing warnings', () => { const notifications = notificationServiceMock.createStartContract(); + jest.useFakeTimers('modern'); describe('handleWarnings', () => { const request = { body: {} }; beforeEach(() => { jest.resetAllMocks(); + jest.advanceTimersByTime(30000); setNotifications(notifications); (notifications.toasts.addWarning as jest.Mock).mockReset(); (extract.extractWarnings as jest.Mock).mockImplementation(() => warnings); @@ -60,10 +64,16 @@ describe('Filtering and showing warnings', () => { test('should notify if timed out', () => { (extract.extractWarnings as jest.Mock).mockImplementation(() => [warnings[0]]); const response = { rawResponse: { timed_out: true } } as unknown as estypes.SearchResponse; - handleWarnings(request, response, theme); + handleWarnings({ request, response, theme }); + // test debounce, addWarning should only be called once + handleWarnings({ request, response, theme }); expect(notifications.toasts.addWarning).toBeCalledTimes(1); expect(notifications.toasts.addWarning).toBeCalledWith({ title: 'Something timed out!' }); + + // test debounce, call addWarning again due to sessionId + handleWarnings({ request, response, theme, sessionId }); + expect(notifications.toasts.addWarning).toBeCalledTimes(2); }); test('should notify if shards failed for unknown type/reason', () => { @@ -71,10 +81,16 @@ describe('Filtering and showing warnings', () => { const response = { rawResponse: { _shards: { failed: 1, total: 2, successful: 1, skipped: 1 } }, } as unknown as estypes.SearchResponse; - handleWarnings(request, response, theme); + handleWarnings({ request, response, theme }); + // test debounce, addWarning should only be called once + handleWarnings({ request, response, theme }); expect(notifications.toasts.addWarning).toBeCalledTimes(1); expect(notifications.toasts.addWarning).toBeCalledWith({ title: 'Some shards failed!' }); + + // test debounce, call addWarning again due to sessionId + handleWarnings({ request, response, theme, sessionId }); + expect(notifications.toasts.addWarning).toBeCalledTimes(2); }); test('should add mount point for shard modal failure button if warning.text is provided', () => { @@ -82,13 +98,19 @@ describe('Filtering and showing warnings', () => { const response = { rawResponse: { _shards: { failed: 1, total: 2, successful: 1, skipped: 1 } }, } as unknown as estypes.SearchResponse; - handleWarnings(request, response, theme); + handleWarnings({ request, response, theme }); + // test debounce, addWarning should only be called once + handleWarnings({ request, response, theme }); expect(notifications.toasts.addWarning).toBeCalledTimes(1); expect(notifications.toasts.addWarning).toBeCalledWith({ title: 'Some shards failed!', text: expect.any(Function), }); + + // test debounce, call addWarning again due to sessionId + handleWarnings({ request, response, theme, sessionId }); + expect(notifications.toasts.addWarning).toBeCalledTimes(2); }); test('should notify once if the response contains multiple failures', () => { @@ -96,7 +118,7 @@ describe('Filtering and showing warnings', () => { const response = { rawResponse: { _shards: { failed: 1, total: 2, successful: 1, skipped: 1 } }, } as unknown as estypes.SearchResponse; - handleWarnings(request, response, theme); + handleWarnings({ request, response, theme }); expect(notifications.toasts.addWarning).toBeCalledTimes(1); expect(notifications.toasts.addWarning).toBeCalledWith({ @@ -111,7 +133,7 @@ describe('Filtering and showing warnings', () => { const response = { rawResponse: { _shards: { failed: 1, total: 2, successful: 1, skipped: 1 } }, } as unknown as estypes.SearchResponse; - handleWarnings(request, response, theme, callback); + handleWarnings({ request, response, theme, callback }); expect(notifications.toasts.addWarning).toBeCalledTimes(1); expect(notifications.toasts.addWarning).toBeCalledWith({ title: 'Some shards failed!' }); @@ -122,7 +144,7 @@ describe('Filtering and showing warnings', () => { const response = { rawResponse: { _shards: { failed: 1, total: 2, successful: 1, skipped: 1 } }, } as unknown as estypes.SearchResponse; - handleWarnings(request, response, theme, callback); + handleWarnings({ request, response, theme, callback }); expect(notifications.toasts.addWarning).toBeCalledTimes(0); }); diff --git a/src/plugins/data/public/search/fetch/handle_warnings.tsx b/src/plugins/data/public/search/fetch/handle_warnings.tsx index 0ffb6389f81c4..1720ea1f76c0c 100644 --- a/src/plugins/data/public/search/fetch/handle_warnings.tsx +++ b/src/plugins/data/public/search/fetch/handle_warnings.tsx @@ -7,10 +7,12 @@ */ import { estypes } from '@elastic/elasticsearch'; +import { debounce } from 'lodash'; import { EuiSpacer } from '@elastic/eui'; import { ThemeServiceStart } from '@kbn/core/public'; import { toMountPoint } from '@kbn/kibana-react-plugin/public'; import React from 'react'; +import type { MountPoint } from '@kbn/core/public'; import { SearchRequest } from '..'; import { getNotifications } from '../../services'; import { ShardFailureOpenModalButton, ShardFailureRequest } from '../../shard_failure_modal'; @@ -21,23 +23,53 @@ import { } from '../types'; import { extractWarnings } from './extract_warnings'; +const getDebouncedWarning = () => { + const addWarning = () => { + const { toasts } = getNotifications(); + return debounce(toasts.addWarning.bind(toasts), 30000, { + leading: true, + }); + }; + const memory: Record> = {}; + + return ( + debounceKey: string, + title: string, + text?: string | MountPoint | undefined + ) => { + memory[debounceKey] = memory[debounceKey] || addWarning(); + return memory[debounceKey]({ title, text }); + }; +}; + +const debouncedWarningWithoutReason = getDebouncedWarning(); +const debouncedTimeoutWarning = getDebouncedWarning(); +const debouncedWarning = getDebouncedWarning(); + /** * @internal * All warnings are expected to come from the same response. Therefore all "text" properties, which contain the * response, will be the same. */ -export function handleWarnings( - request: SearchRequest, - response: estypes.SearchResponse, - theme: ThemeServiceStart, - cb?: WarningHandlerCallback -) { +export function handleWarnings({ + request, + response, + theme, + callback, + sessionId = '', +}: { + request: SearchRequest; + response: estypes.SearchResponse; + theme: ThemeServiceStart; + callback?: WarningHandlerCallback; + sessionId?: string; +}) { const warnings = extractWarnings(response); if (warnings.length === 0) { return; } - const internal = cb ? filterWarnings(warnings, cb) : warnings; + const internal = callback ? filterWarnings(warnings, callback) : warnings; if (internal.length === 0) { return; } @@ -45,9 +77,7 @@ export function handleWarnings( // timeout notification const [timeout] = internal.filter((w) => w.type === 'timed_out'); if (timeout) { - getNotifications().toasts.addWarning({ - title: timeout.message, - }); + debouncedTimeoutWarning(sessionId + timeout.message, timeout.message); } // shard warning failure notification @@ -75,12 +105,12 @@ export function handleWarnings( { theme$: theme.theme$ } ); - getNotifications().toasts.addWarning({ title, text }); + debouncedWarning(sessionId + warning.text, title, text); return; } // timeout warning, or shard warning with no failure reason - getNotifications().toasts.addWarning({ title }); + debouncedWarningWithoutReason(sessionId + title, title); } /** diff --git a/src/plugins/data/public/search/search_service.test.ts b/src/plugins/data/public/search/search_service.test.ts index 432549698eed2..c1c8fc37a0e72 100644 --- a/src/plugins/data/public/search/search_service.test.ts +++ b/src/plugins/data/public/search/search_service.test.ts @@ -27,6 +27,7 @@ describe('Search service', () => { let mockCoreSetup: MockedKeys; let mockCoreStart: MockedKeys; const initializerContext = coreMock.createPluginInitializerContext(); + jest.useFakeTimers('modern'); initializerContext.config.get = jest.fn().mockReturnValue({ search: { aggs: { shardDelay: { enabled: false } }, sessions: { enabled: true } }, }); @@ -35,6 +36,7 @@ describe('Search service', () => { mockCoreSetup = coreMock.createSetup(); mockCoreStart = coreMock.createStart(); searchService = new SearchService(initializerContext); + jest.advanceTimersByTime(30000); }); describe('setup()', () => { @@ -217,7 +219,13 @@ describe('Search service', () => { const responder1 = inspector.adapter.start('request1'); const responder2 = inspector.adapter.start('request2'); responder1.ok(getMockResponseWithShards(shards)); - responder2.ok(getMockResponseWithShards(shards)); + responder2.ok({ + json: { + rawResponse: { + timed_out: true, + }, + }, + }); data.showWarnings(inspector.adapter, callback); @@ -227,8 +235,7 @@ describe('Search service', () => { text: expect.any(Function), }); expect(notifications.toasts.addWarning).nthCalledWith(2, { - title: '2 of 4 shards failed', - text: expect.any(Function), + title: 'Data might be incomplete because your request timed out', }); }); }); diff --git a/src/plugins/data/public/search/search_service.ts b/src/plugins/data/public/search/search_service.ts index c88201405d7f0..e7b753838edff 100644 --- a/src/plugins/data/public/search/search_service.ts +++ b/src/plugins/data/public/search/search_service.ts @@ -240,7 +240,12 @@ export class SearchService implements Plugin { onResponse: (request, response, options) => { if (!options.disableShardFailureWarning) { const { rawResponse } = response; - handleWarnings(request.body, rawResponse, theme); + handleWarnings({ + request: request.body, + response: rawResponse, + theme, + sessionId: options.sessionId, + }); } return response; }, @@ -271,7 +276,7 @@ export class SearchService implements Plugin { showError: (e) => { this.searchInterceptor.showError(e); }, - showWarnings: (adapter, cb) => { + showWarnings: (adapter, callback) => { adapter?.getRequests().forEach((request) => { const rawResponse = ( request.response?.json as { rawResponse: estypes.SearchResponse | undefined } @@ -281,7 +286,12 @@ export class SearchService implements Plugin { return; } - handleWarnings(request.json as SearchRequest, rawResponse, theme, cb); + handleWarnings({ + request: request.json as SearchRequest, + response: rawResponse, + theme, + callback, + }); }); }, session: this.sessionService, diff --git a/test/examples/search/warnings.ts b/test/examples/search/warnings.ts index fc1949549d66e..2cc674fb01024 100644 --- a/test/examples/search/warnings.ts +++ b/test/examples/search/warnings.ts @@ -159,7 +159,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { toasts = await find.allByCssSelector(toastsSelector); expect(toasts.length).to.be(2); }); - const expects = ['2 of 4 shards failed', 'Query result']; + const expects = ['Query result', '2 of 4 shards failed']; await asyncForEach(toasts, async (t, index) => { expect(await t.getVisibleText()).to.eql(expects[index]); });