Skip to content

Commit

Permalink
fix: Wrap the children of ReactPanel with an ErrorBoundary (deephaven…
Browse files Browse the repository at this point in the history
…#565)

- Previously, if a child of the ReactPanel threw, it would cause the
panel to close, which could cause the widget to close unexpectedly.
- Instead, have the ReactPanel catch the error and display an error
view.
- If any of the children of ReactPanel throw, it won't cause the panel
to close. It will just show the error in the panel now.
- Tested against Core with a special branch
[debug-kill-table](https://github.com/mofojed/web-client-ui/tree/debug-kill-table)
  - Run the following snippet to create a panel with a table in it:
```
from deephaven import time_table, ui
p = ui.panel(time_table("PT1s"))
```
- Open the browser console, and enter the command `dhKillTable()`. The
table stops ticking as it's closed.
- Hover the mouse over the table. This will trigger a NPE and a debugger
breakpoint; hit resume a couple times until things resume. An error will
be shown on the panel
- To simulate "reconnecting", then disable the FetchManager and
re-enable it: `dhDisableFetchManager()`, `dhEnableFetchManager()`
  - Table should be reloaded and ticking again.
  • Loading branch information
mofojed authored Jun 21, 2024
1 parent 4b4e9db commit 8cbee84
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
35 changes: 34 additions & 1 deletion plugins/ui/src/js/src/layout/ReactPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { render } from '@testing-library/react';
import { render, within } from '@testing-library/react';
import { LayoutUtils, useListener } from '@deephaven/dashboard';
import { TestUtils } from '@deephaven/utils';
import ReactPanel from './ReactPanel';
import {
ReactPanelManager,
Expand Down Expand Up @@ -260,3 +261,35 @@ it('calls setActiveContentItem if metadata changed while the panel already exist
expect(onClose).not.toHaveBeenCalled();
expect(mockStack.setActiveContentItem).toHaveBeenCalledTimes(1);
});

it('catches an error thrown by children, renders error view', () => {
TestUtils.disableConsoleOutput();

const error = new Error('test error');
const ErrorComponent = () => {
throw error;
};

const portal = document.createElement('div');
const portals = new Map([[mockPanelId, portal]]);

const { rerender } = render(
<PortalPanelManagerContext.Provider value={portals}>
{makeReactPanelManager({
children: <ErrorComponent />,
})}
</PortalPanelManagerContext.Provider>
);
const { getByText } = within(portal);
expect(getByText('Error: test error')).toBeDefined();

rerender(
<PortalPanelManagerContext.Provider value={portals}>
{makeReactPanelManager({
children: <div>Hello</div>,
})}
</PortalPanelManagerContext.Provider>
);

expect(getByText('Hello')).toBeDefined();
});
15 changes: 13 additions & 2 deletions plugins/ui/src/js/src/layout/ReactPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import {
useLayoutManager,
useListener,
} from '@deephaven/dashboard';
import { View, ViewProps, Flex, FlexProps } from '@deephaven/components';
import {
View,
ViewProps,
Flex,
FlexProps,
ErrorBoundary,
} from '@deephaven/components';
import Log from '@deephaven/log';
import PortalPanel from './PortalPanel';
import { ReactPanelControl, useReactPanel } from './ReactPanelManager';
Expand Down Expand Up @@ -94,6 +100,10 @@ function ReactPanel({
// eslint-disable-next-line react-hooks/exhaustive-deps
const contentKey = useMemo(() => shortid.generate(), [metadata]);

// We want to regenerate the error boundary key every time the children change, so that the error is cleared
// eslint-disable-next-line react-hooks/exhaustive-deps
const errorKey = useMemo(() => shortid.generate(), [children]);

const parent = useParentItem();
const { eventHub } = layoutManager;

Expand Down Expand Up @@ -199,7 +209,8 @@ function ReactPanel({
rowGap={rowGap}
columnGap={columnGap}
>
{children}
{/* Have an ErrorBoundary around the children to display an error in the panel if there's any errors thrown when rendering the children */}
<ErrorBoundary key={errorKey}>{children}</ErrorBoundary>
</Flex>
</View>
<ReactPanelContentOverlay />
Expand Down

0 comments on commit 8cbee84

Please sign in to comment.