Skip to content

Commit

Permalink
feat: Display deephaven.ui widget errors in a panel so user can see t…
Browse files Browse the repository at this point in the history
…hem (#436)

- Widget can now emit a `documentError` on the server to indicate there
was an error rendering the document
- Client listens for `documentError` and will display the error message
in a panel along with a Reload button to reload the widget
- Updated `@deephaven/*` packages to 0.76.0, which includes the
`ErrorView` and is also in deephaven-core 0.34.1
- Fixes #437
- Fixes #85 
- Tested with a `ui_boom` and `ui_boom_counter` components:
```python
from deephaven import ui

@ui.component
def ui_boom():
    raise Exception("BOOM!")

@ui.component
def ui_boom_counter():
    value, set_value = ui.use_state(0)

    if value > 5:
        raise ValueError("BOOM! Value is bigger than 5.")
    
    return ui.button(f"Count is {value}", on_press=lambda: set_value(value + 1))

boom = ui_boom()
boom_counter = ui_boom_counter()
```
  • Loading branch information
mofojed authored May 10, 2024
1 parent be887f7 commit b23b571
Show file tree
Hide file tree
Showing 19 changed files with 4,212 additions and 13,270 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pre-commit run --all-files
All steps should pass.

To bypass the pre-commit hook, you can commit with the `--no-verify` flag, for example:

```shell
git commit --no-verify -m "commit message"`
```
Expand All @@ -67,7 +68,7 @@ We use [Playwright](https://playwright.dev/) for end-to-end tests. We test again

You should be able to pass arguments to these commands as if you were running Playwright via CLI directly. For example, to test only `matplotlib.spec.ts` you could run `npm run e2e:docker -- ./tests/matplotlib.spec.ts`, or to test only `matplotlib.spec.ts` in Firefox, you could run `npm run e2e:docker -- --project firefox ./tests/matplotlib.spec.ts`. See [Playwright CLI](https://playwright.dev/docs/test-cli) for more details.

It is highly recommended to use `npm run e2e:docker` (instead of `npm run e2e`) as CI also uses the same environment. You can also use `npm run e2e:update-snapshots` to regenerate snapshots in said environment.
It is highly recommended to use `npm run e2e:docker` (instead of `npm run e2e`) as CI also uses the same environment. You can also use `npm run e2e:update-snapshots` to regenerate snapshots in said environment. Run Playwright in [UI Mode](https://playwright.dev/docs/test-ui-mode) with `npm run e2e:ui` when creating new tests or debugging, as this will allow you to run each test individually, see the browser as it runs it, inspect the console, evaluate locators, etc.

### Running Python tests

Expand Down Expand Up @@ -229,6 +230,6 @@ After you have successfully run `tools/release.sh` once, you should be able to d

As part of the release process, `cog` will, per our `cog.toml` configuration, invoke `tools/update_version.sh <packageName> <newVersion>`, which is a script that uses `sed` to update a plugin's version number in whatever source file we happen to use as the source of truth for version information in the given plugin.
*[WARNING]* If you change where the source of truth for a plugin's version is located, you must update `tools/update_version.sh` to update the correct file with a new version number.
_[WARNING]_ If you change where the source of truth for a plugin's version is located, you must update `tools/update_version.sh` to update the correct file with a new version number.

We use `tools/update_version.sh` to remove any `.dev0` "developer version" suffix before creating a release, and to put the `.dev0` version suffix back after completing the release.
17,059 changes: 3,830 additions & 13,229 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
"test:ci": "run-p test:ci:*",
"test:ci:unit": "jest --config jest.config.unit.cjs --ci --cacheDirectory $PWD/.jest-cache",
"test:ci:lint": "jest --config jest.config.lint.cjs --ci --cacheDirectory $PWD/.jest-cache",
"e2e:docker": "./tools/run_docker.sh e2e-tests",
"e2e": "playwright test",
"e2e:ui": "playwright test --ui",
"e2e:docker": "DEEPHAVEN_PORT=10001 ./tools/run_docker.sh e2e-tests",
"e2e:update-snapshots": "./tools/run_docker.sh update-snapshots",
"update-dh-packages": "lerna run --concurrency 1 update-dh-packages",
"update-dh-packages:ui": "npm run update-dh-packages -- --scope=@deephaven/js-plugin-ui --"
Expand Down
37 changes: 35 additions & 2 deletions plugins/ui/src/deephaven/ui/object_types/ElementMessageStream.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from jsonrpc import JSONRPCResponseManager, Dispatcher
import logging
import threading
import traceback
from enum import Enum
from queue import Queue
from typing import Any, Callable
Expand All @@ -19,6 +20,7 @@
from ..elements import Element
from ..renderer import NodeEncoder, Renderer, RenderedNode
from .._internal import RenderContext, StateUpdateCallable, ExportedRenderState
from .ErrorCode import ErrorCode

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -166,8 +168,13 @@ def _render(self) -> None:
state = self._context.export_state()
self._send_document_update(node, state)
except Exception as e:
logger.exception("Error rendering %s", self._element.name)
raise e
# Send the error to the client for displaying to the user
# If there's an error sending it to the client, then it will be caught by the render exception handler
# and logged as an error message.
# Just log it as debug here so we don't show it in the console and in the error panel.
stack_trace = traceback.format_exc()
logging.debug("Error rendering document: %s %s", repr(e), stack_trace)
self._send_document_error(e, stack_trace)

def _process_callable_queue(self) -> None:
"""
Expand Down Expand Up @@ -199,7 +206,11 @@ def _process_callable_queue(self) -> None:
else:
self._render_state = _RenderState.IDLE
except Exception as e:
# Something catastrophic happened, log it and close the connection
# We're just being safe to make sure there is an error logged if something unexpected does occur,
# as `submit_task` does not log any uncaught exceptions currently: https://github.com/deephaven/deephaven-core/issues/5192
logger.exception(e)
self._connection.on_close()

def _mark_dirty(self) -> None:
"""
Expand Down Expand Up @@ -366,3 +377,25 @@ def _send_document_update(
del self._context
sys.exit()
self._connection.on_data(payload.encode(), new_objects)

def _send_document_error(self, error: Exception, stack_trace: str) -> None:
"""
Send an error to the client. This is called when an error occurs during rendering.
Args:
error: The error that occurred
stack_trace: The stack trace of the error
"""
request = self._make_notification(
"documentError",
json.dumps(
{
"message": str(error),
"type": type(error).__name__,
"stack": stack_trace,
"code": ErrorCode.DOCUMENT_ERROR.value,
}
),
)
payload = json.dumps(request)
self._connection.on_data(payload.encode(), [])
18 changes: 18 additions & 0 deletions plugins/ui/src/deephaven/ui/object_types/ErrorCode.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from enum import Enum


class ErrorCode(int, Enum):
"""
ErrorCode is a list of error codes that can be returned by the server.
"""

# General errors
UNKNOWN = 0
"""
An unknown error occurred on the server.
"""

DOCUMENT_ERROR = 1
"""
There was an error when rendering the document.
"""
5 changes: 3 additions & 2 deletions plugins/ui/src/js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"react-dom": "^17.0.2"
},
"dependencies": {
"@adobe/react-spectrum": "3.33.1",
"@adobe/react-spectrum": "^3.34.1",
"@deephaven/chart": "^0.76.0",
"@deephaven/components": "^0.76.0",
"@deephaven/dashboard": "^0.76.0",
Expand All @@ -51,14 +51,15 @@
"@deephaven/iris-grid": "^0.76.0",
"@deephaven/jsapi-bootstrap": "^0.76.0",
"@deephaven/jsapi-components": "^0.76.0",
"@deephaven/jsapi-types": "^1.0.0-dev0.34.0",
"@deephaven/jsapi-types": "^1.0.0-dev0.33.3",
"@deephaven/log": "^0.76.0",
"@deephaven/plugin": "^0.76.0",
"@deephaven/react-hooks": "^0.76.0",
"@deephaven/redux": "^0.76.0",
"@deephaven/utils": "^0.76.0",
"@fortawesome/react-fontawesome": "^0.2.0",
"@react-types/shared": "^3.22.0",
"classnames": "^2.5.1",
"json-rpc-2.0": "^1.6.0",
"react-redux": "^7.x",
"shortid": "^2.2.16"
Expand Down
2 changes: 1 addition & 1 deletion plugins/ui/src/js/src/DashboardPlugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function DashboardPlugin(
widgetId: string;
widget: WidgetDescriptor;
}) => {
log.info('Opening widget with ID', widgetId, widget);
log.debug('Opening widget with ID', widgetId, widget);
setWidgetMap(prevWidgetMap => {
const newWidgetMap = new Map(prevWidgetMap);
const oldWidget = newWidgetMap.get(widgetId);
Expand Down
2 changes: 2 additions & 0 deletions plugins/ui/src/js/src/layout/ReactPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { ReactPanelProps } from './LayoutUtils';
import { useParentItem } from './ParentItemContext';
import { ReactPanelContext } from './ReactPanelContext';
import { usePortalPanelManager } from './PortalPanelManagerContext';
import ReactPanelContentOverlay from './ReactPanelContentOverlay';

const log = Log.module('@deephaven/js-plugin-ui/ReactPanel');

Expand Down Expand Up @@ -201,6 +202,7 @@ function ReactPanel({
{children}
</Flex>
</View>
<ReactPanelContentOverlay />
</ReactPanelContext.Provider>,
portal,
contentKey
Expand Down
12 changes: 12 additions & 0 deletions plugins/ui/src/js/src/layout/ReactPanelContentOverlay.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import React from 'react';
import { usePanelContentOverlay } from './usePanelContentOverlay';

/** A panel that uses the ReactPanelContentOverlayContext and if that content is set, renders it in a view with a partially transparent background */
export function ReactPanelContentOverlay(): JSX.Element | null {
const overlayContent = usePanelContentOverlay();
return overlayContent != null ? (
<div className="dh-react-panel-overlay">{overlayContent}</div>
) : null;
}

export default ReactPanelContentOverlay;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { createContext } from 'react';

/** Context that defined a ReactNode to overlay on top of the content in a ReactPanel */
export const ReactPanelContentOverlayContext =
createContext<React.ReactNode | null>(null);

export default ReactPanelContentOverlayContext;
12 changes: 12 additions & 0 deletions plugins/ui/src/js/src/layout/usePanelContentOverlay.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { useContext } from 'react';
import { ReactPanelContentOverlayContext } from './ReactPanelContentOverlayContext';

/**
* Gets the overlay content from the nearest panel context.
* @returns The overlay content or null if not in a panel
*/
export function usePanelContentOverlay(): React.ReactNode | null {
return useContext(ReactPanelContentOverlayContext);
}

export default usePanelContentOverlay;
50 changes: 50 additions & 0 deletions plugins/ui/src/js/src/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,53 @@
}
}
}

.ui-widget-error-view {
display: flex;
flex-direction: column;
gap: $spacer-1;
flex-grow: 1;
max-height: 100%;

.widget-error-view-content {
position: relative;
flex-shrink: 1;
overflow: hidden;
display: flex;
flex-direction: column;
flex-grow: 1;
}

.widget-error-view-footer {
display: flex;
justify-content: flex-end;
align-items: center;
gap: $spacer-1;
flex-wrap: wrap;
flex-shrink: 0;
}

.error-view {
flex-grow: 1;
}
}

.dh-react-panel-overlay {
background-color: bg-opacity(80);
backdrop-filter: blur(5px);
position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
padding: $spacer;
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
z-index: 1000;

.ui-widget-error-view {
max-width: 100%;
}
}
2 changes: 1 addition & 1 deletion plugins/ui/src/js/src/widget/DashboardWidgetHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import WidgetHandler from './WidgetHandler';
const log = Log.module('@deephaven/js-plugin-ui/DashboardWidgetHandler');

export interface DashboardWidgetHandlerProps {
/** ID of this widget */
/** ID of this widget instance */
id: WidgetId;

/** Widget for this to handle */
Expand Down
31 changes: 31 additions & 0 deletions plugins/ui/src/js/src/widget/WidgetErrorView.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from 'react';
import { Button, ErrorView } from '@deephaven/components';
import { vsRefresh } from '@deephaven/icons';
import { WidgetError } from './WidgetTypes';

/** Component that takes a WidgetError and displays the contents in an ErrorView, and has a button to reload the widget from a fresh state. */
function WidgetErrorView({
error,
onReload: onReset,
}: {
error: WidgetError;
onReload: () => void;
}): JSX.Element {
const displayMessage = `${error.message.trim()}\n\n${
error.stack ?? ''
}`.trim();
return (
<div className="ui-widget-error-view">
<div className="widget-error-view-content">
<ErrorView message={displayMessage} type={error.type} isExpanded />
</div>
<div className="widget-error-view-footer">
<Button kind="tertiary" icon={vsRefresh} onClick={onReset}>
Reload
</Button>
</div>
</div>
);
}

export default WidgetErrorView;
Loading

0 comments on commit b23b571

Please sign in to comment.