[Controls] Add ability to recover from non-fatal error state (#158087)

Closes #156430 ## Summary The only reason a control embeddable should enter a fatal error state is if, for some reason, the embeddable actually cannot be created (for example, trying to create a control with a type that doesn't exist) - every other error should be considered **recoverable** as much as possible. So, this PR ensures that both the options list and the range slider are able to recover from most errors by switching from calling `onFatalError` to instead handling most errors internally via component state. > **Note** > The time slider control does not have any errors that would be considered "recoverable" because it is actually **much more difficult** to enter an error state for this control; so, I did not need to change anything for this control type. ### Errors: - **Recoverable error:** - **Before:** https://github.com/elastic/kibana/assets/8698078/7737a3e8-1c97-47ba-92ab-55f5e1a6c30a - **After:** https://github.com/elastic/kibana/assets/8698078/e4ced721-2b84-497e-8608-965877409bf5 - **Unrecoverable error:** To test this, I've created a dashboard saved object with a control type that does not exist: [controlTypeDoesNotExistDashboard.ndjson.zip](https://github.com/elastic/kibana/files/11547128/controlTypeDoesNotExistDashboard.ndjson.zip). Try importing this dashboard and ensure that you can actually see an error unlike the "before" state: - **Before:** ![image](https://github.com/elastic/kibana/assets/8698078/27c581b1-3fa8-4c07-b102-c952355dfd32) - **After:** ![image](https://github.com/elastic/kibana/assets/8698078/626ed696-4f8e-44b2-b449-3c2fa1ee1327) ### Flaky Test Runner - [Options list dashboard interavction (`test/functional/apps/dashboard_elements/controls/options_list/options_list_dashboard_interaction.ts`)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2316) ![image](https://github.com/elastic/kibana/assets/8698078/6b71c71c-a47b-4e84-834a-07b8c380a727) - [Range slider (`test/functional/apps/dashboard_elements/controls/range_slider.ts`)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2317) ![image](https://github.com/elastic/kibana/assets/8698078/fe1c9752-68d3-4bca-b31a-361217b91d8e) ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
elastic · May 31, 2023 · 3e3419d · 3e3419d
1 parent ccf0099
commit 3e3419d
Show file tree

Hide file tree

Showing 16 changed files with 194 additions and 128 deletions.
diff --git a/src/plugins/controls/public/control_group/component/control_error_component.tsx b/src/plugins/controls/public/control_group/component/control_error_component.tsx
@@ -0,0 +1,57 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import React, { useState } from 'react';
+
+import { EuiButtonEmpty, EuiPopover } from '@elastic/eui';
+import { FormattedMessage, I18nProvider } from '@kbn/i18n-react';
+import { Markdown } from '@kbn/kibana-react-plugin/public';
+
+interface ControlErrorProps {
+  error: Error | string;
+}
+
+export const ControlError = ({ error }: ControlErrorProps) => {
+  const [isPopoverOpen, setPopoverOpen] = useState(false);
+  const errorMessage = error instanceof Error ? error.message : error;
+
+  const popoverButton = (
+    <EuiButtonEmpty
+      color="danger"
+      iconSize="m"
+      iconType="error"
+      data-test-subj="control-frame-error"
+      onClick={() => setPopoverOpen((open) => !open)}
+      className={'errorEmbeddableCompact__button'}
+      textProps={{ className: 'errorEmbeddableCompact__text' }}
+    >
+      <FormattedMessage
+        id="controls.frame.error.message"
+        defaultMessage="An error occurred. View more"
+      />
+    </EuiButtonEmpty>
+  );
+
+  return (
+    <I18nProvider>
+      <EuiPopover
+        button={popoverButton}
+        isOpen={isPopoverOpen}
+        className="errorEmbeddableCompact__popover"
+        anchorClassName="errorEmbeddableCompact__popoverAnchor"
+        closePopover={() => setPopoverOpen(false)}
+      >
+        <Markdown
+          markdown={errorMessage}
+          openLinksInNewTab={true}
+          data-test-subj="errorMessageMarkdown"
+        />
+      </EuiPopover>
+    </I18nProvider>
+  );
+};
diff --git a/src/plugins/controls/public/control_group/component/control_frame_component.tsx b/src/plugins/controls/public/control_group/component/control_frame_component.tsx
@@ -10,16 +10,13 @@ import classNames from 'classnames';
 import React, { useEffect, useMemo, useState } from 'react';
 
 import {
-  EuiButtonEmpty,
   EuiFormControlLayout,
   EuiFormLabel,
   EuiFormRow,
   EuiLoadingChart,
-  EuiPopover,
   EuiToolTip,
 } from '@elastic/eui';
-import { FormattedMessage } from '@kbn/i18n-react';
-import { Markdown } from '@kbn/kibana-react-plugin/public';
+import { isErrorEmbeddable } from '@kbn/embeddable-plugin/public';
 import { FloatingActions } from '@kbn/presentation-util-plugin/public';
 
 import {
@@ -28,45 +25,7 @@ import {
 } from '../embeddable/control_group_container';
 import { ControlGroupStrings } from '../control_group_strings';
 import { useChildEmbeddable } from '../../hooks/use_child_embeddable';
-
-interface ControlFrameErrorProps {
-  error: Error;
-}
-
-const ControlFrameError = ({ error }: ControlFrameErrorProps) => {
-  const [isPopoverOpen, setPopoverOpen] = useState(false);
-  const popoverButton = (
-    <EuiButtonEmpty
-      color="danger"
-      iconSize="m"
-      iconType="error"
-      onClick={() => setPopoverOpen((open) => !open)}
-      className={'errorEmbeddableCompact__button'}
-      textProps={{ className: 'errorEmbeddableCompact__text' }}
-    >
-      <FormattedMessage
-        id="controls.frame.error.message"
-        defaultMessage="An error occurred. View more"
-      />
-    </EuiButtonEmpty>
-  );
-
-  return (
-    <EuiPopover
-      button={popoverButton}
-      isOpen={isPopoverOpen}
-      className="errorEmbeddableCompact__popover"
-      anchorClassName="errorEmbeddableCompact__popoverAnchor"
-      closePopover={() => setPopoverOpen(false)}
-    >
-      <Markdown
-        markdown={error.message}
-        openLinksInNewTab={true}
-        data-test-subj="errorMessageMarkdown"
-      />
-    </EuiPopover>
-  );
-};
+import { ControlError } from './control_error_component';
 
 export interface ControlFrameProps {
   customPrepend?: JSX.Element;
@@ -82,7 +41,6 @@ export const ControlFrame = ({
   embeddableType,
 }: ControlFrameProps) => {
   const embeddableRoot: React.RefObject<HTMLDivElement> = useMemo(() => React.createRef(), []);
-  const [fatalError, setFatalError] = useState<Error>();
 
   const controlGroup = useControlGroupContainer();
 
@@ -107,19 +65,14 @@ export const ControlFrame = ({
     const inputSubscription = embeddable
       ?.getInput$()
       .subscribe((newInput) => setTitle(newInput.title));
-    const errorSubscription = embeddable?.getOutput$().subscribe({
-      error: setFatalError,
-    });
     return () => {
       inputSubscription?.unsubscribe();
-      errorSubscription?.unsubscribe();
     };
   }, [embeddable, embeddableRoot]);
 
   const embeddableParentClassNames = classNames('controlFrame__control', {
     'controlFrame--twoLine': controlStyle === 'twoLine',
     'controlFrame--oneLine': controlStyle === 'oneLine',
-    'controlFrame--fatalError': !!fatalError,
   });
 
   function renderEmbeddablePrepend() {
@@ -149,18 +102,13 @@ export const ControlFrame = ({
         </>
       }
     >
-      {embeddable && !fatalError && (
+      {embeddable && (
         <div
           className={embeddableParentClassNames}
           id={`controlFrame--${embeddableId}`}
           ref={embeddableRoot}
         >
-          {fatalError && <ControlFrameError error={fatalError} />}
-        </div>
-      )}
-      {fatalError && (
-        <div className={embeddableParentClassNames} id={`controlFrame--${embeddableId}`}>
-          {<ControlFrameError error={fatalError} />}
+          {isErrorEmbeddable(embeddable) && <ControlError error={embeddable.error} />}
         </div>
       )}
       {!embeddable && (

diff --git a/src/plugins/controls/public/control_group/control_group.scss b/src/plugins/controls/public/control_group/control_group.scss
@@ -205,11 +205,4 @@ $controlMinWidth: $euiSize * 14;
   &--twoLine {
     top: (-$euiSizeXS) !important;
   }
-
-  &--fatalError {
-    padding: $euiSizeXS;
-    border-radius: $euiBorderRadius;
-    background-color: $euiColorEmptyShade;
-    box-shadow: 0 0 0 1px $euiColorLightShade;
-  }
 }
diff --git a/src/plugins/controls/public/options_list/components/options_list_control.tsx b/src/plugins/controls/public/options_list/components/options_list_control.tsx
@@ -19,6 +19,7 @@ import { OptionsListPopover } from './options_list_popover';
 import { useOptionsList } from '../embeddable/options_list_embeddable';
 
 import './options_list.scss';
+import { ControlError } from '../../control_group/component/control_error_component';
 
 export const OptionsListControl = ({
   typeaheadSubject,
@@ -31,6 +32,7 @@ export const OptionsListControl = ({
   const optionsList = useOptionsList();
   const dimensions = useResizeObserver(resizeRef.current);
 
+  const error = optionsList.select((state) => state.componentState.error);
   const isPopoverOpen = optionsList.select((state) => state.componentState.popoverOpen);
   const validSelections = optionsList.select((state) => state.componentState.validSelections);
   const invalidSelections = optionsList.select((state) => state.componentState.invalidSelections);
@@ -143,7 +145,9 @@ export const OptionsListControl = ({
     </div>
   );
 
-  return (
+  return error ? (
+    <ControlError error={error} />
+  ) : (
     <EuiFilterGroup
       className={classNames('optionsList--filterGroup', {
         'optionsList--filterGroupSingle': controlStyle !== 'twoLine',

diff --git a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx
@@ -247,7 +247,7 @@ export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput
             })
           );
       } catch (e) {
-        this.onFatalError(e);
+        this.dispatch.setErrorMessage(e.message);
       }
 
       this.dispatch.setDataViewId(this.dataView?.id);
@@ -267,7 +267,7 @@ export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput
 
         this.field = originalField.toSpec();
       } catch (e) {
-        this.onFatalError(e);
+        this.dispatch.setErrorMessage(e.message);
       }
       this.dispatch.setField(this.field);
     }
@@ -331,7 +331,7 @@ export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput
           // from prematurely setting loading to `false` and updating the suggestions to show "No results"
           return;
         }
-        this.onFatalError(response.error);
+        this.dispatch.setErrorMessage(response.error.message);
         return;
       }
 
@@ -365,11 +365,13 @@ export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput
       // publish filter
       const newFilters = await this.buildFilter();
       batch(() => {
+        this.dispatch.setErrorMessage(undefined);
         this.dispatch.setLoading(false);
         this.dispatch.publishFilters(newFilters);
       });
     } else {
       batch(() => {
+        this.dispatch.setErrorMessage(undefined);
         this.dispatch.updateQueryResults({
           availableOptions: [],
         });
@@ -412,14 +414,6 @@ export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput
     this.runOptionsListQuery();
   };
 
-  public onFatalError = (e: Error) => {
-    batch(() => {
-      this.dispatch.setLoading(false);
-      this.dispatch.setPopoverOpen(false);
-    });
-    super.onFatalError(e);
-  };
-
   public destroy = () => {
     super.destroy();
     this.cleanupStateTools();
@@ -433,6 +427,7 @@ export class OptionsListEmbeddable extends Embeddable<OptionsListEmbeddableInput
       ReactDOM.unmountComponentAtNode(this.node);
     }
     this.node = node;
+
     ReactDOM.render(
       <KibanaThemeProvider theme$={pluginServices.getServices().theme.theme$}>
         <OptionsListEmbeddableContext.Provider value={this}>

diff --git a/src/plugins/controls/public/options_list/options_list_reducers.ts b/src/plugins/controls/public/options_list/options_list_reducers.ts
@@ -103,6 +103,12 @@ export const optionsListReducers = {
     state.componentState.invalidSelections = invalidSelections;
     state.componentState.validSelections = validSelections;
   },
+  setErrorMessage: (
+    state: WritableDraft<OptionsListReduxState>,
+    action: PayloadAction<string | undefined>
+  ) => {
+    state.componentState.error = action.payload;
+  },
   setLoading: (state: WritableDraft<OptionsListReduxState>, action: PayloadAction<boolean>) => {
     state.output.loading = action.payload;
   },

diff --git a/src/plugins/controls/public/options_list/types.ts b/src/plugins/controls/public/options_list/types.ts
@@ -33,6 +33,7 @@ export interface OptionsListComponentState {
   totalCardinality?: number;
   popoverOpen: boolean;
   field?: FieldSpec;
+  error?: string;
 }
 
 // public only - redux embeddable state type

diff --git a/src/plugins/controls/public/range_slider/components/range_slider_control.tsx b/src/plugins/controls/public/range_slider/components/range_slider_control.tsx
@@ -21,6 +21,7 @@ import { useRangeSlider } from '../embeddable/range_slider_embeddable';
 import { RangeSliderPopover, EuiDualRangeRef } from './range_slider_popover';
 
 import './range_slider.scss';
+import { ControlError } from '../../control_group/component/control_error_component';
 
 const INVALID_CLASS = 'rangeSliderAnchor__fieldNumber--invalid';
 
@@ -32,7 +33,9 @@ export const RangeSliderControl: FC = () => {
 
   const min = rangeSlider.select((state) => state.componentState.min);
   const max = rangeSlider.select((state) => state.componentState.max);
+  const error = rangeSlider.select((state) => state.componentState.error);
   const isInvalid = rangeSlider.select((state) => state.componentState.isInvalid);
+
   const id = rangeSlider.select((state) => state.explicitInput.id);
   const value = rangeSlider.select((state) => state.explicitInput.value) ?? ['', ''];
   const isLoading = rangeSlider.select((state) => state.output.loading);
@@ -116,7 +119,9 @@ export const RangeSliderControl: FC = () => {
     </button>
   );
 
-  return (
+  return error ? (
+    <ControlError error={error} />
+  ) : (
     <EuiInputPopover
       input={button}
       isOpen={isPopoverOpen}