[searchService] Dedupe shard error toasts (#131776)

* dedupe shard error toasts

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

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,43 +64,61 @@ 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', () => {
       (extract.extractWarnings as jest.Mock).mockImplementation(() => [warnings[2]]);
       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', () => {
       (extract.extractWarnings as jest.Mock).mockImplementation(() => [warnings[1]]);
       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', () => {
       (extract.extractWarnings as jest.Mock).mockImplementation(() => [warnings[1], warnings[2]]);
       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);
     });

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,33 +23,61 @@ 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<string, ReturnType<typeof addWarning>> = {};
+
+  return (
+    debounceKey: string,
+    title: string,
+    text?: string | MountPoint<HTMLElement> | 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;
   }
 
   // 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);
 }
 
 /**

src/plugins/data/public/search/search_service.test.ts

@@ -27,6 +27,7 @@ describe('Search service', () => {
   let mockCoreSetup: MockedKeys<CoreSetup>;
   let mockCoreStart: MockedKeys<CoreStart>;
   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',
         });
       });
     });

src/plugins/data/public/search/search_service.ts

@@ -240,7 +240,12 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
       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<ISearchSetup, ISearchStart> {
       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<ISearchSetup, ISearchStart> {
             return;
           }
 
-          handleWarnings(request.json as SearchRequest, rawResponse, theme, cb);
+          handleWarnings({
+            request: request.json as SearchRequest,
+            response: rawResponse,
+            theme,
+            callback,
+          });
         });
       },
       session: this.sessionService,

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]);
       });