[Console] Fixes for console error handling and loading of autocomplete (#58587) (#59178)

* Fix console error handling when offline

In cases when the client cannot connect to server
the UI would get stuck in a loading state. We need to handle
that case explicitly to stop the progress spinner and report
the error correctly.

* Fix editor request cycle. Request should always complete

The bug was that the request could error in such a way that the
requestFail dispatch was not being called. Leaving the loading spinner
running and an unhelpful error message would appear.

Also partly fixed the loading of autocomplete data and cleaned up
a legacy import.

* Fixed loading of mappings in as they were updated from
settings modal.

* Fix the mappings update logic

TODO, this function needs to be revisited, but for now
it is convenient to have the Settings service passed in every
time so that the poller can be updated.

* Fix poll interval

* Address PR feedback

Rename variable (instance -> editorRegistry) and remove unused
file

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.test.tsx

@@ -25,7 +25,7 @@ import { I18nProvider } from '@kbn/i18n/react';
 import { act } from 'react-dom/test-utils';
 import * as sinon from 'sinon';
 
-import { notificationServiceMock } from '../../../../../../../../core/public/mocks';
+import { serviceContextMock } from '../../../../contexts/services_context.mock';
 
 import { nextTick } from 'test_utils/enzyme_helpers';
 import {
@@ -61,21 +61,7 @@ describe('Legacy (Ace) Console Editor Component Smoke Test', () => {
 
   beforeEach(() => {
     document.queryCommandSupported = sinon.fake(() => true);
-    mockedAppContextValue = {
-      elasticsearchUrl: 'test',
-      services: {
-        trackUiMetric: { count: () => {}, load: () => {} },
-        settings: {} as any,
-        storage: {} as any,
-        history: {
-          getSavedEditorState: () => ({} as any),
-          updateCurrentState: jest.fn(),
-        } as any,
-        notifications: notificationServiceMock.createSetupContract(),
-        objectStorageClient: {} as any,
-      },
-      docLinkVersion: 'NA',
-    };
+    mockedAppContextValue = serviceContextMock.create();
   });
 
   afterEach(() => {

src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx

@@ -65,7 +65,7 @@ const inputId = 'ConAppInputTextarea';
 
 function EditorUI({ initialTextValue }: EditorProps) {
   const {
-    services: { history, notifications },
+    services: { history, notifications, settings: settingsService },
     docLinkVersion,
     elasticsearchUrl,
   } = useServicesContext();
@@ -172,7 +172,7 @@ function EditorUI({ initialTextValue }: EditorProps) {
     setInputEditor(editor);
     setTextArea(editorRef.current!.querySelector('textarea'));
 
-    mappings.retrieveAutoCompleteInfo();
+    mappings.retrieveAutoCompleteInfo(settingsService, settingsService.getAutocomplete());
 
     const unsubscribeResizer = subscribeResizeChecker(editorRef.current!, editor);
     setupAutosave();
@@ -182,7 +182,7 @@ function EditorUI({ initialTextValue }: EditorProps) {
       mappings.clearSubscriptions();
       window.removeEventListener('hashchange', onHashChange);
     };
-  }, [saveCurrentTextObject, initialTextValue, history, setInputEditor]);
+  }, [saveCurrentTextObject, initialTextValue, history, setInputEditor, settingsService]);
 
   useEffect(() => {
     const { current: editor } = editorInstanceRef;

src/plugins/console/public/application/containers/editor/legacy/console_editor/editor_output.tsx

@@ -30,7 +30,10 @@ import { createReadOnlyAceEditor, CustomAceEditor } from '../../../../models/leg
 import { subscribeResizeChecker } from '../subscribe_console_resize_checker';
 import { applyCurrentSettings } from './apply_editor_settings';
 
-function modeForContentType(contentType: string) {
+function modeForContentType(contentType?: string) {
+  if (!contentType) {
+    return 'ace/mode/text';
+  }
   if (contentType.indexOf('application/json') >= 0) {
     return 'ace/mode/json';
   } else if (contentType.indexOf('application/yaml') >= 0) {

src/plugins/console/public/application/containers/settings.tsx

@@ -23,7 +23,7 @@ import { AutocompleteOptions, DevToolsSettingsModal } from '../components';
 // @ts-ignore
 import mappings from '../../lib/mappings/mappings';
 import { useServicesContext, useEditorActionContext } from '../contexts';
-import { DevToolsSettings } from '../../services';
+import { DevToolsSettings, Settings as SettingsService } from '../../services';
 
 const getAutocompleteDiff = (newSettings: DevToolsSettings, prevSettings: DevToolsSettings) => {
   return Object.keys(newSettings.autocomplete).filter(key => {
@@ -32,11 +32,12 @@ const getAutocompleteDiff = (newSettings: DevToolsSettings, prevSettings: DevToo
   });
 };
 
-const refreshAutocompleteSettings = (selectedSettings: any) => {
-  mappings.retrieveAutoCompleteInfo(selectedSettings);
+const refreshAutocompleteSettings = (settings: SettingsService, selectedSettings: any) => {
+  mappings.retrieveAutoCompleteInfo(settings, selectedSettings);
 };
 
 const fetchAutocompleteSettingsIfNeeded = (
+  settings: SettingsService,
   newSettings: DevToolsSettings,
   prevSettings: DevToolsSettings
 ) => {
@@ -60,10 +61,10 @@ const fetchAutocompleteSettingsIfNeeded = (
         },
         {}
       );
-      mappings.retrieveAutoCompleteInfo(changedSettings.autocomplete);
-    } else if (isPollingChanged) {
+      mappings.retrieveAutoCompleteInfo(settings, changedSettings);
+    } else if (isPollingChanged && newSettings.polling) {
       // If the user has turned polling on, then we'll fetch all selected autocomplete settings.
-      mappings.retrieveAutoCompleteInfo();
+      mappings.retrieveAutoCompleteInfo(settings, settings.getAutocomplete());
     }
   }
 };
@@ -81,7 +82,7 @@ export function Settings({ onClose }: Props) {
 
   const onSaveSettings = (newSettings: DevToolsSettings) => {
     const prevSettings = settings.toJSON();
-    fetchAutocompleteSettingsIfNeeded(newSettings, prevSettings);
+    fetchAutocompleteSettingsIfNeeded(settings, newSettings, prevSettings);
 
     // Update the new settings in localStorage
     settings.updateSettings(newSettings);
@@ -98,7 +99,9 @@ export function Settings({ onClose }: Props) {
     <DevToolsSettingsModal
       onClose={onClose}
       onSaveSettings={onSaveSettings}
-      refreshAutocompleteSettings={refreshAutocompleteSettings}
+      refreshAutocompleteSettings={(selectedSettings: any) =>
+        refreshAutocompleteSettings(settings, selectedSettings)
+      }
       settings={settings.toJSON()}
     />
   );

src/plugins/console/public/application/contexts/editor_context/editor_registry.ts

@@ -20,7 +20,7 @@
 import { SenseEditor } from '../../models/sense_editor';
 
 export class EditorRegistry {
-  inputEditor: SenseEditor | undefined;
+  private inputEditor: SenseEditor | undefined;
 
   setInputEditor(inputEditor: SenseEditor) {
     this.inputEditor = inputEditor;

src/plugins/console/public/application/contexts/services_context.mock.ts

@@ -0,0 +1,43 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { notificationServiceMock } from '../../../../../core/public/mocks';
+import { HistoryMock } from '../../services/history.mock';
+import { SettingsMock } from '../../services/settings.mock';
+import { StorageMock } from '../../services/storage.mock';
+
+import { ContextValue } from './services_context';
+
+export const serviceContextMock = {
+  create: (): ContextValue => {
+    const storage = new StorageMock({} as any, 'test');
+    (storage.keys as jest.Mock).mockImplementation(() => []);
+    return {
+      elasticsearchUrl: 'test',
+      services: {
+        trackUiMetric: { count: () => {}, load: () => {} },
+        storage,
+        settings: new SettingsMock(storage),
+        history: new HistoryMock(storage),
+        notifications: notificationServiceMock.createSetupContract(),
+        objectStorageClient: {} as any,
+      },
+      docLinkVersion: 'NA',
+    };
+  },
+};

src/plugins/console/public/application/contexts/services_context.tsx

@@ -50,7 +50,7 @@ export function ServicesContextProvider({ children, value }: ContextProps) {
 export const useServicesContext = () => {
   const context = useContext(ServicesContext);
   if (context === undefined) {
-    throw new Error('useAppContext must be used inside the AppContextProvider.');
+    throw new Error('useServicesContext must be used inside the ServicesContextProvider.');
   }
   return context;
 };

src/plugins/console/public/application/hooks/use_send_current_request_to_es/use_send_current_request_to_es.test.tsx

@@ -0,0 +1,108 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+jest.mock('./send_request_to_es', () => ({ sendRequestToES: jest.fn() }));
+jest.mock('../../contexts/editor_context/editor_registry', () => ({
+  instance: { getInputEditor: jest.fn() },
+}));
+jest.mock('./track', () => ({ track: jest.fn() }));
+jest.mock('../../contexts/request_context', () => ({ useRequestActionContext: jest.fn() }));
+
+import React from 'react';
+import { renderHook, act } from '@testing-library/react-hooks';
+
+import { ContextValue, ServicesContextProvider } from '../../contexts';
+import { serviceContextMock } from '../../contexts/services_context.mock';
+import { useRequestActionContext } from '../../contexts/request_context';
+import { instance as editorRegistry } from '../../contexts/editor_context/editor_registry';
+
+import { sendRequestToES } from './send_request_to_es';
+import { useSendCurrentRequestToES } from './use_send_current_request_to_es';
+
+describe('useSendCurrentRequestToES', () => {
+  let mockContextValue: ContextValue;
+  let dispatch: (...args: any[]) => void;
+  const contexts = ({ children }: { children?: any }) => (
+    <ServicesContextProvider value={mockContextValue}>{children}</ServicesContextProvider>
+  );
+
+  beforeEach(() => {
+    mockContextValue = serviceContextMock.create();
+    dispatch = jest.fn();
+    (useRequestActionContext as jest.Mock).mockReturnValue(dispatch);
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('calls send request to ES', async () => {
+    // Set up mocks
+    (mockContextValue.services.settings.toJSON as jest.Mock).mockReturnValue({});
+    // This request should succeed
+    (sendRequestToES as jest.Mock).mockResolvedValue([]);
+    (editorRegistry.getInputEditor as jest.Mock).mockImplementation(() => ({
+      getRequestsInRange: () => ['test'],
+    }));
+
+    const { result } = renderHook(() => useSendCurrentRequestToES(), { wrapper: contexts });
+    await act(() => result.current());
+    expect(sendRequestToES).toHaveBeenCalledWith({ requests: ['test'] });
+
+    // Second call should be the request success
+    const [, [requestSucceededCall]] = (dispatch as jest.Mock).mock.calls;
+    expect(requestSucceededCall).toEqual({ type: 'requestSuccess', payload: { data: [] } });
+  });
+
+  it('handles known errors', async () => {
+    // Set up mocks
+    (sendRequestToES as jest.Mock).mockRejectedValue({ response: 'nada' });
+    (editorRegistry.getInputEditor as jest.Mock).mockImplementation(() => ({
+      getRequestsInRange: () => ['test'],
+    }));
+
+    const { result } = renderHook(() => useSendCurrentRequestToES(), { wrapper: contexts });
+    await act(() => result.current());
+    // Second call should be the request failure
+    const [, [requestFailedCall]] = (dispatch as jest.Mock).mock.calls;
+
+    // The request must have concluded
+    expect(requestFailedCall).toEqual({ type: 'requestFail', payload: { response: 'nada' } });
+  });
+
+  it('handles unknown errors', async () => {
+    // Set up mocks
+    (sendRequestToES as jest.Mock).mockRejectedValue(NaN /* unexpected error value */);
+    (editorRegistry.getInputEditor as jest.Mock).mockImplementation(() => ({
+      getRequestsInRange: () => ['test'],
+    }));
+
+    const { result } = renderHook(() => useSendCurrentRequestToES(), { wrapper: contexts });
+    await act(() => result.current());
+    // Second call should be the request failure
+    const [, [requestFailedCall]] = (dispatch as jest.Mock).mock.calls;
+
+    // The request must have concluded
+    expect(requestFailedCall).toEqual({ type: 'requestFail', payload: undefined });
+    // It also notified the user
+    expect(mockContextValue.services.notifications.toasts.addError).toHaveBeenCalledWith(NaN, {
+      title: 'Unknown Request Error',
+    });
+  });
+});

src/plugins/console/public/application/hooks/use_send_current_request_to_es/use_send_current_request_to_es.ts

@@ -64,7 +64,7 @@ export const useSendCurrentRequestToES = () => {
         // or templates may have changed, so we'll need to update this data. Assume that if
         // the user disables polling they're trying to optimize performance or otherwise
         // preserve resources, so they won't want this request sent either.
-        mappings.retrieveAutoCompleteInfo();
+        mappings.retrieveAutoCompleteInfo(settings, settings.getAutocomplete());
       }
 
       dispatch({
@@ -74,12 +74,16 @@ export const useSendCurrentRequestToES = () => {
         },
       });
     } catch (e) {
-      if (e.response?.contentType) {
+      if (e?.response) {
         dispatch({
           type: 'requestFail',
           payload: e,
         });
       } else {
+        dispatch({
+          type: 'requestFail',
+          payload: undefined,
+        });
         notifications.toasts.addError(e, {
           title: i18n.translate('console.notification.unknownRequestErrorTitle', {
             defaultMessage: 'Unknown Request Error',

src/plugins/console/public/application/stores/request.ts

@@ -26,7 +26,7 @@ import { ESRequestResult } from '../hooks/use_send_current_request_to_es/send_re
 export type Actions =
   | { type: 'sendRequest'; payload: undefined }
   | { type: 'requestSuccess'; payload: { data: ESRequestResult[] } }
-  | { type: 'requestFail'; payload: ESRequestResult<string> };
+  | { type: 'requestFail'; payload: ESRequestResult<string> | undefined };
 
 export interface Store {
   requestInFlight: boolean;

src/plugins/console/public/lib/mappings/mappings.js

@@ -17,8 +17,6 @@
  * under the License.
  */
 
-import { legacyBackDoorToSettings } from '../../application';
-
 const $ = require('jquery');
 const _ = require('lodash');
 const es = require('../es/es');
@@ -255,7 +253,6 @@ function clear() {
 }
 
 function retrieveSettings(settingsKey, settingsToRetrieve) {
-  const currentSettings = legacyBackDoorToSettings().getAutocomplete();
   const settingKeyToPathMap = {
     fields: '_mapping',
     indices: '_aliases',
@@ -263,16 +260,17 @@ function retrieveSettings(settingsKey, settingsToRetrieve) {
   };
 
   // Fetch autocomplete info if setting is set to true, and if user has made changes.
-  if (currentSettings[settingsKey] && settingsToRetrieve[settingsKey]) {
+  if (settingsToRetrieve[settingsKey] === true) {
     return es.send('GET', settingKeyToPathMap[settingsKey], null);
   } else {
     const settingsPromise = new $.Deferred();
-    // If a user has saved settings, but a field remains checked and unchanged, no need to make changes
-    if (currentSettings[settingsKey]) {
+    if (settingsToRetrieve[settingsKey] === false) {
+      // If the user doesn't want autocomplete suggestions, then clear any that exist
+      return settingsPromise.resolveWith(this, [[JSON.stringify({})]]);
+    } else {
+      // If the user doesn't want autocomplete suggestions, then clear any that exist
       return settingsPromise.resolve();
     }
-    // If the user doesn't want autocomplete suggestions, then clear any that exist
-    return settingsPromise.resolveWith(this, [[JSON.stringify({})]]);
   }
 }
 
@@ -293,9 +291,12 @@ function clearSubscriptions() {
   }
 }
 
-function retrieveAutoCompleteInfo(
-  settingsToRetrieve = legacyBackDoorToSettings().getAutocomplete()
-) {
+/**
+ *
+ * @param settings Settings A way to retrieve the current settings
+ * @param settingsToRetrieve any
+ */
+function retrieveAutoCompleteInfo(settings, settingsToRetrieve) {
   clearSubscriptions();
 
   const mappingPromise = retrieveSettings('fields', settingsToRetrieve);
@@ -334,8 +335,8 @@ function retrieveAutoCompleteInfo(
     pollTimeoutId = setTimeout(() => {
       // This looks strange/inefficient, but it ensures correct behavior because we don't want to send
       // a scheduled request if the user turns off polling.
-      if (legacyBackDoorToSettings().getPolling()) {
-        retrieveAutoCompleteInfo();
+      if (settings.getPolling()) {
+        retrieveAutoCompleteInfo(settings, settings.getAutocomplete());
       }
     }, POLL_INTERVAL);
   });

src/plugins/console/public/services/history.mock.ts

@@ -0,0 +1,31 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { History } from './history';
+
+export class HistoryMock extends History {
+  addToHistory = jest.fn();
+  change = jest.fn();
+  clearHistory = jest.fn();
+  deleteLegacySavedEditorState = jest.fn();
+  getHistory = jest.fn();
+  getHistoryKeys = jest.fn();
+  getLegacySavedEditorState = jest.fn();
+  updateCurrentState = jest.fn();
+}

src/plugins/console/public/services/settings.mock.ts

@@ -0,0 +1,35 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Settings } from './settings';
+
+export class SettingsMock extends Settings {
+  getAutocomplete = jest.fn();
+  getFontSize = jest.fn();
+  getPolling = jest.fn();
+  getTripleQuotes = jest.fn();
+  getWrapMode = jest.fn();
+  setAutocomplete = jest.fn();
+  setFontSize = jest.fn();
+  setPolling = jest.fn();
+  setTripleQuotes = jest.fn();
+  setWrapMode = jest.fn();
+  toJSON = jest.fn();
+  updateSettings = jest.fn();
+}

src/plugins/console/public/services/storage.mock.ts

@@ -0,0 +1,32 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Storage } from './storage';
+
+export class StorageMock extends Storage {
+  delete = jest.fn();
+  decode = jest.fn();
+  decodeKey = jest.fn();
+  encodeKey = jest.fn();
+  encode = jest.fn();
+  has = jest.fn();
+  keys = jest.fn();
+  get = jest.fn();
+  set = jest.fn();
+}