diff --git a/.changeset/few-cats-type.md b/.changeset/few-cats-type.md new file mode 100644 index 0000000000..7d4c0f69cb --- /dev/null +++ b/.changeset/few-cats-type.md @@ -0,0 +1,33 @@ +--- +'@backstage/core': minor +'@backstage/core-api': patch +--- + +This change adds error boundary on extensions. + +This means 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 mount the children of the error boundary again. +- `plugin`. A BackstagePlugin that can be used to lookup 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/app/src/App.tsx b/packages/app/src/App.tsx index 5874465ef6..927e9b4327 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -20,6 +20,7 @@ import { FlatRoutes, OAuthRequestDialog, SignInPage, + ErrorBoundaryFallback, } from '@backstage/core'; import { apiDocsPlugin, ApiExplorerPage } from '@backstage/plugin-api-docs'; import { @@ -66,6 +67,9 @@ const app = createApp({ alert: AlarmIcon, }, components: { + ErrorBoundaryFallback: props => { + return ; + }, SignInPage: props => { return ( { BootErrorPage: () => null, Progress: () => null, Router: BrowserRouter, + ErrorBoundaryFallback: () => null, }; it('runs happy paths', async () => { diff --git a/packages/core-api/src/app/types.ts b/packages/core-api/src/app/types.ts index df0f65f33f..40e9913201 100644 --- a/packages/core-api/src/app/types.ts +++ b/packages/core-api/src/app/types.ts @@ -14,13 +14,14 @@ * limitations under the License. */ +import { AppConfig } from '@backstage/config'; import { ComponentType } from 'react'; +import { AppTheme, ProfileInfo } from '../apis/definitions'; +import { AnyApiFactory } from '../apis/system'; +import type { ErrorBoundaryFallbackProps } from '../extensions/PluginErrorBoundary'; import { IconComponent, IconComponentMap, IconKey } from '../icons/types'; import { AnyExternalRoutes, BackstagePlugin } from '../plugin/types'; import { ExternalRouteRef, RouteRef, SubRouteRef } from '../routing/types'; -import { AnyApiFactory } from '../apis/system'; -import { AppTheme, ProfileInfo } from '../apis/definitions'; -import { AppConfig } from '@backstage/config'; export type BootErrorPageProps = { step: 'load-config' | 'load-chunk'; @@ -58,6 +59,11 @@ export type AppComponents = { BootErrorPage: ComponentType; Progress: ComponentType<{}>; Router: ComponentType<{}>; + ErrorBoundaryFallback: ComponentType< + ErrorBoundaryFallbackProps & { + plugin: BackstagePlugin; + } + >; /** * An optional sign-in page that will be rendered instead of the AppRouter at startup. diff --git a/packages/core-api/src/extensions/PluginErrorBoundary.tsx b/packages/core-api/src/extensions/PluginErrorBoundary.tsx new file mode 100644 index 0000000000..4cf772a638 --- /dev/null +++ b/packages/core-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'; + +export type ErrorBoundaryFallbackProps = { + error: Error; + resetError: () => void; +}; + +type FallbackRender = React.FunctionComponent; + +type Props = { + fallbackRender: FallbackRender; +}; + +type State = { error?: Error }; + +export class PluginErrorBoundary extends React.Component { + static getDerivedStateFromError(error: Error) { + return { error }; + } + + state: State = { error: undefined }; + + resetError = () => { + this.setState({ error: undefined }); + }; + + render() { + const { error } = this.state; + const { fallbackRender } = this.props; + + if (error) { + return fallbackRender({ + error, + resetError: this.resetError, + }); + } + + return this.props.children; + } +} diff --git a/packages/core-api/src/extensions/extensions.test.tsx b/packages/core-api/src/extensions/extensions.test.tsx index 26755b3bcf..db67ed18c9 100644 --- a/packages/core-api/src/extensions/extensions.test.tsx +++ b/packages/core-api/src/extensions/extensions.test.tsx @@ -14,7 +14,13 @@ * limitations under the License. */ +import { errorApiRef } from '../apis/definitions/ErrorApi'; +import { ApiRegistry } from '../apis/system/ApiRegistry'; +import { ApiProvider } from '../apis/system/ApiProvider'; +import { withLogCollector } from '@backstage/test-utils-core'; +import { render, screen } from '@testing-library/react'; import React from 'react'; +import { AppContextProvider } from '../app/AppContext'; import { createPlugin } from '../plugin'; import { createRouteRef } from '../routing'; import { getComponentData } from './componentData'; @@ -23,6 +29,9 @@ import { createReactExtension, createRoutableExtension, } from './extensions'; +import { AppComponents } from '../app/types'; +import { AppContext } from '../app'; +import { MockErrorApi } from '@backstage/test-utils'; const plugin = createPlugin({ id: 'my-plugin', @@ -73,4 +82,44 @@ 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 extension = createComponentExtension({ + component: { + sync: () => { + throw new Error('Test error'); + }, + }, + }); + + const BrokenComponent = plugin.provide(extension); + + const errorApi = new MockErrorApi(); + + const apis = ApiRegistry.from([[errorApiRef, errorApi]]); + + const MockFallback: AppComponents['ErrorBoundaryFallback'] = props => ( + <>Error in {props.plugin.getId()} + ); + + const { error: errors } = await withLogCollector(['error'], async () => { + render( + + ({ + ErrorBoundaryFallback: MockFallback, + }), + } as unknown) as AppContext + } + > + + + , + ); + }); + screen.getByText('Error in my-plugin'); + expect(errors[0]).toMatch('Test error'); + }); }); diff --git a/packages/core-api/src/extensions/extensions.tsx b/packages/core-api/src/extensions/extensions.tsx index 9f6412f960..686c467caf 100644 --- a/packages/core-api/src/extensions/extensions.tsx +++ b/packages/core-api/src/extensions/extensions.tsx @@ -19,6 +19,7 @@ import { useApp } from '../app'; import { BackstagePlugin, Extension } from '../plugin/types'; import { RouteRef, useRouteRef } from '../routing'; import { attachComponentData } from './componentData'; +import { PluginErrorBoundary } from './PluginErrorBoundary'; type ComponentLoader = | { @@ -123,11 +124,28 @@ export function createReactExtension< return { expose(plugin: BackstagePlugin) { - const Result: any = (props: any) => ( - - - - ); + const Result: any = (props: any) => { + const app = useApp(); + const { ErrorBoundaryFallback } = app.getComponents(); + + return ( + + { + return ( + + ); + }} + > + + + + ); + }; attachComponentData(Result, 'core.plugin', plugin); for (const [key, value] of Object.entries(data)) { diff --git a/packages/core-api/src/routing/hooks.test.tsx b/packages/core-api/src/routing/hooks.test.tsx index 8a9b70ed7d..a8fdfda5d0 100644 --- a/packages/core-api/src/routing/hooks.test.tsx +++ b/packages/core-api/src/routing/hooks.test.tsx @@ -14,6 +14,7 @@ * limitations under the License. */ +import { AppContext } from '../app'; import { render } from '@testing-library/react'; import { renderHook } from '@testing-library/react-hooks'; import React, { @@ -23,6 +24,7 @@ import React, { useContext, } from 'react'; import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { AppContextProvider } from '../app/AppContext'; import { createRoutableExtension } from '../extensions'; import { childDiscoverer, @@ -154,23 +156,27 @@ function withRoutingProvider( ); } +const appContext = ({ getComponents: () => ({}) } as unknown) as AppContext; + describe('discovery', () => { it('should handle simple routeRef path creation for routeRefs used in other parts of the app', async () => { const root = ( - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + ); const rendered = render( @@ -205,23 +211,25 @@ describe('discovery', () => { it('should handle routeRefs with parameters', async () => { const root = ( - - - - - - - - + + + + + + + + + + ); const rendered = render(withRoutingProvider(root)); @@ -236,8 +244,37 @@ describe('discovery', () => { it('should handle relative routing within parameterized routePaths', async () => { const root = ( - - + + + + + + + + + + + + + + + ); + + const rendered = render(withRoutingProvider(root)); + + await expect( + rendered.findByText('Path at inside: /foo/blob/baz'), + ).resolves.toBeInTheDocument(); + }); + + it('should throw errors for routing to other routeRefs with unsupported parameters', () => { + const root = ( + + @@ -250,33 +287,8 @@ describe('discovery', () => { routeRef={ref3} params={{ id: 'blob' }} /> - - - ); - - const rendered = render(withRoutingProvider(root)); - - await expect( - rendered.findByText('Path at inside: /foo/blob/baz'), - ).resolves.toBeInTheDocument(); - }); - - it('should throw errors for routing to other routeRefs with unsupported parameters', () => { - const root = ( - - - - - - - - - - + + ); const rendered = render(withRoutingProvider(root)); @@ -295,13 +307,15 @@ describe('discovery', () => { it('should handle relative routing of parameterized routePaths with duplicate param names', () => { const root = ( - - - - - - - + + + + + + + + + ); const { routePaths, routeParents } = traverseElementTree({ diff --git a/packages/core/src/api-wrappers/createApp.tsx b/packages/core/src/api-wrappers/createApp.tsx index e79cdf509f..9b8524c89c 100644 --- a/packages/core/src/api-wrappers/createApp.tsx +++ b/packages/core/src/api-wrappers/createApp.tsx @@ -30,6 +30,7 @@ import LightIcon from '@material-ui/icons/WbSunny'; import DarkIcon from '@material-ui/icons/Brightness2'; import { ErrorPage } from '../layout/ErrorPage'; import { Progress } from '../components/Progress'; +import { ErrorBoundaryFallback } from '../components/ErrorBoundaryFallback'; import { defaultApis } from './defaultApis'; import { lightTheme, darkTheme } from '@backstage/theme'; import { AppConfig, JsonObject } from '@backstage/config'; @@ -130,6 +131,7 @@ export function createApp(options?: AppOptions) { BootErrorPage: DefaultBootErrorPage, Progress: Progress, Router: BrowserRouter, + ErrorBoundaryFallback: ErrorBoundaryFallback, ...options?.components, }; const themes = options?.themes ?? [ diff --git a/packages/core/src/components/ErrorBoundaryFallback/ErrorBoundaryFallback.tsx b/packages/core/src/components/ErrorBoundaryFallback/ErrorBoundaryFallback.tsx new file mode 100644 index 0000000000..f1364061df --- /dev/null +++ b/packages/core/src/components/ErrorBoundaryFallback/ErrorBoundaryFallback.tsx @@ -0,0 +1,41 @@ +/* + * Copyright 2020 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. + */ + +// TODO align this design with the backend errors + +import React from 'react'; +import { AppComponents } from '../..'; +import { ResponseErrorPanel } from '../ResponseErrorPanel'; +import { Button } from '@material-ui/core'; + +export const ErrorBoundaryFallback: AppComponents['ErrorBoundaryFallback'] = ({ + error, + resetError, + plugin, +}) => { + return ( + + Retry + + } + /> + ); +}; diff --git a/packages/core/src/components/ErrorBoundaryFallback/index.ts b/packages/core/src/components/ErrorBoundaryFallback/index.ts new file mode 100644 index 0000000000..ff40974ed7 --- /dev/null +++ b/packages/core/src/components/ErrorBoundaryFallback/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2020 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 { ErrorBoundaryFallback } from './ErrorBoundaryFallback'; diff --git a/packages/core/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx b/packages/core/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx index 52f7ab03f9..42adf1980c 100644 --- a/packages/core/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx +++ b/packages/core/src/components/ResponseErrorPanel/ResponseErrorPanel.tsx @@ -45,6 +45,7 @@ type ResponseErrorListProps = { request?: string; stack?: string; json?: string; + actions?: React.ReactNode; }; const ResponseErrorList = ({ @@ -53,6 +54,7 @@ const ResponseErrorList = ({ message, stack, json, + actions, }: ResponseErrorListProps) => { const classes = useStyles(); @@ -107,12 +109,21 @@ const ResponseErrorList = ({ )} + {actions && ( + <> + + {actions} + + )} ); }; type Props = { error: Error; + defaultExpanded?: boolean; + title?: string; + actions?: React.ReactNode; }; /** @@ -121,13 +132,14 @@ type Props = { * Has special treatment for ResponseError errors, to display rich * server-provided information about what happened. */ -export const ResponseErrorDetails = ({ error }: Props) => { +export const ResponseErrorDetails = ({ error, actions }: Props) => { if (error.name !== 'ResponseError') { return ( ); } @@ -158,10 +170,18 @@ export const ResponseErrorDetails = ({ error }: Props) => { * Has special treatment for ResponseError errors, to display rich * server-provided information about what happened. */ -export const ResponseErrorPanel = ({ error }: Props) => { +export const ResponseErrorPanel = ({ + title, + error, + defaultExpanded, + actions, +}: Props) => { return ( - - + + ); }; diff --git a/packages/core/src/components/WarningPanel/WarningPanel.tsx b/packages/core/src/components/WarningPanel/WarningPanel.tsx index 1faf3bb142..20c71ee106 100644 --- a/packages/core/src/components/WarningPanel/WarningPanel.tsx +++ b/packages/core/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; }; @@ -96,14 +97,14 @@ const capitalize = (s: string) => { */ 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/src/components/index.ts b/packages/core/src/components/index.ts index 14f7f3cac0..c8f9803eae 100644 --- a/packages/core/src/components/index.ts +++ b/packages/core/src/components/index.ts @@ -23,6 +23,7 @@ export * from './CopyTextButton'; export * from './DependencyGraph'; export * from './DismissableBanner'; export * from './EmptyState'; +export * from './ErrorBoundaryFallback'; export * from './ResponseErrorPanel'; export * from './FeatureDiscovery'; export * from './HeaderIconLinkRow'; diff --git a/packages/test-utils/src/testUtils/appWrappers.tsx b/packages/test-utils/src/testUtils/appWrappers.tsx index add7b4479c..0086bf3293 100644 --- a/packages/test-utils/src/testUtils/appWrappers.tsx +++ b/packages/test-utils/src/testUtils/appWrappers.tsx @@ -96,6 +96,7 @@ export function wrapInTestApp( Router: ({ children }) => ( ), + ErrorBoundaryFallback: () => null, }, icons: defaultSystemIcons, plugins: [],