Skip to content

Commit

Permalink
[Time to Visualize] Embeddable Error Handling Without ReplacePanel (#…
Browse files Browse the repository at this point in the history
…82201)

Fixed embeddable error handling so that fatal errors are caught and displayed with an errorEmbeddable no matter when they occur.
  • Loading branch information
ThomThomson authored Nov 5, 2020
1 parent a89176e commit 7c66880
Show file tree
Hide file tree
Showing 21 changed files with 181 additions and 32 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [Embeddable](./kibana-plugin-plugins-embeddable-public.embeddable.md) &gt; [fatalError](./kibana-plugin-plugins-embeddable-public.embeddable.fatalerror.md)

## Embeddable.fatalError property

<b>Signature:</b>

```typescript
fatalError?: Error;
```
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export declare abstract class Embeddable<TEmbeddableInput extends EmbeddableInpu
| Property | Modifiers | Type | Description |
| --- | --- | --- | --- |
| [fatalError](./kibana-plugin-plugins-embeddable-public.embeddable.fatalerror.md) | | <code>Error</code> | |
| [id](./kibana-plugin-plugins-embeddable-public.embeddable.id.md) | | <code>string</code> | |
| [input](./kibana-plugin-plugins-embeddable-public.embeddable.input.md) | | <code>TEmbeddableInput</code> | |
| [isContainer](./kibana-plugin-plugins-embeddable-public.embeddable.iscontainer.md) | | <code>boolean</code> | |
Expand All @@ -43,6 +44,7 @@ export declare abstract class Embeddable<TEmbeddableInput extends EmbeddableInpu
| [getOutput$()](./kibana-plugin-plugins-embeddable-public.embeddable.getoutput_.md) | | |
| [getRoot()](./kibana-plugin-plugins-embeddable-public.embeddable.getroot.md) | | Returns the top most parent embeddable, or itself if this embeddable is not within a parent. |
| [getTitle()](./kibana-plugin-plugins-embeddable-public.embeddable.gettitle.md) | | |
| [onFatalError(e)](./kibana-plugin-plugins-embeddable-public.embeddable.onfatalerror.md) | | |
| [reload()](./kibana-plugin-plugins-embeddable-public.embeddable.reload.md) | | Reload will be called when there is a request to refresh the data or view, even if the input data did not change. |
| [render(el)](./kibana-plugin-plugins-embeddable-public.embeddable.render.md) | | |
| [supportedTriggers()](./kibana-plugin-plugins-embeddable-public.embeddable.supportedtriggers.md) | | |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [Embeddable](./kibana-plugin-plugins-embeddable-public.embeddable.md) &gt; [onFatalError](./kibana-plugin-plugins-embeddable-public.embeddable.onfatalerror.md)

## Embeddable.onFatalError() method

<b>Signature:</b>

```typescript
protected onFatalError(e: Error): void;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| e | <code>Error</code> | |

<b>Returns:</b>

`void`

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [IEmbeddable](./kibana-plugin-plugins-embeddable-public.iembeddable.md) &gt; [fatalError](./kibana-plugin-plugins-embeddable-public.iembeddable.fatalerror.md)

## IEmbeddable.fatalError property

If this embeddable has encountered a fatal error, that error will be stored here

<b>Signature:</b>

```typescript
fatalError?: Error;
```
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface IEmbeddable<I extends EmbeddableInput = EmbeddableInput, O exte
| Property | Type | Description |
| --- | --- | --- |
| [enhancements](./kibana-plugin-plugins-embeddable-public.iembeddable.enhancements.md) | <code>object</code> | Extra abilities added to Embeddable by <code>*_enhanced</code> plugins. |
| [fatalError](./kibana-plugin-plugins-embeddable-public.iembeddable.fatalerror.md) | <code>Error</code> | If this embeddable has encountered a fatal error, that error will be stored here |
| [id](./kibana-plugin-plugins-embeddable-public.iembeddable.id.md) | <code>string</code> | A unique identifier for this embeddable. Mainly only used by containers to map their Panel States to a child embeddable instance. |
| [isContainer](./kibana-plugin-plugins-embeddable-public.iembeddable.iscontainer.md) | <code>boolean</code> | Is this embeddable an instance of a Container class, can it contain nested embeddables? |
| [parent](./kibana-plugin-plugins-embeddable-public.iembeddable.parent.md) | <code>IContainer</code> | If this embeddable is nested inside a container, this will contain a reference to its parent. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { coreMock } from '../../../../../core/public/mocks';
import { CoreStart } from 'kibana/public';
import { AddToLibraryAction } from '.';
import { embeddablePluginMock } from 'src/plugins/embeddable/public/mocks';
import { ViewMode } from '../../../../embeddable/public';
import { ErrorEmbeddable, ViewMode } from '../../../../embeddable/public';

const { setup, doStart } = embeddablePluginMock.createInstance();
setup.registerEmbeddableFactory(
Expand Down Expand Up @@ -86,6 +86,16 @@ beforeEach(async () => {
}
});

test('Add to library is incompatible with Error Embeddables', async () => {
const action = new AddToLibraryAction({ toasts: coreStart.notifications.toasts });
const errorEmbeddable = new ErrorEmbeddable(
'Wow what an awful error',
{ id: ' 404' },
embeddable.getRoot() as IContainer
);
expect(await action.isCompatible({ embeddable: errorEmbeddable })).toBe(false);
});

test('Add to library is compatible when embeddable on dashboard has value type input', async () => {
const action = new AddToLibraryAction({ toasts: coreStart.notifications.toasts });
embeddable.updateInput(await embeddable.getInputAsValueType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
PanelNotFoundError,
EmbeddableInput,
isReferenceOrValueEmbeddable,
isErrorEmbeddable,
} from '../../../../embeddable/public';
import { NotificationsStart } from '../../../../../core/public';
import { DashboardPanelState, DASHBOARD_CONTAINER_TYPE, DashboardContainer } from '..';
Expand Down Expand Up @@ -61,7 +62,8 @@ export class AddToLibraryAction implements ActionByType<typeof ACTION_ADD_TO_LIB

public async isCompatible({ embeddable }: AddToLibraryActionContext) {
return Boolean(
embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
!isErrorEmbeddable(embeddable) &&
embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
embeddable.getRoot() &&
embeddable.getRoot().isContainer &&
embeddable.getRoot().type === DASHBOARD_CONTAINER_TYPE &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { isErrorEmbeddable, IContainer } from '../../embeddable_plugin';
import { isErrorEmbeddable, IContainer, ErrorEmbeddable } from '../../embeddable_plugin';
import { DashboardContainer, DashboardPanelState } from '../embeddable';
import { getSampleDashboardInput, getSampleDashboardPanel } from '../test_helpers';
import {
Expand Down Expand Up @@ -86,6 +86,16 @@ beforeEach(async () => {
}
});

test('Clone is incompatible with Error Embeddables', async () => {
const action = new ClonePanelAction(coreStart);
const errorEmbeddable = new ErrorEmbeddable(
'Wow what an awful error',
{ id: ' 404' },
embeddable.getRoot() as IContainer
);
expect(await action.isCompatible({ embeddable: errorEmbeddable })).toBe(false);
});

test('Clone adds a new embeddable', async () => {
const dashboard = embeddable.getRoot() as IContainer;
const originalPanelCount = Object.keys(dashboard.getInput().panels).length;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
PanelNotFoundError,
EmbeddableInput,
SavedObjectEmbeddableInput,
isErrorEmbeddable,
} from '../../../../embeddable/public';
import {
placePanelBeside,
Expand Down Expand Up @@ -66,7 +67,8 @@ export class ClonePanelAction implements ActionByType<typeof ACTION_CLONE_PANEL>

public async isCompatible({ embeddable }: ClonePanelActionContext) {
return Boolean(
embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
!isErrorEmbeddable(embeddable) &&
embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
embeddable.getRoot() &&
embeddable.getRoot().isContainer &&
embeddable.getRoot().type === DASHBOARD_CONTAINER_TYPE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { coreMock } from '../../../../../core/public/mocks';
import { CoreStart } from 'kibana/public';
import { LibraryNotificationAction, UnlinkFromLibraryAction } from '.';
import { embeddablePluginMock } from 'src/plugins/embeddable/public/mocks';
import { ViewMode } from '../../../../embeddable/public';
import { ErrorEmbeddable, IContainer, ViewMode } from '../../../../embeddable/public';

const { setup, doStart } = embeddablePluginMock.createInstance();
setup.registerEmbeddableFactory(
Expand Down Expand Up @@ -87,6 +87,16 @@ beforeEach(async () => {
embeddable.updateInput({ viewMode: ViewMode.EDIT });
});

test('Notification is incompatible with Error Embeddables', async () => {
const action = new LibraryNotificationAction(unlinkAction);
const errorEmbeddable = new ErrorEmbeddable(
'Wow what an awful error',
{ id: ' 404' },
embeddable.getRoot() as IContainer
);
expect(await action.isCompatible({ embeddable: errorEmbeddable })).toBe(false);
});

test('Notification is shown when embeddable on dashboard has reference type input', async () => {
const action = new LibraryNotificationAction(unlinkAction);
embeddable.updateInput(await embeddable.getInputAsRefType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@

import { i18n } from '@kbn/i18n';
import React from 'react';
import { IEmbeddable, ViewMode, isReferenceOrValueEmbeddable } from '../../embeddable_plugin';
import {
IEmbeddable,
ViewMode,
isReferenceOrValueEmbeddable,
isErrorEmbeddable,
} from '../../embeddable_plugin';
import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin';
import { reactToUiComponent } from '../../../../kibana_react/public';
import { UnlinkFromLibraryAction } from '.';
Expand Down Expand Up @@ -79,6 +84,7 @@ export class LibraryNotificationAction implements ActionByType<typeof ACTION_LIB

public isCompatible = async ({ embeddable }: LibraryNotificationActionContext) => {
return (
!isErrorEmbeddable(embeddable) &&
embeddable.getRoot().isContainer &&
embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
isReferenceOrValueEmbeddable(embeddable) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ import { coreMock } from '../../../../../core/public/mocks';
import { CoreStart } from 'kibana/public';
import { UnlinkFromLibraryAction } from '.';
import { embeddablePluginMock } from 'src/plugins/embeddable/public/mocks';
import { ViewMode, SavedObjectEmbeddableInput } from '../../../../embeddable/public';
import {
ViewMode,
SavedObjectEmbeddableInput,
ErrorEmbeddable,
} from '../../../../embeddable/public';

const { setup, doStart } = embeddablePluginMock.createInstance();
setup.registerEmbeddableFactory(
Expand Down Expand Up @@ -80,6 +84,16 @@ beforeEach(async () => {
embeddable.updateInput({ viewMode: ViewMode.EDIT });
});

test('Unlink is incompatible with Error Embeddables', async () => {
const action = new UnlinkFromLibraryAction({ toasts: coreStart.notifications.toasts });
const errorEmbeddable = new ErrorEmbeddable(
'Wow what an awful error',
{ id: ' 404' },
embeddable.getRoot() as IContainer
);
expect(await action.isCompatible({ embeddable: errorEmbeddable })).toBe(false);
});

test('Unlink is compatible when embeddable on dashboard has reference type input', async () => {
const action = new UnlinkFromLibraryAction({ toasts: coreStart.notifications.toasts });
embeddable.updateInput(await embeddable.getInputAsRefType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
PanelNotFoundError,
EmbeddableInput,
isReferenceOrValueEmbeddable,
isErrorEmbeddable,
} from '../../../../embeddable/public';
import { NotificationsStart } from '../../../../../core/public';
import { DashboardPanelState, DASHBOARD_CONTAINER_TYPE, DashboardContainer } from '..';
Expand Down Expand Up @@ -61,7 +62,8 @@ export class UnlinkFromLibraryAction implements ActionByType<typeof ACTION_UNLIN

public async isCompatible({ embeddable }: UnlinkFromLibraryActionContext) {
return Boolean(
embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
!isErrorEmbeddable(embeddable) &&
embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
embeddable.getRoot() &&
embeddable.getRoot().isContainer &&
embeddable.getRoot().type === DASHBOARD_CONTAINER_TYPE &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import ReactDOM from 'react-dom';
import angular from 'angular';
import deepEqual from 'fast-deep-equal';

import { Observable, pipe, Subscription, merge } from 'rxjs';
import { Observable, pipe, Subscription, merge, EMPTY } from 'rxjs';
import {
filter,
map,
Expand All @@ -35,6 +35,7 @@ import {
startWith,
switchMap,
distinctUntilChanged,
catchError,
} from 'rxjs/operators';
import { History } from 'history';
import { SavedObjectSaveOpts, SavedObject } from 'src/plugins/saved_objects/public';
Expand Down Expand Up @@ -464,7 +465,10 @@ export class DashboardAppController {
switchMap((newChildIds: string[]) =>
merge(
...newChildIds.map((childId) =>
dashboardContainer!.getChild(childId).getOutput$()
dashboardContainer!
.getChild(childId)
.getOutput$()
.pipe(catchError(() => EMPTY))
)
)
)
Expand Down
15 changes: 12 additions & 3 deletions src/plugins/embeddable/public/lib/embeddables/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export abstract class Embeddable<
public readonly isContainer: boolean = false;
public abstract readonly type: string;
public readonly id: string;
public fatalError?: Error;

protected output: TEmbeddableOutput;
protected input: TEmbeddableInput;
Expand Down Expand Up @@ -88,9 +89,12 @@ export abstract class Embeddable<
map(({ title }) => title || ''),
distinctUntilChanged()
)
.subscribe((title) => {
this.renderComplete.setTitle(title);
});
.subscribe(
(title) => {
this.renderComplete.setTitle(title);
},
() => {}
);
}

public getIsContainer(): this is IContainer {
Expand Down Expand Up @@ -193,6 +197,11 @@ export abstract class Embeddable<
}
}

protected onFatalError(e: Error) {
this.fatalError = e;
this.output$.error(e);
}

private onResetInput(newInput: TEmbeddableInput) {
if (!isEqual(this.input, newInput)) {
const oldLastReloadRequestTime = this.input.lastReloadRequestTime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const ERROR_EMBEDDABLE_TYPE = 'error';
export function isErrorEmbeddable<TEmbeddable extends IEmbeddable>(
embeddable: TEmbeddable | ErrorEmbeddable
): embeddable is ErrorEmbeddable {
return (embeddable as ErrorEmbeddable).error !== undefined;
return Boolean(embeddable.fatalError || (embeddable as ErrorEmbeddable).error !== undefined);
}

export class ErrorEmbeddable extends Embeddable<EmbeddableInput, EmbeddableOutput> {
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ export interface IEmbeddable<
*/
enhancements?: object;

/**
* If this embeddable has encountered a fatal error, that error will be stored here
**/
fatalError?: Error;

/**
* A functional representation of the isContainer variable, but helpful for typescript to
* know the shape if this returns true
Expand Down
27 changes: 20 additions & 7 deletions src/plugins/embeddable/public/lib/panel/embeddable_panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { EditPanelAction } from '../actions';
import { CustomizePanelModal } from './panel_header/panel_actions/customize_title/customize_panel_modal';
import { EmbeddableStart } from '../../plugin';
import { EmbeddableErrorLabel } from './embeddable_error_label';
import { EmbeddableStateTransfer } from '..';
import { EmbeddableStateTransfer, ErrorEmbeddable } from '..';

const sortByOrderField = (
{ order: orderA }: { order?: number },
Expand Down Expand Up @@ -85,6 +85,7 @@ interface State {
notifications: Array<Action<EmbeddableContext>>;
loading?: boolean;
error?: EmbeddableError;
errorEmbeddable?: ErrorEmbeddable;
}

interface PanelUniversalActions {
Expand Down Expand Up @@ -199,6 +200,9 @@ export class EmbeddablePanel extends React.Component<Props, State> {
if (this.parentSubscription) {
this.parentSubscription.unsubscribe();
}
if (this.state.errorEmbeddable) {
this.state.errorEmbeddable.destroy();
}
this.props.embeddable.destroy();
}

Expand Down Expand Up @@ -257,12 +261,21 @@ export class EmbeddablePanel extends React.Component<Props, State> {
public componentDidMount() {
if (this.embeddableRoot.current) {
this.subscription.add(
this.props.embeddable.getOutput$().subscribe((output: EmbeddableOutput) => {
this.setState({
error: output.error,
loading: output.loading,
});
})
this.props.embeddable.getOutput$().subscribe(
(output: EmbeddableOutput) => {
this.setState({
error: output.error,
loading: output.loading,
});
},
(error) => {
if (this.embeddableRoot.current) {
const errorEmbeddable = new ErrorEmbeddable(error, { id: this.props.embeddable.id });
errorEmbeddable.render(this.embeddableRoot.current);
this.setState({ errorEmbeddable });
}
}
)
);
this.props.embeddable.render(this.embeddableRoot.current);
}
Expand Down
Loading

0 comments on commit 7c66880

Please sign in to comment.