From a091d35f11985c7e6a153ebcf8aeada8f44d8eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 22 Oct 2021 11:52:05 +0200 Subject: [PATCH] ErrorBoundary: Support recovering from errors in PanelChrome & PanelRenderer (#40785) * ErrorBoundary: Support recovering from errors in PanelChrome & PanelRenderer * Rename recover to dependencies * Pushed an update that fixed test and adds new error mode to DebugPanel --- .../ErrorBoundary/ErrorBoundary.test.tsx | 35 ++++++++++++++++ .../ErrorBoundary/ErrorBoundary.tsx | 40 ++++++++++++++++--- .../dashboard/dashgrid/PanelChrome.tsx | 20 +++++++--- .../panel/components/PanelRenderer.tsx | 40 ++++++++++--------- public/app/features/sandbox/TestStuffPage.tsx | 14 ++++--- public/app/plugins/panel/debug/DebugPanel.tsx | 23 +++++------ public/app/plugins/panel/debug/module.tsx | 4 +- public/app/plugins/panel/debug/types.ts | 1 + public/app/routes/routes.tsx | 6 +++ 9 files changed, 133 insertions(+), 50 deletions(-) diff --git a/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.test.tsx b/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.test.tsx index a6f8368633f..ea648f1a84f 100644 --- a/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.test.tsx +++ b/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.test.tsx @@ -33,4 +33,39 @@ describe('ErrorBoundary', () => { expect(context.contexts.react).toHaveProperty('componentStack'); expect(context.contexts.react.componentStack).toMatch(/^\s+at ErrorThrower (.*)\s+at ErrorBoundary (.*)\s*$/); }); + + it('should recover when when recover props change', async () => { + const problem = new Error('things went terribly wrong'); + let renderCount = 0; + + const { rerender } = render( + + {({ error }) => { + if (!error) { + renderCount += 1; + return ; + } else { + return

{error.message}

; + } + }} + + ); + + await screen.findByText(problem.message); + + rerender( + + {({ error }) => { + if (!error) { + renderCount += 1; + return ; + } else { + return

{error.message}

; + } + }} + + ); + + expect(renderCount).toBe(2); + }); }); diff --git a/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx b/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx index 3ec96221a1b..6496443c96f 100644 --- a/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx +++ b/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx @@ -14,6 +14,12 @@ export interface ErrorBoundaryApi { interface Props { children: (r: ErrorBoundaryApi) => ReactNode; + /** Will re-render children after error if recover values changes */ + dependencies?: any[]; + /** Callback called on error */ + onError?: (error: Error) => void; + /** Callback error state is cleared due to recover props change */ + onRecover?: () => void; } interface State { @@ -29,10 +35,29 @@ export class ErrorBoundary extends PureComponent { componentDidCatch(error: Error, errorInfo: ErrorInfo) { captureException(error, { contexts: { react: { componentStack: errorInfo.componentStack } } }); - this.setState({ - error: error, - errorInfo: errorInfo, - }); + this.setState({ error, errorInfo }); + + if (this.props.onError) { + this.props.onError(error); + } + } + + componentDidUpdate(prevProps: Props) { + const { dependencies, onRecover } = this.props; + + if (this.state.error) { + if (dependencies && prevProps.dependencies) { + for (let i = 0; i < dependencies.length; i++) { + if (dependencies[i] !== prevProps.dependencies[i]) { + this.setState({ error: null, errorInfo: null }); + if (onRecover) { + onRecover(); + } + break; + } + } + } + } } render() { @@ -60,6 +85,9 @@ export interface ErrorBoundaryAlertProps { /** 'page' will render full page error with stacktrace. 'alertbox' will render an . Default 'alertbox' */ style?: 'page' | 'alertbox'; + + /** Will re-render children after error if recover values changes */ + dependencies?: any[]; } export class ErrorBoundaryAlert extends PureComponent { @@ -69,10 +97,10 @@ export class ErrorBoundaryAlert extends PureComponent { }; render() { - const { title, children, style } = this.props; + const { title, children, style, dependencies } = this.props; return ( - + {({ error, errorInfo }) => { if (!errorInfo) { return children; diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index 44f89fa57e6..7f9995eef85 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -309,12 +309,17 @@ export class PanelChrome extends PureComponent { this.props.panel.updateFieldConfig(config); }; - onPanelError = (message: string) => { - if (this.state.errorMessage !== message) { - this.setState({ errorMessage: message }); + onPanelError = (error: Error) => { + const errorMessage = error.message || DEFAULT_PLUGIN_ERROR; + if (this.state.errorMessage !== errorMessage) { + this.setState({ errorMessage }); } }; + onPanelErrorRecover = () => { + this.setState({ errorMessage: undefined }); + }; + onAnnotationCreate = async (event: AnnotationEventUIModel) => { const isRegion = event.from !== event.to; const anno = { @@ -458,7 +463,7 @@ export class PanelChrome extends PureComponent { } render() { - const { dashboard, panel, isViewing, isEditing, width, height } = this.props; + const { dashboard, panel, isViewing, isEditing, width, height, plugin } = this.props; const { errorMessage, data } = this.state; const { transparent } = panel; @@ -489,10 +494,13 @@ export class PanelChrome extends PureComponent { alertState={alertState} data={data} /> - + {({ error }) => { if (error) { - this.onPanelError(error.message || DEFAULT_PLUGIN_ERROR); return null; } return this.renderPanel(width, height); diff --git a/public/app/features/panel/components/PanelRenderer.tsx b/public/app/features/panel/components/PanelRenderer.tsx index c1358f94139..72ceb49bafd 100644 --- a/public/app/features/panel/components/PanelRenderer.tsx +++ b/public/app/features/panel/components/PanelRenderer.tsx @@ -5,7 +5,7 @@ import { appEvents } from 'app/core/core'; import { useAsync } from 'react-use'; import { getPanelOptionsWithDefaults, OptionDefaults } from '../../dashboard/state/getPanelOptionsWithDefaults'; import { importPanelPlugin } from '../../plugins/importPanelPlugin'; -import { useTheme2 } from '@grafana/ui'; +import { ErrorBoundaryAlert, useTheme2 } from '@grafana/ui'; const defaultFieldConfig = { defaults: {}, overrides: [] }; @@ -51,24 +51,26 @@ export function PanelRenderer

(pr const PanelComponent = plugin.panel; return ( - str} - onOptionsChange={onOptionsChange} - onFieldConfigChange={setFieldConfig} - onChangeTimeRange={onChangeTimeRange} - eventBus={appEvents} - /> + + str} + onOptionsChange={onOptionsChange} + onFieldConfigChange={setFieldConfig} + onChangeTimeRange={onChangeTimeRange} + eventBus={appEvents} + /> + ); } diff --git a/public/app/features/sandbox/TestStuffPage.tsx b/public/app/features/sandbox/TestStuffPage.tsx index 6f17f924e0f..5430cc72b75 100644 --- a/public/app/features/sandbox/TestStuffPage.tsx +++ b/public/app/features/sandbox/TestStuffPage.tsx @@ -1,4 +1,3 @@ -import { LegendDisplayMode } from '@grafana/schema'; import { ApplyFieldOverrideOptions, DataTransformerConfig, @@ -7,7 +6,7 @@ import { NavModelItem, PanelData, } from '@grafana/data'; -import { Table, TimeSeries } from '@grafana/ui'; +import { Table } from '@grafana/ui'; import { config } from 'app/core/config'; import React, { FC, useMemo, useState } from 'react'; import { useObservable } from 'react-use'; @@ -16,6 +15,7 @@ import { PanelQueryRunner } from '../query/state/PanelQueryRunner'; import { QueryGroupOptions } from 'app/types'; import Page from '../../core/components/Page/Page'; import AutoSizer from 'react-virtualized-auto-sizer'; +import { PanelRenderer } from '../panel/components/PanelRenderer'; interface State { queryRunner: PanelQueryRunner; @@ -66,12 +66,14 @@ export const TestStuffPage: FC = () => { {({ width }) => { return (

- diff --git a/public/app/plugins/panel/debug/DebugPanel.tsx b/public/app/plugins/panel/debug/DebugPanel.tsx index aabd71c4f00..dc964027353 100644 --- a/public/app/plugins/panel/debug/DebugPanel.tsx +++ b/public/app/plugins/panel/debug/DebugPanel.tsx @@ -13,18 +13,17 @@ export class DebugPanel extends Component { render() { const { options } = this.props; - if (options.mode === DebugMode.Events) { - return ; + switch (options.mode) { + case DebugMode.Events: + return ; + case DebugMode.Cursor: + return ; + case DebugMode.State: + return ; + case DebugMode.ThrowError: + throw new Error('I failed you and for that i am deeply sorry'); + default: + return ; } - - if (options.mode === DebugMode.Cursor) { - return ; - } - - if (options.mode === DebugMode.State) { - return ; - } - - return ; } } diff --git a/public/app/plugins/panel/debug/module.tsx b/public/app/plugins/panel/debug/module.tsx index a5169af95fe..a2bfecebece 100644 --- a/public/app/plugins/panel/debug/module.tsx +++ b/public/app/plugins/panel/debug/module.tsx @@ -5,7 +5,7 @@ import { DebugMode, DebugPanelOptions } from './types'; export const plugin = new PanelPlugin(DebugPanel).useFieldConfig().setPanelOptions((builder) => { builder - .addRadio({ + .addSelect({ path: 'mode', name: 'Mode', defaultValue: DebugMode.Render, @@ -14,7 +14,9 @@ export const plugin = new PanelPlugin(DebugPanel).useFieldCon { label: 'Render', value: DebugMode.Render }, { label: 'Events', value: DebugMode.Events }, { label: 'Cursor', value: DebugMode.Cursor }, + { label: 'Cursor', value: DebugMode.Cursor }, { label: 'Share state', value: DebugMode.State }, + { label: 'Throw error', value: DebugMode.ThrowError }, ], }, }) diff --git a/public/app/plugins/panel/debug/types.ts b/public/app/plugins/panel/debug/types.ts index 0db4d56f237..d4563b13826 100644 --- a/public/app/plugins/panel/debug/types.ts +++ b/public/app/plugins/panel/debug/types.ts @@ -13,6 +13,7 @@ export enum DebugMode { Events = 'events', Cursor = 'cursor', State = 'State', + ThrowError = 'ThrowError', } export interface DebugPanelOptions { diff --git a/public/app/routes/routes.tsx b/public/app/routes/routes.tsx index bc85a765316..5cfde164306 100644 --- a/public/app/routes/routes.tsx +++ b/public/app/routes/routes.tsx @@ -502,6 +502,12 @@ export function getAppRoutes(): RouteDescriptor[] { () => import(/* webpackChunkName: "BenchmarksPage"*/ 'app/features/sandbox/BenchmarksPage') ), }, + { + path: '/sandbox/test', + component: SafeDynamicImport( + () => import(/* webpackChunkName: "TestStuffPage"*/ 'app/features/sandbox/TestStuffPage') + ), + }, { path: '/dashboards/f/:uid/:slug/library-panels', component: SafeDynamicImport(