diff --git a/.changeset/few-cats-type.md b/.changeset/few-cats-type.md new file mode 100644 index 0000000000..45a59d5ada --- /dev/null +++ b/.changeset/few-cats-type.md @@ -0,0 +1,34 @@ +--- +'@backstage/core-app-api': patch +'@backstage/core-components': patch +'@backstage/core-plugin-api': patch +--- + +This change adds automatic error boundaries around extensions. + +This means that all exposed parts of a plugin are wrapped in a general error boundary component, that is plugin aware. The default design for the error box is borrowed from `@backstage/errors`. To override the default "fallback", one must provide a component named `ErrorBoundaryFallback` to `createApp`, like so: + +```ts +const app = createApp({ + components: { + ErrorBoundaryFallback: props => { + // a custom fallback component + return ( + <> +

Oops.

+

+ The plugin {props.plugin.getId()} failed with {props.error.message} +

+ + + ); + }, + }, +}); +``` + +The props here include: + +- `error`. An `Error` object or something that inherits it that represents the error that was thrown from any inner component. +- `resetError`. A callback that will simply attempt to mount the children of the error boundary again. +- `plugin`. A `BackstagePlugin` that can be used to look up info to be presented in the error message. For instance, you may want to keep a map of your internal plugins and team names or slack channels and present these when an error occurs. Typically, you'll do that by getting the plugin ID with `plugin.getId()`. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 36c4f7419b..c64f9db999 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -120,6 +120,7 @@ export type AppComponents = { BootErrorPage: ComponentType; Progress: ComponentType<{}>; Router: ComponentType<{}>; + ErrorBoundaryFallback: ComponentType; SignInPage?: ComponentType; }; @@ -220,6 +221,13 @@ export class ErrorApiForwarder implements ErrorApi { post(error: Error, context?: ErrorContext): void; } +// @public (undocumented) +export type ErrorBoundaryFallbackProps = { + plugin?: BackstagePlugin; + error: Error; + resetError: () => void; +}; + // @public (undocumented) export const FlatRoutes: (props: FlatRoutesProps) => JSX.Element | null; diff --git a/packages/core-app-api/src/app/App.test.tsx b/packages/core-app-api/src/app/App.test.tsx index cde3af9456..0539c2146c 100644 --- a/packages/core-app-api/src/app/App.test.tsx +++ b/packages/core-app-api/src/app/App.test.tsx @@ -159,6 +159,7 @@ describe('Integration Test', () => { BootErrorPage: () => null, Progress: () => null, Router: BrowserRouter, + ErrorBoundaryFallback: () => null, }; it('runs happy paths', async () => { diff --git a/packages/core-app-api/src/app/createApp.tsx b/packages/core-app-api/src/app/createApp.tsx index c11ac840f9..b93df63ff0 100644 --- a/packages/core-app-api/src/app/createApp.tsx +++ b/packages/core-app-api/src/app/createApp.tsx @@ -14,21 +14,27 @@ * limitations under the License. */ +import { AppConfig, JsonObject } from '@backstage/config'; +import { Button } from '@material-ui/core'; +import { ErrorPage, ErrorPanel, Progress } from '@backstage/core-components'; +import { darkTheme, lightTheme } from '@backstage/theme'; +import DarkIcon from '@material-ui/icons/Brightness2'; +import LightIcon from '@material-ui/icons/WbSunny'; import React, { PropsWithChildren } from 'react'; -import { AppOptions, BootErrorPageProps, AppConfigLoader } from './types'; -import { defaultAppIcons } from './icons'; import { BrowserRouter, MemoryRouter, useInRouterContext, } from 'react-router-dom'; -import LightIcon from '@material-ui/icons/WbSunny'; -import DarkIcon from '@material-ui/icons/Brightness2'; -import { ErrorPage, Progress } from '@backstage/core-components'; -import { defaultApis } from './defaultApis'; -import { lightTheme, darkTheme } from '@backstage/theme'; -import { AppConfig, JsonObject } from '@backstage/config'; import { PrivateAppImpl } from './App'; +import { defaultApis } from './defaultApis'; +import { defaultAppIcons } from './icons'; +import { + AppConfigLoader, + AppOptions, + BootErrorPageProps, + ErrorBoundaryFallbackProps, +} from './types'; /** * The default config loader, which expects that config is available at compile-time @@ -115,6 +121,23 @@ export function createApp(options?: AppOptions) { ); }; + const DefaultErrorBoundaryFallback = ({ + error, + resetError, + plugin, + }: ErrorBoundaryFallbackProps) => { + return ( + + + + ); + }; const apis = options?.apis ?? []; const icons = { ...defaultAppIcons, ...options?.icons }; @@ -124,6 +147,7 @@ export function createApp(options?: AppOptions) { BootErrorPage: DefaultBootErrorPage, Progress: Progress, Router: BrowserRouter, + ErrorBoundaryFallback: DefaultErrorBoundaryFallback, ...options?.components, }; const themes = options?.themes ?? [ diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index 85be0c5f53..b7a6754232 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -59,11 +59,18 @@ export type SignInPageProps = { onResult(result: SignInResult): void; }; +export type ErrorBoundaryFallbackProps = { + plugin?: BackstagePlugin; + error: Error; + resetError: () => void; +}; + export type AppComponents = { NotFoundErrorPage: ComponentType<{}>; BootErrorPage: ComponentType; Progress: ComponentType<{}>; Router: ComponentType<{}>; + ErrorBoundaryFallback: ComponentType; /** * An optional sign-in page that will be rendered instead of the AppRouter at startup. diff --git a/packages/core-app-api/src/routing/RoutingProvider.test.tsx b/packages/core-app-api/src/routing/RoutingProvider.test.tsx index 8f8ffc820b..b75e68e7f0 100644 --- a/packages/core-app-api/src/routing/RoutingProvider.test.tsx +++ b/packages/core-app-api/src/routing/RoutingProvider.test.tsx @@ -48,6 +48,7 @@ import { import { validateRoutes } from './validation'; import { RouteResolver } from './RouteResolver'; import { AnyRouteRef, RouteFunc } from './types'; +import { AppContextProvider } from '../app/AppContext'; const MockComponent = ({ children }: PropsWithChildren<{ path?: string }>) => ( <>{children} @@ -125,6 +126,12 @@ const Extension5 = plugin.provide( }), ); +const mockContext = { + getComponents: () => ({ Progress: () => null } as any), + getSystemIcon: jest.fn(), + getPlugins: jest.fn(), +}; + function withRoutingProvider( root: ReactElement, routeBindings: [ExternalRouteRef, RouteRef][] = [], @@ -154,20 +161,22 @@ function withRoutingProvider( describe('discovery', () => { it('should handle simple routeRef path creation for routeRefs used in other parts of the app', async () => { const root = ( - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + ); const rendered = render( @@ -202,23 +211,25 @@ describe('discovery', () => { it('should handle routeRefs with parameters', async () => { const root = ( - - - - - - - - + + + + + + + + + + ); const rendered = render(withRoutingProvider(root)); @@ -233,22 +244,24 @@ describe('discovery', () => { it('should handle relative routing within parameterized routePaths', async () => { const root = ( - - - - - - - - - - - - + + + + + + + + + + + + + + ); const rendered = render(withRoutingProvider(root)); diff --git a/packages/core-components/src/components/ErrorPanel/ErrorPanel.tsx b/packages/core-components/src/components/ErrorPanel/ErrorPanel.tsx new file mode 100644 index 0000000000..1f7d2a4be7 --- /dev/null +++ b/packages/core-components/src/components/ErrorPanel/ErrorPanel.tsx @@ -0,0 +1,114 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed 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 { List, ListItem, ListItemText, makeStyles } from '@material-ui/core'; +import React, { PropsWithChildren } from 'react'; +import { CopyTextButton } from '../CopyTextButton'; +import { WarningPanel } from '../WarningPanel'; + +const useStyles = makeStyles(theme => ({ + text: { + fontFamily: 'monospace', + whiteSpace: 'pre', + overflowX: 'auto', + marginRight: theme.spacing(2), + }, + divider: { + margin: theme.spacing(2), + }, +})); + +type ErrorListProps = { + error: string; + message: string; + request?: string; + stack?: string; + json?: string; +}; + +const ErrorList = ({ + error, + message, + stack, + children, +}: PropsWithChildren) => { + const classes = useStyles(); + + return ( + + + + + + + + + + + + {stack && ( + + + + + )} + + {children} + + ); +}; + +export type ErrorPanelProps = { + error: Error; + defaultExpanded?: boolean; + title?: string; +}; + +/** + * Renders a warning panel as the effect of an error. + */ +export const ErrorPanel = ({ + title, + error, + defaultExpanded, + children, +}: PropsWithChildren) => { + return ( + + + + ); +}; diff --git a/packages/core-components/src/components/ErrorPanel/index.ts b/packages/core-components/src/components/ErrorPanel/index.ts new file mode 100644 index 0000000000..7103266eab --- /dev/null +++ b/packages/core-components/src/components/ErrorPanel/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed 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. + */ + +export { ErrorPanel } from './ErrorPanel'; +export type { ErrorPanelProps } from './ErrorPanel'; diff --git a/packages/core-components/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx b/packages/core-components/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx index 52f7ab03f9..33da6261ec 100644 --- a/packages/core-components/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx +++ b/packages/core-components/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx @@ -15,17 +15,11 @@ */ import { ResponseError } from '@backstage/errors'; -import { - Divider, - List, - ListItem, - ListItemText, - makeStyles, -} from '@material-ui/core'; +import { Divider, ListItem, ListItemText, makeStyles } from '@material-ui/core'; import React from 'react'; import { CodeSnippet } from '../CodeSnippet'; import { CopyTextButton } from '../CopyTextButton'; -import { WarningPanel } from '../WarningPanel'; +import { ErrorPanel, ErrorPanelProps } from '../ErrorPanel'; const useStyles = makeStyles(theme => ({ text: { @@ -39,95 +33,25 @@ const useStyles = makeStyles(theme => ({ }, })); -type ResponseErrorListProps = { - error: string; - message: string; - request?: string; - stack?: string; - json?: string; -}; - -const ResponseErrorList = ({ - error, - request, - message, - stack, - json, -}: ResponseErrorListProps) => { - const classes = useStyles(); - - return ( - - - - - - - - - - {request && ( - - - - - )} - {stack && ( - - - - - )} - {json && ( - <> - - - } - /> - - - - )} - - ); -}; - -type Props = { - error: Error; -}; - /** - * Renders details about a failed server request. + * Renders a warning panel as the effect of a failed server request. * * Has special treatment for ResponseError errors, to display rich * server-provided information about what happened. */ -export const ResponseErrorDetails = ({ error }: Props) => { +export const ResponseErrorPanel = ({ + title, + error, + defaultExpanded, +}: ErrorPanelProps) => { + const classes = useStyles(); + if (error.name !== 'ResponseError') { return ( - ); } @@ -142,26 +66,32 @@ export const ResponseErrorDetails = ({ error }: Props) => { const jsonString = JSON.stringify(data, undefined, 2); return ( - - ); -}; - -/** - * Renders a warning panel as the effect of a failed server request. - * - * Has special treatment for ResponseError errors, to display rich - * server-provided information about what happened. - */ -export const ResponseErrorPanel = ({ error }: Props) => { - return ( - - - + + {requestString && ( + + + + + )} + <> + + + } + /> + + + + ); }; diff --git a/packages/core-components/src/components/ResponseErrorPanel/index.ts b/packages/core-components/src/components/ResponseErrorPanel/index.ts index ac62553fd5..1fc6221a44 100644 --- a/packages/core-components/src/components/ResponseErrorPanel/index.ts +++ b/packages/core-components/src/components/ResponseErrorPanel/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { ResponseErrorDetails, ResponseErrorPanel } from './ResponseErrorPanel'; +export { ResponseErrorPanel } from './ResponseErrorPanel'; diff --git a/packages/core-components/src/components/WarningPanel/WarningPanel.tsx b/packages/core-components/src/components/WarningPanel/WarningPanel.tsx index 1faf3bb142..3b280dea9d 100644 --- a/packages/core-components/src/components/WarningPanel/WarningPanel.tsx +++ b/packages/core-components/src/components/WarningPanel/WarningPanel.tsx @@ -77,6 +77,7 @@ type Props = { title?: string; severity?: 'warning' | 'error' | 'info'; message?: React.ReactNode; + defaultExpanded?: boolean; children?: React.ReactNode; }; @@ -92,18 +93,21 @@ const capitalize = (s: string) => { * @param {string} [title] A title for the warning. If not supplied, "Warning" will be used. * @param {Object} [message] Optional more detailed user-friendly message elaborating on the cause of the error. * @param {Object} [children] Objects to provide context, such as a stack trace or detailed error reporting. - * Will be available inside an unfolded accordion. + * Will be available inside an unfolded accordion. */ export const WarningPanel = (props: Props) => { const classes = useStyles(props); - const { severity, title, message, children } = props; + const { severity, title, message, children, defaultExpanded } = props; // If no severity or title provided, the heading will read simply "Warning" const subTitle = (severity ? capitalize(severity) : 'Warning') + (title ? `: ${title}` : ''); return ( - + } className={classes.summary} diff --git a/packages/core-components/src/components/index.ts b/packages/core-components/src/components/index.ts index c8797f576b..28b6f6637a 100644 --- a/packages/core-components/src/components/index.ts +++ b/packages/core-components/src/components/index.ts @@ -22,6 +22,7 @@ export * from './CopyTextButton'; export * from './DependencyGraph'; export * from './DismissableBanner'; export * from './EmptyState'; +export * from './ErrorPanel'; export * from './ResponseErrorPanel'; export * from './FeatureDiscovery'; export * from './HeaderIconLinkRow'; diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index 686d096b7f..a6f196847a 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -70,6 +70,7 @@ export type AppComponents = { BootErrorPage: ComponentType; Progress: ComponentType<{}>; Router: ComponentType<{}>; + ErrorBoundaryFallback: ComponentType; SignInPage?: ComponentType; }; @@ -238,6 +239,13 @@ export type ErrorApi = { // @public (undocumented) export const errorApiRef: ApiRef; +// @public (undocumented) +export type ErrorBoundaryFallbackProps = { + plugin?: BackstagePlugin; + error: Error; + resetError: () => void; +}; + // @public export type ErrorContext = { hidden?: boolean; diff --git a/packages/core-plugin-api/src/app/types.ts b/packages/core-plugin-api/src/app/types.ts index 66796fd5a4..26e264983c 100644 --- a/packages/core-plugin-api/src/app/types.ts +++ b/packages/core-plugin-api/src/app/types.ts @@ -50,11 +50,18 @@ export type SignInPageProps = { onResult(result: SignInResult): void; }; +export type ErrorBoundaryFallbackProps = { + plugin?: BackstagePlugin; + error: Error; + resetError: () => void; +}; + export type AppComponents = { NotFoundErrorPage: ComponentType<{}>; BootErrorPage: ComponentType; Progress: ComponentType<{}>; Router: ComponentType<{}>; + ErrorBoundaryFallback: ComponentType; /** * An optional sign-in page that will be rendered instead of the AppRouter at startup. diff --git a/packages/core-plugin-api/src/extensions/PluginErrorBoundary.tsx b/packages/core-plugin-api/src/extensions/PluginErrorBoundary.tsx new file mode 100644 index 0000000000..e33d9dcb43 --- /dev/null +++ b/packages/core-plugin-api/src/extensions/PluginErrorBoundary.tsx @@ -0,0 +1,56 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed 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 React from 'react'; +import { AppContext } from '../app/types'; +import { BackstagePlugin } from '../plugin'; + +type Props = { + app: AppContext; + plugin: BackstagePlugin; +}; + +type State = { error: Error | undefined }; + +export class PluginErrorBoundary extends React.Component { + static getDerivedStateFromError(error: Error) { + return { error }; + } + + state: State = { error: undefined }; + + handleErrorReset = () => { + this.setState({ error: undefined }); + }; + + render() { + const { error } = this.state; + const { app, plugin } = this.props; + const { ErrorBoundaryFallback } = app.getComponents(); + + if (error) { + return ( + + ); + } + + return this.props.children; + } +} diff --git a/packages/core-plugin-api/src/extensions/extensions.test.tsx b/packages/core-plugin-api/src/extensions/extensions.test.tsx index 29592574ec..798d3c8fc2 100644 --- a/packages/core-plugin-api/src/extensions/extensions.test.tsx +++ b/packages/core-plugin-api/src/extensions/extensions.test.tsx @@ -14,7 +14,10 @@ * limitations under the License. */ +import { withLogCollector } from '@backstage/test-utils-core'; +import { render, screen } from '@testing-library/react'; import React from 'react'; +import { useApp, ErrorBoundaryFallbackProps } from '../app'; import { createPlugin } from '../plugin'; import { createRouteRef } from '../routing'; import { getComponentData } from './componentData'; @@ -24,6 +27,10 @@ import { createRoutableExtension, } from './extensions'; +jest.mock('../app'); + +const mocked = (f: Function) => f as jest.Mock; + const plugin = createPlugin({ id: 'my-plugin', }); @@ -73,4 +80,31 @@ describe('extensions', () => { expect(getComponentData(element2, 'core.plugin')).toBe(plugin); expect(getComponentData(element2, 'core.mountPoint')).toBe(routeRef); }); + + it('should wrap extended component with error boundary', async () => { + const BrokenComponent = plugin.provide( + createComponentExtension({ + component: { + sync: () => { + throw new Error('Test error'); + }, + }, + }), + ); + + mocked(useApp).mockReturnValue({ + getComponents: () => ({ + Progress: () => null, + ErrorBoundaryFallback: (props: ErrorBoundaryFallbackProps) => ( + <>Error in {props.plugin?.getId()} + ), + }), + }); + + const { error: errors } = await withLogCollector(['error'], async () => { + render(); + }); + screen.getByText('Error in my-plugin'); + expect(errors[0]).toMatch('Test error'); + }); }); diff --git a/packages/core-plugin-api/src/extensions/extensions.tsx b/packages/core-plugin-api/src/extensions/extensions.tsx index d6597ba8a9..349f44726f 100644 --- a/packages/core-plugin-api/src/extensions/extensions.tsx +++ b/packages/core-plugin-api/src/extensions/extensions.tsx @@ -19,6 +19,7 @@ import { useApp } from '../app'; import { RouteRef, useRouteRef } from '../routing'; import { attachComponentData } from './componentData'; import { Extension, BackstagePlugin } from '../plugin/types'; +import { PluginErrorBoundary } from './PluginErrorBoundary'; type ComponentLoader = | { @@ -123,11 +124,18 @@ export function createReactExtension< return { expose(plugin: BackstagePlugin) { - const Result: any = (props: any) => ( - - - - ); + const Result: any = (props: any) => { + const app = useApp(); + const { Progress } = app.getComponents(); + + return ( + }> + + + + + ); + }; attachComponentData(Result, 'core.plugin', plugin); for (const [key, value] of Object.entries(data)) {