Skip to content

Commit b654a5f

Browse files
authored
added error boundary for chunk load errors (opendatahub-io#6310)
* added error boundary for chunk load errors * addressed comments * added code coverage
1 parent 564318b commit b654a5f

10 files changed

Lines changed: 400 additions & 53 deletions

frontend/src/bootstrap.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { ThemeProvider } from './app/ThemeContext';
88
import SDKInitialize from './SDKInitialize';
99
import { BrowserStorageContextProvider } from './components/browserStorage/BrowserStorageContext';
1010
import ErrorBoundary from './components/error/ErrorBoundary';
11+
import RouteErrorElement from './components/error/RouteErrorElement';
1112
import { ReduxContext } from './redux/context';
1213

1314
// Creates a data router using createBrowserRouter
@@ -23,6 +24,7 @@ const router = createBrowserRouter([
2324
</BrowserStorageContextProvider>
2425
</SDKInitialize>
2526
),
27+
errorElement: <RouteErrorElement />,
2628
},
2729
]);
2830

frontend/src/components/error/ErrorBoundary.tsx

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import * as React from 'react';
2-
import { Button, Split, SplitItem, Title } from '@patternfly/react-core';
3-
import { TimesIcon } from '@patternfly/react-icons';
42
import ErrorDetails from './ErrorDetails';
3+
import ErrorFallbackLayout from './ErrorFallbackLayout';
54
import UpdateState from './UpdateState';
65

76
type ErrorBoundaryProps = {
@@ -51,42 +50,14 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
5150
}
5251
return (
5352
<div className="pf-v6-u-p-lg" data-testid="error-boundary">
54-
<Split>
55-
<SplitItem isFilled>
56-
<Title headingLevel="h1" className="pf-v6-u-mb-sm">
57-
An error occurred
58-
</Title>
59-
<p className="pf-v6-u-mb-md">
60-
Try{' '}
61-
<Button
62-
data-testid="reload-link"
63-
variant="link"
64-
isInline
65-
onClick={() => window.location.reload()}
66-
>
67-
reloading
68-
</Button>{' '}
69-
the page if there was a recent update.
70-
</p>
71-
</SplitItem>
72-
<SplitItem>
73-
<Button
74-
icon={<TimesIcon />}
75-
data-testid="close-error-button"
76-
variant="plain"
77-
aria-label="Close"
78-
onClick={() => {
79-
this.setState({ hasError: false });
80-
}}
81-
/>
82-
</SplitItem>
83-
</Split>
84-
<ErrorDetails
85-
title={error.name}
86-
errorMessage={error.message}
87-
componentStack={errorInfo.componentStack || ''}
88-
stack={error.stack}
89-
/>
53+
<ErrorFallbackLayout onClose={() => this.setState({ hasError: false })}>
54+
<ErrorDetails
55+
title={error.name}
56+
errorMessage={error.message}
57+
componentStack={errorInfo.componentStack || ''}
58+
stack={error.stack}
59+
/>
60+
</ErrorFallbackLayout>
9061
</div>
9162
);
9263
}

frontend/src/components/error/ErrorDetails.tsx

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
type ErrorDetailsProps = {
1313
title: string;
1414
errorMessage?: string;
15-
componentStack: string;
15+
componentStack?: string;
1616
stack?: string;
1717
};
1818

@@ -34,20 +34,22 @@ const ErrorDetails: React.FC<ErrorDetailsProps> = ({
3434
</DescriptionListGroup>
3535
) : null}
3636

37-
<DescriptionListGroup>
38-
<DescriptionListTerm>Component trace:</DescriptionListTerm>
39-
<DescriptionListDescription>
40-
<ClipboardCopy
41-
isExpanded
42-
isCode
43-
hoverTip="Copy"
44-
clickTip="Copied"
45-
variant={ClipboardCopyVariant.expansion}
46-
>
47-
{componentStack.trim()}
48-
</ClipboardCopy>
49-
</DescriptionListDescription>
50-
</DescriptionListGroup>
37+
{componentStack ? (
38+
<DescriptionListGroup>
39+
<DescriptionListTerm>Component trace:</DescriptionListTerm>
40+
<DescriptionListDescription>
41+
<ClipboardCopy
42+
isExpanded
43+
isCode
44+
hoverTip="Copy"
45+
clickTip="Copied"
46+
variant={ClipboardCopyVariant.expansion}
47+
>
48+
{componentStack.trim()}
49+
</ClipboardCopy>
50+
</DescriptionListDescription>
51+
</DescriptionListGroup>
52+
) : null}
5153

5254
{stack ? (
5355
<DescriptionListGroup>
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import * as React from 'react';
2+
import { Button, Split, SplitItem, Title } from '@patternfly/react-core';
3+
import { TimesIcon } from '@patternfly/react-icons';
4+
5+
type ErrorFallbackLayoutProps = {
6+
onClose?: () => void;
7+
reloadButtonTestId?: string;
8+
closeButtonTestId?: string;
9+
children?: React.ReactNode;
10+
};
11+
12+
const ErrorFallbackLayout: React.FC<ErrorFallbackLayoutProps> = ({
13+
onClose,
14+
reloadButtonTestId = 'reload-link',
15+
closeButtonTestId = 'close-error-button',
16+
children,
17+
}) => (
18+
<>
19+
<Split>
20+
<SplitItem isFilled>
21+
<Title headingLevel="h1" className="pf-v6-u-mb-sm">
22+
An error occurred
23+
</Title>
24+
<p className="pf-v6-u-mb-md">
25+
Try{' '}
26+
<Button
27+
data-testid={reloadButtonTestId}
28+
variant="link"
29+
isInline
30+
onClick={() => window.location.reload()}
31+
>
32+
reloading
33+
</Button>{' '}
34+
the page if there was a recent update.
35+
</p>
36+
</SplitItem>
37+
{onClose ? (
38+
<SplitItem>
39+
<Button
40+
icon={<TimesIcon />}
41+
data-testid={closeButtonTestId}
42+
variant="plain"
43+
aria-label="Close"
44+
onClick={onClose}
45+
/>
46+
</SplitItem>
47+
) : null}
48+
</Split>
49+
{children}
50+
</>
51+
);
52+
53+
export default ErrorFallbackLayout;
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import * as React from 'react';
2+
import { PageSection } from '@patternfly/react-core';
3+
import { useRouteError } from 'react-router-dom';
4+
import ErrorDetails from './ErrorDetails';
5+
import ErrorFallbackLayout from './ErrorFallbackLayout';
6+
import { getRouteErrorDetails, isChunkLoadError } from './routeErrorUtils';
7+
import UpdateState from './UpdateState';
8+
9+
const RouteErrorElement: React.FC = () => {
10+
const error = useRouteError();
11+
const [showErrorDetails, setShowErrorDetails] = React.useState(false);
12+
const { title, errorMessage, stack } = getRouteErrorDetails(error);
13+
14+
React.useEffect(() => {
15+
setShowErrorDetails(false);
16+
}, [error]);
17+
18+
if (isChunkLoadError(error) && !showErrorDetails) {
19+
return <UpdateState onClose={() => setShowErrorDetails(true)} />;
20+
}
21+
22+
return (
23+
<PageSection
24+
hasBodyWrapper={false}
25+
className="pf-v6-u-p-lg"
26+
data-testid="router-error-boundary"
27+
>
28+
<ErrorFallbackLayout reloadButtonTestId="router-reload-link">
29+
<ErrorDetails title={title} errorMessage={errorMessage} stack={stack} />
30+
</ErrorFallbackLayout>
31+
</PageSection>
32+
);
33+
};
34+
35+
export default RouteErrorElement;
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import * as React from 'react';
2+
import '@testing-library/jest-dom';
3+
import { render, screen } from '@testing-library/react';
4+
import ErrorDetails from '#~/components/error/ErrorDetails';
5+
6+
describe('ErrorDetails', () => {
7+
it('should render description, component trace, and stack trace', () => {
8+
render(
9+
<ErrorDetails
10+
title="ChunkLoadError"
11+
errorMessage="Loading chunk 145 failed"
12+
componentStack={'\n at App\n at NavSidebar'}
13+
stack={'ChunkLoadError: Loading chunk 145 failed\n at __webpack_require__'}
14+
/>,
15+
);
16+
17+
expect(screen.getByText('ChunkLoadError')).toBeInTheDocument();
18+
expect(screen.getByText('Description:')).toBeInTheDocument();
19+
expect(screen.getByText('Loading chunk 145 failed')).toBeInTheDocument();
20+
expect(screen.getByText('Component trace:')).toBeInTheDocument();
21+
expect(screen.getByDisplayValue(/at App\s+at NavSidebar/)).toBeInTheDocument();
22+
expect(screen.getByText('Stack trace:')).toBeInTheDocument();
23+
});
24+
25+
it('should omit component trace when componentStack is not provided', () => {
26+
render(<ErrorDetails title="Route error" errorMessage="Unknown route error" />);
27+
28+
expect(screen.getByText('Route error')).toBeInTheDocument();
29+
expect(screen.getByText('Unknown route error')).toBeInTheDocument();
30+
expect(screen.queryByText('Component trace:')).not.toBeInTheDocument();
31+
});
32+
});
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import * as React from 'react';
2+
import { act } from 'react';
3+
import '@testing-library/jest-dom';
4+
import { render, screen } from '@testing-library/react';
5+
import ErrorFallbackLayout from '#~/components/error/ErrorFallbackLayout';
6+
7+
describe('ErrorFallbackLayout', () => {
8+
it('should reload the page when reload link is clicked', () => {
9+
Object.defineProperty(window, 'location', {
10+
configurable: true,
11+
value: { reload: jest.fn() },
12+
});
13+
14+
render(<ErrorFallbackLayout />);
15+
16+
const reloadButton = screen.getByTestId('reload-link');
17+
act(() => reloadButton.click());
18+
19+
expect(window.location.reload).toHaveBeenCalled();
20+
});
21+
22+
it('should render and handle close button when onClose is provided', () => {
23+
const onClose = jest.fn();
24+
25+
render(
26+
<ErrorFallbackLayout onClose={onClose}>
27+
<div>details</div>
28+
</ErrorFallbackLayout>,
29+
);
30+
31+
expect(screen.getByText('details')).toBeInTheDocument();
32+
const closeButton = screen.getByTestId('close-error-button');
33+
act(() => closeButton.click());
34+
35+
expect(onClose).toHaveBeenCalledTimes(1);
36+
});
37+
38+
it('should not render close button when onClose is not provided', () => {
39+
render(<ErrorFallbackLayout />);
40+
41+
expect(screen.queryByTestId('close-error-button')).not.toBeInTheDocument();
42+
});
43+
});
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import * as React from 'react';
2+
import { act } from 'react';
3+
import '@testing-library/jest-dom';
4+
import { render, screen } from '@testing-library/react';
5+
import { useRouteError } from 'react-router-dom';
6+
import RouteErrorElement from '#~/components/error/RouteErrorElement';
7+
8+
jest.mock('react-router-dom', () => ({
9+
...jest.requireActual('react-router-dom'),
10+
useRouteError: jest.fn(),
11+
}));
12+
13+
const useRouteErrorMock = jest.mocked(useRouteError);
14+
15+
describe('RouteErrorElement', () => {
16+
beforeEach(() => {
17+
jest.clearAllMocks();
18+
Object.defineProperty(window, 'location', {
19+
configurable: true,
20+
value: { reload: jest.fn() },
21+
});
22+
});
23+
24+
it('should render update state for ChunkLoadError and show details when requested', async () => {
25+
const error = new Error('Loading chunk 145 failed');
26+
error.name = 'ChunkLoadError';
27+
useRouteErrorMock.mockReturnValue(error);
28+
29+
render(<RouteErrorElement />);
30+
31+
expect(screen.getByTestId('error-update-state')).toBeInTheDocument();
32+
expect(screen.queryByTestId('router-error-boundary')).not.toBeInTheDocument();
33+
34+
const showErrorButton = screen.getByTestId('show-error-button');
35+
act(() => showErrorButton.click());
36+
37+
expect(screen.queryByTestId('error-update-state')).not.toBeInTheDocument();
38+
expect(screen.getByTestId('router-error-boundary')).toBeInTheDocument();
39+
expect(screen.getByText('ChunkLoadError')).toBeInTheDocument();
40+
expect(screen.getByText('Loading chunk 145 failed')).toBeInTheDocument();
41+
});
42+
43+
it('should reset to update state when route error changes', () => {
44+
let currentError: unknown = Object.assign(new Error('Loading chunk 145 failed'), {
45+
name: 'ChunkLoadError',
46+
});
47+
useRouteErrorMock.mockImplementation(() => currentError);
48+
49+
const { rerender } = render(<RouteErrorElement />);
50+
expect(screen.getByTestId('error-update-state')).toBeInTheDocument();
51+
52+
act(() => {
53+
screen.getByTestId('show-error-button').click();
54+
});
55+
expect(screen.getByTestId('router-error-boundary')).toBeInTheDocument();
56+
57+
currentError = Object.assign(new Error('Loading chunk src_images_icons_Settings failed'), {
58+
name: 'ChunkLoadError',
59+
});
60+
rerender(<RouteErrorElement />);
61+
62+
expect(screen.getByTestId('error-update-state')).toBeInTheDocument();
63+
expect(screen.queryByTestId('router-error-boundary')).not.toBeInTheDocument();
64+
});
65+
66+
it('should render route error fallback for non-chunk errors', () => {
67+
useRouteErrorMock.mockReturnValue(new Error('regular route error'));
68+
69+
render(<RouteErrorElement />);
70+
71+
expect(screen.queryByTestId('error-update-state')).not.toBeInTheDocument();
72+
expect(screen.getByTestId('router-error-boundary')).toBeInTheDocument();
73+
expect(screen.getByText('Error')).toBeInTheDocument();
74+
expect(screen.getByText('regular route error')).toBeInTheDocument();
75+
});
76+
});

0 commit comments

Comments
 (0)