Skip to content

Commit fe656a3

Browse files
authored
[SideNav] Wrap SideNav in ErrorBoundary with retry (#239945)
## Summary Sidenav is an important but complex component. It is also part of the core React tree. An error there might break the whole app. Here is an example of such a sneaky bug #238460 To fix this, I suggest we wrap the component in an error boundary. Instead of showing an error, we will reset the component's state and start over. when maxRetry hits, we show the error.
1 parent ec9b5a9 commit fe656a3

File tree

8 files changed

+146
-21
lines changed

8 files changed

+146
-21
lines changed

src/core/packages/chrome/browser-internal/src/ui/project/sidenav/navigation/navigation.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import type { IBasePath as BasePath } from '@kbn/core-http-browser';
2222
import type { ApplicationStart } from '@kbn/core-application-browser';
2323
import type { NavigationTourManager } from '@kbn/core-chrome-navigation-tour';
2424
import { NavigationTour } from '@kbn/core-chrome-navigation-tour';
25+
import { KibanaSectionErrorBoundary } from '@kbn/shared-ux-error-boundary';
2526
import useObservable from 'react-use/lib/useObservable';
2627
import type { NavigationItems } from './to_navigation_items';
2728
import { toNavigationItems } from './to_navigation_items';
@@ -67,7 +68,7 @@ export const Navigation = (props: ChromeNavigationProps) => {
6768
const { navItems, logoItem, activeItemId, solutionId } = state;
6869

6970
return (
70-
<>
71+
<KibanaSectionErrorBoundary sectionName={'Navigation'} maxRetries={3}>
7172
<NavigationTour
7273
tourManager={props.navigationTourManager}
7374
key={
@@ -89,7 +90,7 @@ export const Navigation = (props: ChromeNavigationProps) => {
8990
activeItemId={activeItemId}
9091
data-test-subj={classnames(dataTestSubj, 'projectSideNav', 'projectSideNavV2')}
9192
/>
92-
</>
93+
</KibanaSectionErrorBoundary>
9394
);
9495
};
9596

src/core/packages/chrome/browser-internal/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
"@kbn/core-feature-flags-browser-mocks",
7070
"@kbn/shared-ux-feedback-snippet",
7171
"@kbn/core-chrome-navigation-tour",
72+
"@kbn/shared-ux-error-boundary",
7273
],
7374
"exclude": [
7475
"target/**/*",

src/platform/packages/shared/react/kibana_context/root/root_provider.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type { AnalyticsServiceStart } from '@kbn/core-analytics-browser';
1414
import type { I18nStart } from '@kbn/core-i18n-browser';
1515
import type { ExecutionContextStart } from '@kbn/core-execution-context-browser';
1616
import { SharedUXRouterContext } from '@kbn/shared-ux-router';
17+
import { KibanaErrorBoundaryProvider } from '@kbn/shared-ux-error-boundary';
1718

1819
// @ts-expect-error EUI exports this component internally, but Kibana isn't picking it up its types
1920
import { useIsNestedEuiProvider } from '@elastic/eui/lib/components/provider/nested';
@@ -54,9 +55,11 @@ export const KibanaRootContextProvider: FC<PropsWithChildren<KibanaRootContextPr
5455
}) => {
5556
const hasEuiProvider = useIsNestedEuiProvider();
5657
const rootContextProvider = (
57-
<SharedUXRouterContext.Provider value={{ services: { executionContext } }}>
58-
<i18n.Context>{children}</i18n.Context>
59-
</SharedUXRouterContext.Provider>
58+
<KibanaErrorBoundaryProvider analytics={props.analytics}>
59+
<SharedUXRouterContext.Provider value={{ services: { executionContext } }}>
60+
<i18n.Context>{children}</i18n.Context>
61+
</SharedUXRouterContext.Provider>
62+
</KibanaErrorBoundaryProvider>
6063
);
6164

6265
if (hasEuiProvider) {

src/platform/packages/shared/react/kibana_context/root/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
"@kbn/core-execution-context-browser",
2727
"@kbn/core-execution-context-browser-mocks",
2828
"@kbn/shared-ux-router",
29-
"@kbn/core-chrome-layout-constants"
29+
"@kbn/core-chrome-layout-constants",
30+
"@kbn/shared-ux-error-boundary"
3031
]
3132
}

src/platform/packages/shared/shared-ux/error_boundary/README.mdx

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,61 @@ date: 2023-10-03
99

1010
## Description
1111

12-
This package exports `KibanaErrorBoundary` and `KibanaSectionErrorBoundary` components.
12+
This package exports error boundary components to handle rendering errors gracefully:
1313

14-
- `KibanaErrorBoundary` is designed to capture rendering errors by blocking the main part of the UI.
15-
- `KibanaSectionErrorBoundary` is designed to capture errors at a more granular level.
14+
- `KibanaErrorBoundary` - Captures rendering errors at the page level and blocks the main UI
15+
- `KibanaSectionErrorBoundary` - Captures errors at a granular section level with optional automatic retry capability
1616

1717
In general, it's best to use `KibanaErrorBoundary` and block the whole page. If it is acceptable to assume the risk of allowing users to interact with a UI that has an error state, then using `KibanaSectionErrorBoundary` may be an acceptable alternative, but this must be judged on a case-by-case basis.
1818

1919
## API
2020

21+
### KibanaErrorBoundary
22+
23+
Page-level error boundary that captures rendering errors and blocks the main UI until refresh.
24+
25+
```tsx
26+
<KibanaErrorBoundary>
27+
<YourApp />
28+
</KibanaErrorBoundary>
29+
```
30+
31+
**Props:**
32+
33+
- `children` (ReactNode, required) - Child components to render and protect
34+
35+
### KibanaSectionErrorBoundary
36+
37+
Section-level error boundary that captures errors without blocking the entire page. Supports optional automatic retry with fresh component state.
38+
39+
```tsx
40+
// Without retries (default)
41+
<KibanaSectionErrorBoundary sectionName="Dashboard Panel">
42+
<YourComponent />
43+
</KibanaSectionErrorBoundary>
44+
45+
// With automatic retry attempts
46+
<KibanaSectionErrorBoundary sectionName="Dashboard Panel" maxRetries={3}>
47+
<YourComponent />
48+
</KibanaSectionErrorBoundary>
49+
```
50+
51+
**Props:**
52+
53+
- `sectionName` (string, required) - Name of the section for error messaging
54+
- `maxRetries` (number, optional, default: 0) - Number of times to automatically remount children when an error occurs. When > 0, the section will attempt to recover by remounting with fresh state before showing the error prompt.
55+
- `children` (ReactNode, required) - Child components to render and protect
56+
57+
**Retry Behavior:**
58+
When `maxRetries > 0`, if an error is caught, the component will:
59+
60+
1. Automatically remount children with a fresh Fragment key (forcing fresh component state)
61+
2. Increment retry counter
62+
3. If another error occurs and retries are exhausted, show the error prompt
63+
4. If no error occurs during retry, render normally
64+
65+
This is useful for components that may have transient errors that resolve with fresh state, or where component state corruption could be recovered by remounting.
66+
2167
## EUI Promotion Status
2268

2369
This component is specialized for error messages internal to Kibana and is not intended for promotion to EUI.

src/platform/packages/shared/shared-ux/error_boundary/src/ui/error_boundary.recoverable.stories.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ export const SectionErrorInCallout: StoryFn = () => {
6464
<ChunkLoadErrorComponent />
6565
</KibanaSectionErrorBoundary>
6666
</EuiFormFieldset>
67+
<EuiFormFieldset legend={{ children: 'Section C with 3 retries' }}>
68+
<KibanaSectionErrorBoundary sectionName="sectionC" maxRetries={3}>
69+
<ChunkLoadErrorComponent />
70+
</KibanaSectionErrorBoundary>
71+
</EuiFormFieldset>
6772
</KibanaErrorBoundaryDepsProvider>
6873
</Template>
6974
);

src/platform/packages/shared/shared-ux/error_boundary/src/ui/section_error_boundary.test.tsx

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,13 @@ describe('<KibanaSectionErrorBoundary>', () => {
4545
jest.useRealTimers();
4646
});
4747

48-
const Template: FC<PropsWithChildren<unknown>> = ({ children }) => {
48+
const Template: FC<PropsWithChildren<{ maxRetries?: number }>> = ({
49+
children,
50+
maxRetries = 0,
51+
}) => {
4952
return (
5053
<KibanaErrorBoundaryDepsProvider {...services}>
51-
<KibanaSectionErrorBoundary sectionName="test section name">
54+
<KibanaSectionErrorBoundary sectionName="test section name" maxRetries={maxRetries}>
5255
{children}
5356
</KibanaSectionErrorBoundary>
5457
</KibanaErrorBoundaryDepsProvider>
@@ -217,4 +220,39 @@ describe('<KibanaSectionErrorBoundary>', () => {
217220
expect(payload.component_name).toBe('BadComponent');
218221
expect(payload.error_message).toBe('Error: This is an error to show the test user!');
219222
});
223+
224+
describe('maxRetries behavior', () => {
225+
it('defaults to maxRetries=0 and shows error immediately', async () => {
226+
const { getByTestId, getByText } = render(
227+
<Template>
228+
<BadComponent />
229+
</Template>
230+
);
231+
await user.click(getByTestId('clickForErrorBtn'));
232+
233+
// Error prompt should be visible immediately with no retries
234+
expect(getByText(strings.section.callout.fatal.title('test section name'))).toBeVisible();
235+
});
236+
237+
it('shows error prompt after maxRetries exhausted', async () => {
238+
const { getByTestId, getByText, queryByText } = render(
239+
<Template maxRetries={1}>
240+
<BadComponent />
241+
</Template>
242+
);
243+
244+
// Trigger first error (will retry)
245+
await user.click(getByTestId('clickForErrorBtn'));
246+
247+
// Error prompt should NOT be visible yet (it's retrying)
248+
expect(queryByText(strings.section.callout.fatal.title('test section name'))).toBeNull();
249+
250+
// Trigger error again (second error, exceeds maxRetries)
251+
// Since we're in a retried state, clicking the button again triggers another error
252+
await user.click(getByTestId('clickForErrorBtn'));
253+
254+
// Now error prompt should be visible (retries exhausted)
255+
expect(getByText(strings.section.callout.fatal.title('test section name'))).toBeVisible();
256+
});
257+
});
220258
});

src/platform/packages/shared/shared-ux/error_boundary/src/ui/section_error_boundary.tsx

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
import React from 'react';
10+
import React, { Fragment } from 'react';
1111
import { apm } from '@elastic/apm-rum';
1212

1313
import { getErrorBoundaryLabels } from '../../lib';
@@ -21,6 +21,13 @@ import {
2121

2222
interface SectionErrorBoundaryProps {
2323
sectionName: string;
24+
/** How many times to retry remounting before showing error (default: 0, no retries) */
25+
maxRetries?: number;
26+
}
27+
28+
interface SectionErrorBoundaryState extends BaseErrorBoundaryState {
29+
/** How many times we've retried */
30+
retryCount?: number;
2431
}
2532

2633
/**
@@ -34,6 +41,13 @@ interface SectionErrorBoundaryProps {
3441
* If it is acceptable to assume the risk of allowing users to interact with a UI that
3542
* has an error state, then using `KibanaSectionErrorBoundary` may be an acceptable alternative,
3643
* but this must be judged on a case-by-case basis.
44+
*
45+
* @example
46+
* ```tsx
47+
* <KibanaSectionErrorBoundary sectionName="Dashboard" maxRetries={3}>
48+
* <MySection />
49+
* </KibanaSectionErrorBoundary>
50+
* ```
3751
*/
3852
export const KibanaSectionErrorBoundary = (
3953
props: React.PropsWithChildren<SectionErrorBoundaryProps>
@@ -44,7 +58,7 @@ export const KibanaSectionErrorBoundary = (
4458

4559
class SectionErrorBoundaryInternal extends BaseErrorBoundary<
4660
React.PropsWithChildren<SectionErrorBoundaryProps> & BaseErrorBoundaryProps,
47-
BaseErrorBoundaryState
61+
SectionErrorBoundaryState
4862
> {
4963
constructor(props: SectionErrorBoundaryProps & BaseErrorBoundaryProps) {
5064
super(props);
@@ -54,6 +68,7 @@ class SectionErrorBoundaryInternal extends BaseErrorBoundary<
5468
errorInfo: null,
5569
componentName: null,
5670
isFatal: null,
71+
retryCount: 0,
5772
};
5873
}
5974

@@ -68,21 +83,36 @@ class SectionErrorBoundaryInternal extends BaseErrorBoundary<
6883
const enqueuedError = this.props.services.errorService.enqueueError(error, errorInfo);
6984
const { id: errorId, isFatal, name } = enqueuedError;
7085

71-
this.setState({
72-
error,
73-
errorInfo,
74-
componentName: name,
75-
isFatal,
76-
errorId,
86+
this.setState((prevState) => {
87+
const nextRetryCount = (prevState.retryCount ?? 0) + 1;
88+
89+
return {
90+
error,
91+
errorInfo,
92+
componentName: name,
93+
isFatal,
94+
errorId,
95+
retryCount: nextRetryCount,
96+
};
7797
});
7898
}
7999

80100
render() {
81-
if (!this.state.error) {
101+
const { error, retryCount = 0 } = this.state;
102+
const { maxRetries = 0 } = this.props;
103+
const hasRetriesRemaining = retryCount <= maxRetries && error !== null;
104+
105+
// If there are retries remaining, remount children with fresh state
106+
if (hasRetriesRemaining) {
107+
return <Fragment key={retryCount}>{this.props.children}</Fragment>;
108+
}
109+
110+
// Once retries are exhausted or no error, show normal children or error prompt
111+
if (!error) {
82112
return this.props.children;
83113
}
84114

85-
const { error, errorInfo, componentName, isFatal } = this.state;
115+
const { errorInfo, componentName, isFatal } = this.state;
86116

87117
if (isFatal) {
88118
return (

0 commit comments

Comments
 (0)