From 873d5cfc027c2fd5575d957b90a7d2880604d9dd Mon Sep 17 00:00:00 2001 From: Maxim Stykow Date: Fri, 23 Feb 2024 17:04:50 +0100 Subject: [PATCH] fix: introduce proper error boundary - add methods to quit or relaunch the app - create error boundary that displays error and offers options to relaunch or quit closes #2564 Signed-off-by: Maxim Stykow --- package.json | 1 + renovate.json5 | 1 + src/ElectronBackend/main/main.ts | 6 + src/ElectronBackend/preload.ts | 2 + src/Frontend/Components/App/App.tsx | 23 ++-- .../ErrorBoundary/ErrorBoundary.tsx | 110 ------------------ .../__tests__/ErrorBoundary.test.tsx | 62 ---------- .../ErrorFallback/ErrorFallback.style.ts | 22 ++++ .../ErrorFallback/ErrorFallback.tsx | 52 +++++++++ src/shared/ipc-channels.ts | 2 + src/shared/shared-types.ts | 2 + src/shared/text.ts | 5 + src/testing/setup-tests.ts | 2 + yarn.lock | 12 ++ 14 files changed, 119 insertions(+), 183 deletions(-) delete mode 100644 src/Frontend/Components/ErrorBoundary/ErrorBoundary.tsx delete mode 100644 src/Frontend/Components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx create mode 100644 src/Frontend/Components/ErrorFallback/ErrorFallback.style.ts create mode 100644 src/Frontend/Components/ErrorFallback/ErrorFallback.tsx diff --git a/package.json b/package.json index a5ecc3b55..952d57511 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "re-resizable": "^6.9.11", "react": "^18.2.0", "react-dom": "^18.2.0", + "react-error-boundary": "^4.0.12", "react-hot-toast": "^2.4.1", "react-hotkeys-hook": "^4.5.0", "react-redux": "^9.1.0", diff --git a/renovate.json5 b/renovate.json5 index f589e9557..042c64464 100644 --- a/renovate.json5 +++ b/renovate.json5 @@ -48,6 +48,7 @@ 'prettier', 'prop-types', 'proxy-memoize', + 'react-error-boundary', 'react-hot-toast', 'react-hotkeys-hook', 'react-virtuoso', diff --git a/src/ElectronBackend/main/main.ts b/src/ElectronBackend/main/main.ts index 8e61217b8..f45a17fff 100644 --- a/src/ElectronBackend/main/main.ts +++ b/src/ElectronBackend/main/main.ts @@ -58,6 +58,12 @@ export async function main(): Promise { }, ); + ipcMain.handle(IpcChannel.Quit, () => { + mainWindow.close(); + }); + ipcMain.handle(IpcChannel.Relaunch, () => { + mainWindow.reload(); + }); ipcMain.handle( IpcChannel.ConvertInputFile, getConvertInputFileToDotOpossumAndOpenListener(mainWindow), diff --git a/src/ElectronBackend/preload.ts b/src/ElectronBackend/preload.ts index fbf0391f1..ea8dc6120 100644 --- a/src/ElectronBackend/preload.ts +++ b/src/ElectronBackend/preload.ts @@ -9,6 +9,8 @@ import { IpcChannel } from '../shared/ipc-channels'; import { ElectronAPI } from '../shared/shared-types'; const electronAPI: ElectronAPI = { + quit: () => ipcRenderer.invoke(IpcChannel.Quit), + relaunch: () => ipcRenderer.invoke(IpcChannel.Relaunch), openLink: (link) => ipcRenderer.invoke(IpcChannel.OpenLink, { link }), openFile: () => ipcRenderer.invoke(IpcChannel.OpenFile), deleteFile: () => ipcRenderer.invoke(IpcChannel.DeleteFile), diff --git a/src/Frontend/Components/App/App.tsx b/src/Frontend/Components/App/App.tsx index c04d03eb5..9293bf2c6 100644 --- a/src/Frontend/Components/App/App.tsx +++ b/src/Frontend/Components/App/App.tsx @@ -4,6 +4,7 @@ // SPDX-License-Identifier: Apache-2.0 import '@fontsource-variable/karla'; import { StyledEngineProvider, ThemeProvider } from '@mui/material/styles'; +import { ErrorBoundary } from 'react-error-boundary'; import { View } from '../../enums/enums'; import { useAppSelector } from '../../state/hooks'; @@ -12,7 +13,7 @@ import { getSelectedView } from '../../state/selectors/view-selector'; import { usePanelSizes } from '../../state/variables/use-panel-sizes'; import { useSignalsWorker } from '../../web-workers/use-signals-worker'; import { AuditView } from '../AuditView/AuditView'; -import { ErrorBoundary } from '../ErrorBoundary/ErrorBoundary'; +import { ErrorFallback } from '../ErrorFallback/ErrorFallback'; import { GlobalPopup } from '../GlobalPopup/GlobalPopup'; import { ProcessPopup } from '../ProcessPopup/ProcessPopup'; import { ReportView } from '../ReportView/ReportView'; @@ -32,18 +33,18 @@ export function App() { usePanelSizes(); // pre-hydrate size of panels return ( - - - - - - + + + + + + {renderView()} - - - - + + + + ); function renderView() { diff --git a/src/Frontend/Components/ErrorBoundary/ErrorBoundary.tsx b/src/Frontend/Components/ErrorBoundary/ErrorBoundary.tsx deleted file mode 100644 index d9903e20c..000000000 --- a/src/Frontend/Components/ErrorBoundary/ErrorBoundary.tsx +++ /dev/null @@ -1,110 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// SPDX-FileCopyrightText: Nico Carl -// -// SPDX-License-Identifier: Apache-2.0 -import MuiBox from '@mui/material/Box'; -import { Action } from '@reduxjs/toolkit'; -import { noop } from 'lodash'; -import { Component, Dispatch, ErrorInfo, ReactNode } from 'react'; -import { connect } from 'react-redux'; - -import { AllowedFrontendChannels } from '../../../shared/ipc-channels'; -import { SendErrorInformationArgs } from '../../../shared/shared-types'; -import { OpossumColors } from '../../shared-styles'; -import { resetResourceState } from '../../state/actions/resource-actions/all-views-simple-actions'; -import { resetViewState } from '../../state/actions/view-actions/view-actions'; - -const classes = { - root: { - background: OpossumColors.lightBlue, - width: '100vw', - height: '100vh', - }, -}; - -// catches errors that are not thrown during render -// it's known to fire twice in dev mode: https://github.com/facebook/react/issues/19613 -window.addEventListener('error', (event): void => { - if (event.error) { - sendErrorInfo(event.error, { - componentStack: event.error.stack, - }); - } -}); - -interface ErrorBoundaryState { - hasError: boolean; -} - -interface DispatchProps { - resetState(): void; -} - -interface ErrorBoundaryProps extends DispatchProps { - children: ReactNode; -} - -function sendErrorInfo(error: Error, errorInfo: ErrorInfo): void { - const sendErrorInformationArgs: SendErrorInformationArgs = { - error, - errorInfo, - }; - window.electronAPI.sendErrorInformation(sendErrorInformationArgs); -} - -class ProtoErrorBoundary extends Component< - ErrorBoundaryProps, - ErrorBoundaryState -> { - private removeListener: () => void = noop; - - constructor(props: ErrorBoundaryProps) { - super(props); - this.state = { hasError: false }; - } - - override componentDidMount(): void { - this.removeListener = window.electronAPI.on( - AllowedFrontendChannels.RestoreFrontend, - () => { - this.props.resetState(); - this.setState({ hasError: false }); - }, - ); - } - - override componentWillUnmount(): void { - this.removeListener(); - } - - static getDerivedStateFromError(): ErrorBoundaryState { - return { hasError: true }; - } - - override componentDidCatch(error: Error, errorInfo: ErrorInfo): void { - sendErrorInfo(error, errorInfo); - } - - override render(): ReactNode { - if (this.state.hasError) { - return ; - } - - return this.props.children; - } -} - -function mapDispatchToProps(dispatch: Dispatch): DispatchProps { - return { - resetState: (): void => { - dispatch(resetResourceState()); - dispatch(resetViewState()); - }, - }; -} - -export const ErrorBoundary = connect( - null, - mapDispatchToProps, -)(ProtoErrorBoundary); diff --git a/src/Frontend/Components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx b/src/Frontend/Components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx deleted file mode 100644 index 2eb85ea24..000000000 --- a/src/Frontend/Components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx +++ /dev/null @@ -1,62 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// SPDX-FileCopyrightText: Nico Carl -// -// SPDX-License-Identifier: Apache-2.0 -import { screen } from '@testing-library/react'; - -import { AllowedFrontendChannels } from '../../../../shared/ipc-channels'; -import { initialResourceState } from '../../../state/reducers/resource-reducer'; -import { initialViewState } from '../../../state/reducers/view-reducer'; -import { renderComponent } from '../../../test-helpers/render'; -import { ErrorBoundary } from '../ErrorBoundary'; - -describe('ErrorBoundary', () => { - function TestComponent(props: { throws: boolean }): JSX.Element { - if (props.throws) { - throw new Error(); - } - - return
{'Test'}
; - } - - it('renders its children', () => { - renderComponent( - - - , - ); - - expect(window.electronAPI.openFile).toHaveBeenCalledTimes(0); - expect(window.electronAPI.on).toHaveBeenCalledTimes(1); - - screen.getByText('Test'); - }); - - it('renders fallback and restores state', () => { - // we expect warnings that we do not want to see - jest.spyOn(console, 'error').mockImplementation(); - - const { store } = renderComponent( - - - , - ); - - const expectedNumberOfCalls = 3; - expect(window.electronAPI.sendErrorInformation).toHaveBeenCalledTimes( - expectedNumberOfCalls, - ); - - expect(window.electronAPI.on).toHaveBeenCalledTimes(1); - expect(window.electronAPI.on).toHaveBeenCalledWith( - AllowedFrontendChannels.RestoreFrontend, - expect.anything(), - ); - - expect(store.getState().resourceState).toMatchObject(initialResourceState); - expect(store.getState().viewState).toMatchObject(initialViewState); - - expect(screen.queryByText('Test')).not.toBeInTheDocument(); - }); -}); diff --git a/src/Frontend/Components/ErrorFallback/ErrorFallback.style.ts b/src/Frontend/Components/ErrorFallback/ErrorFallback.style.ts new file mode 100644 index 000000000..135ff8c91 --- /dev/null +++ b/src/Frontend/Components/ErrorFallback/ErrorFallback.style.ts @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { styled } from '@mui/material'; + +export const Container = styled('div')({ + display: 'flex', + flexDirection: 'row', + height: '100%', + width: '100%', + alignItems: 'center', + justifyContent: 'center', +}); + +export const TextContainer = styled('div')({ + display: 'flex', + flexDirection: 'column', + gap: '20px', + width: 'fit-content', + maxWidth: '600px', +}); diff --git a/src/Frontend/Components/ErrorFallback/ErrorFallback.tsx b/src/Frontend/Components/ErrorFallback/ErrorFallback.tsx new file mode 100644 index 000000000..95a3c42a1 --- /dev/null +++ b/src/Frontend/Components/ErrorFallback/ErrorFallback.tsx @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import MuiButton from '@mui/material/Button'; +import MuiButtonGroup from '@mui/material/ButtonGroup'; +import MuiTypography from '@mui/material/Typography'; +import { FallbackProps } from 'react-error-boundary'; + +import { text } from '../../../shared/text'; +import { OpossumColors } from '../../shared-styles'; +import { Container, TextContainer } from './ErrorFallback.style'; + +export const ErrorFallback: React.FC = ({ + error, + resetErrorBoundary, +}) => { + return ( + + + + {text.errorBoundary.unexpectedError} + + + {error.message} + + + { + resetErrorBoundary(); + window.electronAPI.relaunch(); + }} + > + {text.errorBoundary.relaunch} + + { + window.electronAPI.quit(); + }} + > + {text.errorBoundary.quit} + + + + + ); +}; diff --git a/src/shared/ipc-channels.ts b/src/shared/ipc-channels.ts index 7bfd1cc1f..51f8ed075 100644 --- a/src/shared/ipc-channels.ts +++ b/src/shared/ipc-channels.ts @@ -17,6 +17,8 @@ export enum IpcChannel { OpenDotOpossumFile = 'open-dot-opossum-file', GetUserSettings = 'get-user-settings', SetUserSettings = 'set-user-settings', + Quit = 'quit', + Relaunch = 'relaunch', } export enum AllowedFrontendChannels { diff --git a/src/shared/shared-types.ts b/src/shared/shared-types.ts index f513420cd..71562a414 100644 --- a/src/shared/shared-types.ts +++ b/src/shared/shared-types.ts @@ -235,6 +235,8 @@ export interface FileSupportPopupArgs { export type Listener = (event: IpcRendererEvent, ...args: Array) => void; export interface ElectronAPI { + quit: () => void; + relaunch: () => void; openLink: (link: string) => Promise; openFile: () => Promise; deleteFile: () => Promise; diff --git a/src/shared/text.ts b/src/shared/text.ts index 24a7eb3dd..cabe64877 100644 --- a/src/shared/text.ts +++ b/src/shared/text.ts @@ -229,4 +229,9 @@ export const text = { applyChanges: 'Apply changes', revertAll: 'Revert all', }, + errorBoundary: { + unexpectedError: "We're sorry, an unexpected error occurred!", + relaunch: 'Relaunch App', + quit: 'Quit App', + }, } as const; diff --git a/src/testing/setup-tests.ts b/src/testing/setup-tests.ts index 4dba1862c..9d47c4891 100644 --- a/src/testing/setup-tests.ts +++ b/src/testing/setup-tests.ts @@ -32,6 +32,8 @@ class ResizeObserver { } global.window.electronAPI = { + quit: jest.fn(), + relaunch: jest.fn(), openLink: jest.fn().mockReturnValue(Promise.resolve()), openFile: jest.fn(), deleteFile: jest.fn(), diff --git a/yarn.lock b/yarn.lock index 160273778..6fea47666 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9910,6 +9910,7 @@ __metadata: re-resizable: "npm:^6.9.11" react: "npm:^18.2.0" react-dom: "npm:^18.2.0" + react-error-boundary: "npm:^4.0.12" react-hot-toast: "npm:^2.4.1" react-hotkeys-hook: "npm:^4.5.0" react-redux: "npm:^9.1.0" @@ -10456,6 +10457,17 @@ __metadata: languageName: node linkType: hard +"react-error-boundary@npm:^4.0.12": + version: 4.0.12 + resolution: "react-error-boundary@npm:4.0.12" + dependencies: + "@babel/runtime": "npm:^7.12.5" + peerDependencies: + react: ">=16.13.1" + checksum: 10/ba59f885eae3c3786548086c6c2088a9f511080c4052e778017959e9e0b6461892efdcf58fcfc2b3a6bc3e79e17cf842fc8ccdc6d82698c51c2ccab12c8c0b85 + languageName: node + linkType: hard + "react-hot-toast@npm:^2.4.1": version: 2.4.1 resolution: "react-hot-toast@npm:2.4.1"