Skip to content

Commit 9261fe0

Browse files
[8.19] [Unified Doc Viewer] Fix doc viewer tab error handling (#229220) (#229697)
# Backport This will backport the following commits from `main` to `8.19`: - [[Unified Doc Viewer] Fix doc viewer tab error handling (#229220)](#229220) <!--- Backport version: 10.0.1 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Davis McPhee","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-07-28T03:40:19Z","message":"[Unified Doc Viewer] Fix doc viewer tab error handling (#229220)\n\n## Summary\n\nThis PR fixes the Unified Doc Viewer error handling so unhandled errors\nwithin specific tabs don't cause all tabs to stop working. It also\nsimplifies the doc viewer tab code and removes cruft from when we used\nto support both React and Angular doc views. Lastly, it updates the doc\nviewer tab error boundary to use `KibanaSectionErrorBoundary` instead of\n`EuiErrorBoundary` so that unhandled tab errors are captured in\ntelemetry.\n\nResolves #207389.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n## Release note\n\nThis PR removes support for non-React document viewer tabs registered\nthrough the Unified Doc Viewer plugin. If you maintain an external\nplugin that registers non-React document viewer tabs, consider migrating\nto React or implementing a wrapper similar to how the removed\n[`DocViewRenderTab`](https://github.com/elastic/kibana/blob/0f156d6501e5262fb853ec6124809223390e4806/src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/doc_viewer_render_tab.tsx)\nworked. Note that backward compatibility with external plugins is [not\nsupported](https://www.elastic.co/docs/extend/kibana/external-plugin-development)\nby Kibana.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>\nCo-authored-by: Matthias Wilhelm <[email protected]>","sha":"cbafad4df27592287d8ab13c820438de84d07eaf","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:DataDiscovery","backport:version","v9.1.0","v8.19.0","v9.2.0","v9.1.1"],"title":"[Unified Doc Viewer] Fix doc viewer tab error handling","number":229220,"url":"https://github.com/elastic/kibana/pull/229220","mergeCommit":{"message":"[Unified Doc Viewer] Fix doc viewer tab error handling (#229220)\n\n## Summary\n\nThis PR fixes the Unified Doc Viewer error handling so unhandled errors\nwithin specific tabs don't cause all tabs to stop working. It also\nsimplifies the doc viewer tab code and removes cruft from when we used\nto support both React and Angular doc views. Lastly, it updates the doc\nviewer tab error boundary to use `KibanaSectionErrorBoundary` instead of\n`EuiErrorBoundary` so that unhandled tab errors are captured in\ntelemetry.\n\nResolves #207389.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n## Release note\n\nThis PR removes support for non-React document viewer tabs registered\nthrough the Unified Doc Viewer plugin. If you maintain an external\nplugin that registers non-React document viewer tabs, consider migrating\nto React or implementing a wrapper similar to how the removed\n[`DocViewRenderTab`](https://github.com/elastic/kibana/blob/0f156d6501e5262fb853ec6124809223390e4806/src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/doc_viewer_render_tab.tsx)\nworked. Note that backward compatibility with external plugins is [not\nsupported](https://www.elastic.co/docs/extend/kibana/external-plugin-development)\nby Kibana.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>\nCo-authored-by: Matthias Wilhelm <[email protected]>","sha":"cbafad4df27592287d8ab13c820438de84d07eaf"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/229550","number":229550,"state":"MERGED","mergeCommit":{"sha":"84f86498510d3cc25c60087e958741e4b55ca5e0","message":"[9.1] [Unified Doc Viewer] Fix doc viewer tab error handling (#229220) (#229550)\n\n# Backport\n\nThis will backport the following commits from `main` to `9.1`:\n- [[Unified Doc Viewer] Fix doc viewer tab error handling\n(#229220)](https://github.com/elastic/kibana/pull/229220)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\nCo-authored-by: Davis McPhee <[email protected]>\nCo-authored-by: Matthias Wilhelm <[email protected]>"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/229220","number":229220,"mergeCommit":{"message":"[Unified Doc Viewer] Fix doc viewer tab error handling (#229220)\n\n## Summary\n\nThis PR fixes the Unified Doc Viewer error handling so unhandled errors\nwithin specific tabs don't cause all tabs to stop working. It also\nsimplifies the doc viewer tab code and removes cruft from when we used\nto support both React and Angular doc views. Lastly, it updates the doc\nviewer tab error boundary to use `KibanaSectionErrorBoundary` instead of\n`EuiErrorBoundary` so that unhandled tab errors are captured in\ntelemetry.\n\nResolves #207389.\n\n### Checklist\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [x] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [x] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n## Release note\n\nThis PR removes support for non-React document viewer tabs registered\nthrough the Unified Doc Viewer plugin. If you maintain an external\nplugin that registers non-React document viewer tabs, consider migrating\nto React or implementing a wrapper similar to how the removed\n[`DocViewRenderTab`](https://github.com/elastic/kibana/blob/0f156d6501e5262fb853ec6124809223390e4806/src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/doc_viewer_render_tab.tsx)\nworked. Note that backward compatibility with external plugins is [not\nsupported](https://www.elastic.co/docs/extend/kibana/external-plugin-development)\nby Kibana.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>\nCo-authored-by: Matthias Wilhelm <[email protected]>","sha":"cbafad4df27592287d8ab13c820438de84d07eaf"}}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <[email protected]>
1 parent 1a83bd5 commit 9261fe0

File tree

17 files changed

+182
-364
lines changed

17 files changed

+182
-364
lines changed

src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/__snapshots__/doc_viewer.test.tsx.snap

Lines changed: 66 additions & 57 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/__snapshots__/doc_viewer_render_tab.test.tsx.snap

Lines changed: 0 additions & 20 deletions
This file was deleted.

src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/doc_viewer.test.tsx

Lines changed: 83 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@
88
*/
99

1010
import React from 'react';
11-
import { mount, shallow } from 'enzyme';
1211
import { act, render, screen } from '@testing-library/react';
13-
import { findTestSubject } from '@elastic/eui/lib/test';
1412
import { buildDataTableRecord } from '@kbn/discover-utils';
15-
import { DocViewer, INITIAL_TAB } from './doc_viewer';
13+
import { DocViewer, DocViewerProps, INITIAL_TAB } from './doc_viewer';
1614
import type { DocViewRenderProps } from '../../types';
1715
import { DocViewsRegistry } from '../..';
1816
import { dataViewMock, esHitsMockWithSort } from '@kbn/discover-utils/src/__mocks__';
17+
import { analyticsServiceMock } from '@kbn/core-analytics-browser-mocks';
18+
import { KibanaErrorBoundaryProvider } from '@kbn/shared-ux-error-boundary';
19+
import userEvent from '@testing-library/user-event';
1920

21+
const analytics = analyticsServiceMock.createAnalyticsServiceStart();
2022
const records = esHitsMockWithSort.map((hit) => buildDataTableRecord(hit, dataViewMock));
2123

2224
const mockSetLocalStorage = jest.fn();
@@ -32,58 +34,100 @@ jest.mock('react-use/lib/useLocalStorage', () => {
3234
});
3335
});
3436

37+
const WrappedDocViewer = (props: DocViewerProps) => (
38+
<KibanaErrorBoundaryProvider analytics={analytics}>
39+
<DocViewer {...props} />
40+
</KibanaErrorBoundaryProvider>
41+
);
42+
3543
describe('<DocViewer />', () => {
3644
test('Render <DocViewer/> with 3 different tabs', () => {
3745
const registry = new DocViewsRegistry();
38-
registry.add({ id: 'function', order: 10, title: 'Render function', render: jest.fn() });
46+
registry.add({
47+
id: 'function',
48+
order: 10,
49+
title: 'Render function',
50+
component: jest.fn(() => null),
51+
});
3952
registry.add({
4053
id: 'component',
4154
order: 20,
4255
title: 'React component',
4356
component: () => <div>test</div>,
4457
});
45-
// @ts-expect-error This should be invalid and will throw an error when rendering
46-
registry.add({ id: 'invalid', order: 30, title: 'Invalid doc view' });
58+
registry.add({
59+
id: 'invalid',
60+
order: 30,
61+
title: 'Invalid doc view',
62+
component: () => {
63+
throw new Error('Invalid');
64+
},
65+
});
4766

4867
const renderProps = { hit: {} } as DocViewRenderProps;
4968

50-
const wrapper = shallow(<DocViewer docViews={registry.getAll()} {...renderProps} />);
69+
const { container } = render(
70+
<WrappedDocViewer docViews={registry.getAll()} {...renderProps} />
71+
);
5172

52-
expect(wrapper).toMatchSnapshot();
73+
expect(container).toMatchSnapshot();
5374
});
5475

55-
test('Render <DocViewer/> with 1 tab displaying error message', () => {
56-
function SomeComponent() {
57-
// this is just a placeholder
58-
return null;
59-
}
60-
76+
test('Render <DocViewer/> with 1 tab displaying error message', async () => {
77+
const errorMsg = 'Catch me if you can!';
6178
const registry = new DocViewsRegistry();
6279
registry.add({
6380
id: 'component',
6481
order: 10,
6582
title: 'React component',
66-
component: SomeComponent,
83+
component: () => {
84+
throw new Error(errorMsg);
85+
},
6786
});
68-
6987
const renderProps = {
7088
hit: buildDataTableRecord({ _index: 't', _id: '1' }),
7189
} as DocViewRenderProps;
72-
const errorMsg = 'Catch me if you can!';
90+
render(<WrappedDocViewer docViews={registry.getAll()} {...renderProps} />);
91+
expect(screen.getByTestId('sectionErrorBoundaryPromptBody')).toBeInTheDocument();
92+
await userEvent.click(screen.getByText('Show details'));
93+
expect(screen.getByText(new RegExp(`${errorMsg}`))).toBeInTheDocument();
94+
});
7395

74-
const wrapper = mount(<DocViewer docViews={registry.getAll()} {...renderProps} />);
75-
const error = new Error(errorMsg);
76-
wrapper.find(SomeComponent).simulateError(error);
77-
const errorMsgComponent = findTestSubject(wrapper, 'docViewerError');
78-
expect(errorMsgComponent.text()).toMatch(new RegExp(`${errorMsg}`));
96+
test('should not prevent rendering of other tabs when one tab throws an error', async () => {
97+
const registry = new DocViewsRegistry();
98+
registry.add({
99+
id: 'valid',
100+
order: 10,
101+
title: 'Valid doc view',
102+
component: () => <div>Valid</div>,
103+
});
104+
registry.add({
105+
id: 'error',
106+
order: 20,
107+
title: 'Error doc view',
108+
component: () => {
109+
throw new Error('Invalid');
110+
},
111+
});
112+
const renderProps = {
113+
hit: buildDataTableRecord({ _index: 't', _id: '1' }),
114+
} as DocViewRenderProps;
115+
render(<WrappedDocViewer docViews={registry.getAll()} {...renderProps} />);
116+
expect(screen.queryByTestId('sectionErrorBoundaryPromptBody')).not.toBeInTheDocument();
117+
await userEvent.click(screen.getByText('Error doc view'));
118+
expect(screen.getByTestId('sectionErrorBoundaryPromptBody')).toBeInTheDocument();
119+
await userEvent.click(screen.getByText('Valid doc view'));
120+
expect(screen.queryByTestId('sectionErrorBoundaryPromptBody')).not.toBeInTheDocument();
79121
});
80122

81123
test('should save active tab to local storage', () => {
82124
const registry = new DocViewsRegistry();
83-
registry.add({ id: 'test1', order: 10, title: 'Render function', render: jest.fn() });
84-
registry.add({ id: 'test2', order: 20, title: 'Render function', render: jest.fn() });
125+
registry.add({ id: 'test1', order: 10, title: 'Render function', component: jest.fn() });
126+
registry.add({ id: 'test2', order: 20, title: 'Render function', component: jest.fn() });
85127

86-
render(<DocViewer docViews={registry.getAll()} hit={records[0]} dataView={dataViewMock} />);
128+
render(
129+
<WrappedDocViewer docViews={registry.getAll()} hit={records[0]} dataView={dataViewMock} />
130+
);
87131

88132
expect(screen.getByTestId('docViewerTab-test1').getAttribute('aria-selected')).toBe('true');
89133
expect(screen.getByTestId('docViewerTab-test2').getAttribute('aria-selected')).toBe('false');
@@ -107,12 +151,14 @@ describe('<DocViewer />', () => {
107151

108152
test('should restore active tab from local storage', () => {
109153
const registry = new DocViewsRegistry();
110-
registry.add({ id: 'test1', order: 10, title: 'Render function', render: jest.fn() });
111-
registry.add({ id: 'test2', order: 20, title: 'Render function', render: jest.fn() });
154+
registry.add({ id: 'test1', order: 10, title: 'Render function', component: jest.fn() });
155+
registry.add({ id: 'test2', order: 20, title: 'Render function', component: jest.fn() });
112156

113157
mockTestInitialLocalStorageValue = 'kbn_doc_viewer_tab_test2';
114158

115-
render(<DocViewer docViews={registry.getAll()} hit={records[0]} dataView={dataViewMock} />);
159+
render(
160+
<WrappedDocViewer docViews={registry.getAll()} hit={records[0]} dataView={dataViewMock} />
161+
);
116162

117163
expect(screen.getByTestId('docViewerTab-test1').getAttribute('aria-selected')).toBe('false');
118164
expect(screen.getByTestId('docViewerTab-test2').getAttribute('aria-selected')).toBe('true');
@@ -122,12 +168,14 @@ describe('<DocViewer />', () => {
122168

123169
test('should not restore a tab from local storage if unavailable', () => {
124170
const registry = new DocViewsRegistry();
125-
registry.add({ id: 'test1', order: 10, title: 'Render function', render: jest.fn() });
126-
registry.add({ id: 'test2', order: 20, title: 'Render function', render: jest.fn() });
171+
registry.add({ id: 'test1', order: 10, title: 'Render function', component: jest.fn() });
172+
registry.add({ id: 'test2', order: 20, title: 'Render function', component: jest.fn() });
127173

128174
mockTestInitialLocalStorageValue = 'kbn_doc_viewer_tab_test3';
129175

130-
render(<DocViewer docViews={registry.getAll()} hit={records[0]} dataView={dataViewMock} />);
176+
render(
177+
<WrappedDocViewer docViews={registry.getAll()} hit={records[0]} dataView={dataViewMock} />
178+
);
131179

132180
expect(screen.getByTestId('docViewerTab-test1').getAttribute('aria-selected')).toBe('true');
133181
expect(screen.getByTestId('docViewerTab-test2').getAttribute('aria-selected')).toBe('false');
@@ -139,12 +187,12 @@ describe('<DocViewer />', () => {
139187
const initialTabId = 'test2';
140188

141189
const registry = new DocViewsRegistry();
142-
registry.add({ id: 'test1', order: 10, title: 'Render 1st Tab', render: jest.fn() });
143-
registry.add({ id: initialTabId, order: 20, title: 'Render 2nd Tab', render: jest.fn() });
144-
registry.add({ id: 'test3', order: 30, title: 'Render 3rd Tab', render: jest.fn() });
190+
registry.add({ id: 'test1', order: 10, title: 'Render 1st Tab', component: jest.fn() });
191+
registry.add({ id: initialTabId, order: 20, title: 'Render 2nd Tab', component: jest.fn() });
192+
registry.add({ id: 'test3', order: 30, title: 'Render 3rd Tab', component: jest.fn() });
145193

146194
render(
147-
<DocViewer
195+
<WrappedDocViewer
148196
docViews={registry.getAll()}
149197
initialTabId={initialTabId}
150198
hit={records[0]}

src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/doc_viewer.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,16 @@ export const DocViewer = forwardRef<DocViewerApi, DocViewerInternalProps>(
3636
({ docViews, initialTabId, ...renderProps }, ref) => {
3737
const tabs = docViews
3838
.filter(({ enabled }) => enabled) // Filter out disabled doc views
39-
.map(({ id, title, render, component }: DocView) => ({
39+
.map(({ id, title, component }: DocView) => ({
4040
id: getFullTabId(id), // `id` value is used to persist the selected tab in localStorage
4141
name: title,
4242
content: (
4343
<DocViewerTab
44+
key={id}
4445
id={id}
4546
title={title}
4647
component={component}
4748
renderProps={renderProps}
48-
render={render}
4949
/>
5050
),
5151
['data-test-subj']: `docViewerTab-${id}`,

src/platform/packages/shared/kbn-unified-doc-viewer/src/components/doc_viewer/doc_viewer_error.test.tsx

Lines changed: 0 additions & 25 deletions
This file was deleted.

0 commit comments

Comments
 (0)