From dcb5403042f01cb534565f3dd79585a829314dcb Mon Sep 17 00:00:00 2001 From: Tejas Kumar Date: Thu, 20 May 2021 17:13:17 +0200 Subject: [PATCH 1/5] Make error boundary more helpful Signed-off-by: Tejas Kumar --- .../layout/ErrorBoundary/ErrorBoundary.tsx | 53 ++++++++++++++++--- .../src/layout/InfoCard/InfoCard.tsx | 5 +- .../src/layout/TabbedCard/TabbedCard.tsx | 5 +- packages/core/src/layout/constants.ts | 19 +++++++ 4 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 packages/core/src/layout/constants.ts diff --git a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx index 4134808311..189803de59 100644 --- a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx +++ b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx @@ -14,10 +14,12 @@ * limitations under the License. */ -import React, { ComponentClass, Component, ErrorInfo } from 'react'; +import { Button, CardContent, CardHeader } from '@material-ui/core'; +import React, { ComponentClass, Component, ErrorInfo, useState } from 'react'; +import { slackChannel as defaultSlackChannel } from '../constants'; type Props = { - slackChannel?: string; + slackChannel?: typeof defaultSlackChannel; onError?: (error: Error, errorInfo: string) => null; }; @@ -28,15 +30,54 @@ type State = { type EProps = { error?: Error; - slackChannel?: string; + slackChannel?: typeof defaultSlackChannel; children?: React.ReactNode; }; -const Error = ({ slackChannel }: EProps) => { +const Error = ({ slackChannel, error }: EProps) => { + const [isShowingTrace, setIsShowingTrace] = useState(false); return (
- Something went wrong here.{' '} - {slackChannel && <>Please contact {slackChannel} for help.} + + +

+ Error: {error?.message ?? 'No Further Info'} +

+
+ +
+ {isShowingTrace && ( + 
+            
+              {error?.stack?.split('\n').map(line => (
+                <>
+                  {line}
+                  

+                
+              )) ?? ''}
+            
+
+ )} + {slackChannel && ( +

+ Please contact{' '} + + {slackChannel.name} + {' '} + for help. +

+ )} +
); }; diff --git a/packages/core-components/src/layout/InfoCard/InfoCard.tsx b/packages/core-components/src/layout/InfoCard/InfoCard.tsx index 9a64bfa6f3..5240c77486 100644 --- a/packages/core-components/src/layout/InfoCard/InfoCard.tsx +++ b/packages/core-components/src/layout/InfoCard/InfoCard.tsx @@ -28,6 +28,7 @@ import { import classNames from 'classnames'; import { ErrorBoundary } from '../ErrorBoundary'; import { BottomLink, BottomLinkProps } from '../BottomLink'; +import { slackChannel as defaultSlackChannel } from '../constants'; const useStyles = makeStyles(theme => ({ noPadding: { @@ -112,7 +113,7 @@ type Props = { subheader?: ReactNode; divider?: boolean; deepLink?: BottomLinkProps; - slackChannel?: string; + slackChannel?: typeof defaultSlackChannel; variant?: InfoCardVariants; style?: object; cardStyle?: object; @@ -133,7 +134,7 @@ export const InfoCard = ({ subheader, divider = true, deepLink, - slackChannel = '#backstage', + slackChannel = defaultSlackChannel, variant, children, headerStyle, diff --git a/packages/core-components/src/layout/TabbedCard/TabbedCard.tsx b/packages/core-components/src/layout/TabbedCard/TabbedCard.tsx index 6204182cb1..be906100d5 100644 --- a/packages/core-components/src/layout/TabbedCard/TabbedCard.tsx +++ b/packages/core-components/src/layout/TabbedCard/TabbedCard.tsx @@ -33,6 +33,7 @@ import { } from '@material-ui/core'; import { BottomLink, BottomLinkProps } from '../BottomLink'; import { ErrorBoundary } from '../ErrorBoundary'; +import { slackChannel as defaultSlackChannel } from '../constants'; const useTabsStyles = makeStyles(theme => ({ root: { @@ -52,7 +53,7 @@ const BoldHeader = withStyles(theme => ({ }))(CardHeader); type Props = { - slackChannel?: string; + slackChannel?: typeof defaultSlackChannel; children?: ReactElement[]; onChange?: (event: React.ChangeEvent<{}>, value: number | string) => void; title?: string; @@ -61,7 +62,7 @@ type Props = { }; const TabbedCard = ({ - slackChannel = '#backstage', + slackChannel = defaultSlackChannel, children, title, deepLink, diff --git a/packages/core/src/layout/constants.ts b/packages/core/src/layout/constants.ts new file mode 100644 index 0000000000..a7005fc5ce --- /dev/null +++ b/packages/core/src/layout/constants.ts @@ -0,0 +1,19 @@ +/* + * 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 const slackChannel = { + name: '#backstage', + href: 'https://spotify.slack.com/archives/C54P1J36Z', +}; From 3db266fe43db8c3dce1338f1b070efb5f2af9581 Mon Sep 17 00:00:00 2001 From: Tejas Kumar Date: Thu, 20 May 2021 17:18:52 +0200 Subject: [PATCH 2/5] Add changeset Signed-off-by: Tejas Kumar --- .changeset/rude-pumpkins-hang.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rude-pumpkins-hang.md diff --git a/.changeset/rude-pumpkins-hang.md b/.changeset/rude-pumpkins-hang.md new file mode 100644 index 0000000000..8fe5cc2c1a --- /dev/null +++ b/.changeset/rude-pumpkins-hang.md @@ -0,0 +1,5 @@ +--- +'@backstage/core': patch +--- + +Make error boundary more helpful From 04ce54adc4e4a0a5616bc90264b6df59be3ee6a7 Mon Sep 17 00:00:00 2001 From: Tejas Kumar Date: Thu, 20 May 2021 17:35:45 +0200 Subject: [PATCH 3/5] Fix test Signed-off-by: Tejas Kumar --- .../src/layout/ErrorBoundary/ErrorBoundary.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx index 1dbeeca8c6..7a02449250 100644 --- a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx +++ b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx @@ -56,7 +56,7 @@ describe('', () => { ); expect(getByRole('alert')).toBeInTheDocument(); - expect(getByText(/something went wrong here/i)).toBeInTheDocument(); + expect(getByText(/something went wrong/i)).toBeInTheDocument(); }); expect(error).toEqual([ From 7dacc9b863ef7002a21bc794dbe59e50aba407ee Mon Sep 17 00:00:00 2001 From: Tejas Kumar Date: Wed, 16 Jun 2021 10:15:25 +0200 Subject: [PATCH 4/5] Make Slack URL an example and fallback if no href Signed-off-by: Tejas Kumar --- .../src/layout/ErrorBoundary/ErrorBoundary.tsx | 18 +++++++++++------- packages/core/src/layout/constants.ts | 12 +++++++++--- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx index 189803de59..5ecbf7b723 100644 --- a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx +++ b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx @@ -67,13 +67,17 @@ const Error = ({ slackChannel, error }: EProps) => { {slackChannel && (

Please contact{' '} - - {slackChannel.name} - {' '} + {slackChannel.href ? ( + + {slackChannel.name} + + ) : ( + slackChannel.name + )}{' '} for help.

)} diff --git a/packages/core/src/layout/constants.ts b/packages/core/src/layout/constants.ts index a7005fc5ce..0b469d9cd3 100644 --- a/packages/core/src/layout/constants.ts +++ b/packages/core/src/layout/constants.ts @@ -13,7 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export const slackChannel = { - name: '#backstage', - href: 'https://spotify.slack.com/archives/C54P1J36Z', + +type SlackChannel = { + name: string; + href?: string; +}; + +export const slackChannel: SlackChannel = { + name: '#backstage', + href: 'https://slack.com/channels/your-channel', }; From a3f258377c3a5793e01f35d2574e077fdcb95f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 24 Jun 2021 21:32:27 +0200 Subject: [PATCH 5/5] use ErrorPanel and update cards to use errorBoundaryProps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/rude-pumpkins-hang.md | 31 +++++- .../components/WarningPanel/WarningPanel.tsx | 1 + .../ErrorBoundary/ErrorBoundary.test.tsx | 25 +++-- .../layout/ErrorBoundary/ErrorBoundary.tsx | 102 ++++++------------ .../src/layout/ErrorBoundary/index.ts | 1 + .../src/layout/InfoCard/InfoCard.tsx | 19 ++-- .../src/layout/TabbedCard/TabbedCard.tsx | 15 ++- packages/core/src/layout/constants.ts | 25 ----- 8 files changed, 106 insertions(+), 113 deletions(-) delete mode 100644 packages/core/src/layout/constants.ts diff --git a/.changeset/rude-pumpkins-hang.md b/.changeset/rude-pumpkins-hang.md index 8fe5cc2c1a..9a95b0e455 100644 --- a/.changeset/rude-pumpkins-hang.md +++ b/.changeset/rude-pumpkins-hang.md @@ -1,5 +1,32 @@ --- -'@backstage/core': patch +'@backstage/core-components': patch --- -Make error boundary more helpful +Make `ErrorBoundary` display more helpful information about the error that +occurred. + +The `slackChannel` (optional) prop can now be passed as an object on the form +`{ name: string; href?: string; }` in addition to the old string form. If you +are using the error boundary like + +```tsx + + + +``` + +you may like to migrate it to + +```tsx +const support = { + name: '#support', + href: 'https://slack.com/channels/your-channel', +}; + + + + +``` + +Also deprecated the prop `slackChannel` on `TabbedCard` and `InfoCard`, while +adding the prop `errorBoundaryProps` to replace it. diff --git a/packages/core-components/src/components/WarningPanel/WarningPanel.tsx b/packages/core-components/src/components/WarningPanel/WarningPanel.tsx index c90874fba0..37ae5b2c62 100644 --- a/packages/core-components/src/components/WarningPanel/WarningPanel.tsx +++ b/packages/core-components/src/components/WarningPanel/WarningPanel.tsx @@ -107,6 +107,7 @@ export const WarningPanel = (props: Props) => { } diff --git a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx index 7a02449250..f8adb81a2a 100644 --- a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx +++ b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.test.tsx @@ -17,7 +17,13 @@ import React from 'react'; import { ErrorBoundary } from './ErrorBoundary'; -import { renderInTestApp, withLogCollector } from '@backstage/test-utils'; +import { + MockErrorApi, + renderInTestApp, + withLogCollector, +} from '@backstage/test-utils'; +import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; +import { errorApiRef } from '@backstage/core-plugin-api'; type BombProps = { shouldThrow?: boolean; @@ -35,24 +41,29 @@ const Bomb = ({ shouldThrow }: BombProps) => { describe('', () => { it('should render error boundary with and without error', async () => { const { error } = await withLogCollector(['error'], async () => { + const apis = ApiRegistry.with(errorApiRef, new MockErrorApi()); const { rerender, queryByRole, getByRole, getByText, } = await renderInTestApp( - - - , + + + + + , ); expect(queryByRole('alert')).not.toBeInTheDocument(); expect(getByText(/working component/i)).toBeInTheDocument(); rerender( - - - , + + + + + , ); expect(getByRole('alert')).toBeInTheDocument(); diff --git a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx index 5ecbf7b723..b8dfd7ff3a 100644 --- a/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx +++ b/packages/core-components/src/layout/ErrorBoundary/ErrorBoundary.tsx @@ -14,12 +14,17 @@ * limitations under the License. */ -import { Button, CardContent, CardHeader } from '@material-ui/core'; -import React, { ComponentClass, Component, ErrorInfo, useState } from 'react'; -import { slackChannel as defaultSlackChannel } from '../constants'; +import React, { ComponentClass, Component, ErrorInfo } from 'react'; +import { Button } from '../../components/Button'; +import { ErrorPanel } from '../../components/ErrorPanel'; -type Props = { - slackChannel?: typeof defaultSlackChannel; +type SlackChannel = { + name: string; + href?: string; +}; + +export type ErrorBoundaryProps = { + slackChannel?: string | SlackChannel; onError?: (error: Error, errorInfo: string) => null; }; @@ -28,71 +33,30 @@ type State = { errorInfo?: ErrorInfo; }; -type EProps = { - error?: Error; - slackChannel?: typeof defaultSlackChannel; - children?: React.ReactNode; -}; +const SlackLink = (props: { slackChannel?: string | SlackChannel }) => { + const { slackChannel } = props; + + if (!slackChannel) { + return null; + } else if (typeof slackChannel === 'string') { + return <>Please contact {slackChannel} for help.; + } else if (!slackChannel.href) { + return <>Please contact {slackChannel.name} for help.; + } -const Error = ({ slackChannel, error }: EProps) => { - const [isShowingTrace, setIsShowingTrace] = useState(false); return ( -
- - -

- Error: {error?.message ?? 'No Further Info'} -

-
- -
- {isShowingTrace && ( - 
-            
-              {error?.stack?.split('\n').map(line => (
-                <>
-                  {line}
-                  

-                
-              )) ?? ''}
-            
-
- )} - {slackChannel && ( -

- Please contact{' '} - {slackChannel.href ? ( - - {slackChannel.name} - - ) : ( - slackChannel.name - )}{' '} - for help. -

- )} - -
+ ); }; export const ErrorBoundary: ComponentClass< - Props, + ErrorBoundaryProps, State -> = class ErrorBoundary extends Component { - constructor(props: Props) { +> = class ErrorBoundary extends Component { + constructor(props: ErrorBoundaryProps) { super(props); - this.state = { error: undefined, errorInfo: undefined, @@ -106,13 +70,17 @@ export const ErrorBoundary: ComponentClass< } render() { - const { slackChannel } = this.props; - const { error, errorInfo } = this.state; + const { slackChannel, children } = this.props; + const { error } = this.state; - if (!errorInfo) { - return this.props.children; + if (!error) { + return children; } - return ; + return ( + + + + ); } }; diff --git a/packages/core-components/src/layout/ErrorBoundary/index.ts b/packages/core-components/src/layout/ErrorBoundary/index.ts index 47c1b2bace..a0640bd5ef 100644 --- a/packages/core-components/src/layout/ErrorBoundary/index.ts +++ b/packages/core-components/src/layout/ErrorBoundary/index.ts @@ -14,4 +14,5 @@ * limitations under the License. */ +export type { ErrorBoundaryProps } from './ErrorBoundary'; export { ErrorBoundary } from './ErrorBoundary'; diff --git a/packages/core-components/src/layout/InfoCard/InfoCard.tsx b/packages/core-components/src/layout/InfoCard/InfoCard.tsx index 5240c77486..763038388b 100644 --- a/packages/core-components/src/layout/InfoCard/InfoCard.tsx +++ b/packages/core-components/src/layout/InfoCard/InfoCard.tsx @@ -26,9 +26,8 @@ import { makeStyles, } from '@material-ui/core'; import classNames from 'classnames'; -import { ErrorBoundary } from '../ErrorBoundary'; +import { ErrorBoundary, ErrorBoundaryProps } from '../ErrorBoundary'; import { BottomLink, BottomLinkProps } from '../BottomLink'; -import { slackChannel as defaultSlackChannel } from '../constants'; const useStyles = makeStyles(theme => ({ noPadding: { @@ -96,8 +95,8 @@ export type InfoCardVariants = 'flex' | 'fullHeight' | 'gridItem'; * You can custom style an InfoCard with the 'style' (outer container) and 'cardStyle' (inner container) * styles. * - * The InfoCard serves as an error boundary. As a result, if you provide a 'slackChannel' property this - * specifies the channel to display in the error component that is displayed if an error occurs + * The InfoCard serves as an error boundary. As a result, if you provide an 'errorBoundaryProps' property this + * specifies the extra information to display in the error component that is displayed if an error occurs * in any descendent components. * * By default the InfoCard has no custom layout of its children, but is treated as a block element. A @@ -113,7 +112,9 @@ type Props = { subheader?: ReactNode; divider?: boolean; deepLink?: BottomLinkProps; - slackChannel?: typeof defaultSlackChannel; + /** @deprecated Use errorBoundaryProps instead */ + slackChannel?: string; + errorBoundaryProps?: ErrorBoundaryProps; variant?: InfoCardVariants; style?: object; cardStyle?: object; @@ -134,7 +135,8 @@ export const InfoCard = ({ subheader, divider = true, deepLink, - slackChannel = defaultSlackChannel, + slackChannel, + errorBoundaryProps, variant, children, headerStyle, @@ -170,9 +172,12 @@ export const InfoCard = ({ }); } + const errProps: ErrorBoundaryProps = + errorBoundaryProps || (slackChannel ? { slackChannel } : {}); + return ( - + {title && ( ({ root: { @@ -53,7 +52,9 @@ const BoldHeader = withStyles(theme => ({ }))(CardHeader); type Props = { - slackChannel?: typeof defaultSlackChannel; + /** @deprecated Use errorBoundaryProps instead */ + slackChannel?: string; + errorBoundaryProps?: ErrorBoundaryProps; children?: ReactElement[]; onChange?: (event: React.ChangeEvent<{}>, value: number | string) => void; title?: string; @@ -62,7 +63,8 @@ type Props = { }; const TabbedCard = ({ - slackChannel = defaultSlackChannel, + slackChannel, + errorBoundaryProps, children, title, deepLink, @@ -88,9 +90,12 @@ const TabbedCard = ({ }); } + const errProps: ErrorBoundaryProps = + errorBoundaryProps || (slackChannel ? { slackChannel } : {}); + return ( - + {title && }