From a89dcc36cf7ec3a97f631f5552eadd1644e4dcf8 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Wed, 12 Jun 2024 08:04:55 -0600 Subject: [PATCH] [Embeddable Rebuild] Better error handling (#185903) Closes https://github.com/elastic/kibana/issues/184962 ## Summary This PR adds error handling to the `ReactEmbeddableRenderer` component so that, if any error is thrown when trying to build the API / embeddable (i.e. when the state is deserialized, when the embeddable is being built, etc.), the resulting panel will be deletable. The previous error handling assumed that the API would always exist, which isn't necessarily true - this handles this scenario by providing a dummy API (which provides a blocking error **and** access to the parent API to allow for panel deletion) to the presentation panel when an error is thrown in `ReactEmbeddableRenderer` | Before | After | |--------|--------| | ![image](https://github.com/elastic/kibana/assets/8698078/e3ed043e-5ab6-49a2-b5ce-d5ee7a5f394f) | ![image](https://github.com/elastic/kibana/assets/8698078/406cf098-88b6-4387-ac25-43f711505c2b) | | No panel actions available, since no API is provided to the presentation panel | ![image](https://github.com/elastic/kibana/assets/8698078/45010401-63dc-4259-8b1b-3914d791f2cf) | | ![image](https://github.com/elastic/kibana/assets/8698078/f3973864-a1cb-4f2d-ab80-9630a9d620c8) | ![image](https://github.com/elastic/kibana/assets/8698078/e36bd761-b932-401a-85e3-5d9059b63392) | ### 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 ### 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) --- .../react_embeddable_renderer.test.tsx | 37 ++++ .../react_embeddable_renderer.tsx | 168 ++++++++++-------- 2 files changed, 135 insertions(+), 70 deletions(-) diff --git a/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.test.tsx b/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.test.tsx index 3b6c8cddaa085b..5c753777eae9b7 100644 --- a/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.test.tsx +++ b/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.test.tsx @@ -212,6 +212,43 @@ describe('react embeddable renderer', () => { ) ); }); + + it('catches error when thrown in deserialize', async () => { + const buildEmbeddable = jest.fn(); + const errorInInitializeFactory: ReactEmbeddableFactory<{ name: string; bork: string }> = { + ...testEmbeddableFactory, + type: 'errorInDeserialize', + buildEmbeddable, + deserializeState: (state) => { + throw new Error('error in deserialize'); + }, + }; + registerReactEmbeddableFactory('errorInDeserialize', () => + Promise.resolve(errorInInitializeFactory) + ); + setupPresentationPanelServices(); + + const onApiAvailable = jest.fn(); + const embeddable = render( + ({ + getSerializedStateForChild: () => ({ + rawState: {}, + }), + })} + /> + ); + + await waitFor(() => expect(embeddable.getByTestId('errorMessageMarkdown')).toBeInTheDocument()); + expect(onApiAvailable).not.toBeCalled(); + expect(buildEmbeddable).not.toBeCalled(); + expect(embeddable.getByTestId('errorMessageMarkdown')).toHaveTextContent( + 'error in deserialize' + ); + }); }); describe('reactEmbeddable phase events', () => { diff --git a/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx b/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx index a10420a33f67fe..8d8a632caec1fa 100644 --- a/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx +++ b/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx @@ -6,7 +6,11 @@ * Side Public License, v 1. */ -import { HasSerializedChildState, SerializedPanelState } from '@kbn/presentation-containers'; +import { + apiIsPresentationContainer, + HasSerializedChildState, + SerializedPanelState, +} from '@kbn/presentation-containers'; import { PresentationPanel, PresentationPanelProps } from '@kbn/presentation-panel-plugin/public'; import { apiPublishesDataLoading, @@ -20,9 +24,9 @@ import { v4 as generateId } from 'uuid'; import { getReactEmbeddableFactory } from './react_embeddable_registry'; import { initializeReactEmbeddableState } from './react_embeddable_state'; import { + BuildReactEmbeddableApiRegistration, DefaultEmbeddableApi, SetReactEmbeddableApiRegistration, - BuildReactEmbeddableApiRegistration, } from './types'; const ON_STATE_CHANGE_DEBOUNCE = 100; @@ -94,12 +98,6 @@ export const ReactEmbeddableRenderer = < const factory = await getReactEmbeddableFactory(type); const subscriptions = new Subscription(); - const { initialState, startStateDiffing } = await initializeReactEmbeddableState< - SerializedState, - Api, - RuntimeState - >(uuid, factory, parentApi); - const setApi = ( apiRegistration: SetReactEmbeddableApiRegistration ) => { @@ -114,75 +112,105 @@ export const ReactEmbeddableRenderer = < return fullApi; }; - const buildApi = ( - apiRegistration: BuildReactEmbeddableApiRegistration, - comparators: StateComparators - ) => { - if (onAnyStateChange) { - /** - * To avoid unnecessary re-renders, only subscribe to the comparator publishing subjects if - * an `onAnyStateChange` callback is provided - */ - const comparatorDefinitions: Array< - ComparatorDefinition - > = Object.values(comparators); - subscriptions.add( - combineLatest(comparatorDefinitions.map((comparator) => comparator[0])) - .pipe( - skip(1), - debounceTime(ON_STATE_CHANGE_DEBOUNCE), - switchMap(() => { - const isAsync = - apiRegistration.serializeState.prototype?.name === 'AsyncFunction'; - return isAsync - ? (apiRegistration.serializeState() as Promise< - SerializedPanelState - >) - : Promise.resolve(apiRegistration.serializeState()); + const buildEmbeddable = async () => { + const { initialState, startStateDiffing } = await initializeReactEmbeddableState< + SerializedState, + Api, + RuntimeState + >(uuid, factory, parentApi); + + const buildApi = ( + apiRegistration: BuildReactEmbeddableApiRegistration, + comparators: StateComparators + ) => { + if (onAnyStateChange) { + /** + * To avoid unnecessary re-renders, only subscribe to the comparator publishing subjects if + * an `onAnyStateChange` callback is provided + */ + const comparatorDefinitions: Array< + ComparatorDefinition + > = Object.values(comparators); + subscriptions.add( + combineLatest(comparatorDefinitions.map((comparator) => comparator[0])) + .pipe( + skip(1), + debounceTime(ON_STATE_CHANGE_DEBOUNCE), + switchMap(() => { + const isAsync = + apiRegistration.serializeState.prototype?.name === 'AsyncFunction'; + return isAsync + ? (apiRegistration.serializeState() as Promise< + SerializedPanelState + >) + : Promise.resolve(apiRegistration.serializeState()); + }) + ) + .subscribe((serializedState) => { + onAnyStateChange(serializedState); }) - ) - .subscribe((serializedState) => { - onAnyStateChange(serializedState); - }) + ); + } + + const { unsavedChanges, resetUnsavedChanges, cleanup, snapshotRuntimeState } = + startStateDiffing(comparators); + + const fullApi = setApi({ + ...apiRegistration, + unsavedChanges, + resetUnsavedChanges, + snapshotRuntimeState, + } as unknown as SetReactEmbeddableApiRegistration); + + cleanupFunction.current = () => cleanup(); + return fullApi; + }; + + const { api, Component } = await factory.buildEmbeddable( + initialState, + buildApi, + uuid, + parentApi, + setApi + ); + + if (apiPublishesDataLoading(api)) { + subscriptions.add( + api.dataLoading.subscribe((loading) => reportPhaseChange(Boolean(loading))) ); + } else { + reportPhaseChange(false); } - const { unsavedChanges, resetUnsavedChanges, cleanup, snapshotRuntimeState } = - startStateDiffing(comparators); - - const fullApi = setApi({ - ...apiRegistration, - unsavedChanges, - resetUnsavedChanges, - snapshotRuntimeState, - } as unknown as SetReactEmbeddableApiRegistration); - - cleanupFunction.current = () => cleanup(); - return fullApi; + return { api, Component }; }; - const { api, Component } = await factory.buildEmbeddable( - initialState, - buildApi, - uuid, - parentApi, - setApi - ); - - if (apiPublishesDataLoading(api)) { - subscriptions.add( - api.dataLoading.subscribe((loading) => reportPhaseChange(Boolean(loading))) - ); - } else { - reportPhaseChange(false); + try { + const { api, Component } = await buildEmbeddable(); + return React.forwardRef((_, ref) => { + // expose the api into the imperative handle + useImperativeHandle(ref, () => api, []); + + return ; + }); + } catch (e) { + /** + * critical error encountered when trying to build the api / embeddable; + * since no API is available, create a dummy API that allows the panel to be deleted + * */ + const errorApi = { + uuid, + blockingError: new BehaviorSubject(e), + } as unknown as Api; + if (apiIsPresentationContainer(parentApi)) { + errorApi.parentApi = parentApi; + } + return React.forwardRef((_, ref) => { + // expose the dummy error api into the imperative handle + useImperativeHandle(ref, () => errorApi, []); + return null; + }); } - - return React.forwardRef((_, ref) => { - // expose the api into the imperative handle - useImperativeHandle(ref, () => api, []); - - return ; - }); })(); }, /**