Repository: deephaven/deephaven-plugins (language: python)

Issue:
Deephaven UI can blackscreen/crash the app when fixing a bad component
**Description**

I was running some example code in a PR that was missing `ui.use_state` and just used `use_state` instead. The component panel rendered w/ the error view. I fixed the code and re-ran the component and it blackscreened the app.

**Steps to reproduce**

1. Run this code that omits the `ui.` prefix for `use_state`
```py
from deephaven import ui
@ui.component
def my_comp():
    state, set_state = use_state(4)
    return state

c = my_comp()
```

2. Run this corrected code with the errored panel still open
```py
from deephaven import ui
@ui.component
def my_comp():
    state, set_state = ui.use_state(4)
    return state

c = my_comp()
```

**Expected results**

Panel with the content 4 opens.

**Actual results**

App blackscreens.

**Additional details and attachments**

This only seems to happen if the first time a panel/component was rendered it has an error rendering. If you run correct, errored, correct then it does not cause the blackscreen.

```
Error: Widget not found: Qvz7kWgfnT2yW8VLxIL2W
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:7530:17)
    at jd (react-dom.production.min.js:158:387)
    at G3 (react-dom.production.min.js:159:447)
    at Object.useState (react-dom.production.min.js:174:261)
    at zi.useState (react.production.min.js:23:281)
    at DashboardPlugin (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:7371:43)
    at Jae (react-dom.production.min.js:157:137)
    at LT (react-dom.production.min.js:180:154)
    at _ye (react-dom.production.min.js:269:343)
    at Eye (react-dom.production.min.js:250:347)
    at rYe (react-dom.production.min.js:250:278)
    at Nb (react-dom.production.min.js:250:138)
    at ZT (react-dom.production.min.js:243:163)
    at react-dom.production.min.js:123:115
    at e.unstable_runWithPriority (scheduler.production.min.js:18:343)
    at gv (react-dom.production.min.js:122:325)
    at Ybe (react-dom.production.min.js:123:61)
    at rp (react-dom.production.min.js:122:428)
    at om (react-dom.production.min.js:237:203)
    at roe (react-dom.production.min.js:170:408)
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:7526:7)
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:7345:39)
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:7198:15)
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5349:24)
    at callMethod (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5454:18)
    at noopMiddleware (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5486:10)
    at JSONRPCServer2.callMethod (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5466:51)
    at JSONRPCServer2.eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5415:31)
    at step (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5289:19)
    at Object.eval [as next] (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5236:14)
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5222:67)
    at new Promise (<anonymous>)
    at __awaiter$1 (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5204:10)
    at JSONRPCServer2.receiveSingle (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5405:14)
    at JSONRPCServer2.receive (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5378:21)
    at JSONRPCServerAndClient2.eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5682:38)
    at step (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5607:19)
    at Object.eval [as next] (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5554:14)
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5540:67)
    at new Promise (<anonymous>)
    at __awaiter (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5522:10)
    at JSONRPCServerAndClient2.receiveAndSend (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:5670:14)
    at receiveData (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:7233:32)
    at eval (eval at <anonymous> (loadRemoteModule.js:21:24), <anonymous>:7245:11)
    at Object.$lambda$3 (dh-core.js:8217:5)
    at Function.onInvoke_12 (dh-core.js:8935:16)
    at lambda (dh-core.js:174:22)
    at Array.forEach (<anonymous>)
    at Object.$fireEvent_0 (dh-core.js:8193:15)
    at Object.$lambda$6_5 (dh-core.js:28739:11)
    at Function.apply_170 (dh-core.js:28966:10)
    at lambda (dh-core.js:174:22)
    at dh-internal.js:1:792286
    at Array.forEach (<anonymous>)
    at dh-internal.js:1:792265
    at dh-internal.js:1:43529
    at Array.forEach (<anonymous>)
    at e.rawOnMessage (dh-internal.js:1:43491)
    at dh-internal.js:1:41304
    at Array.forEach (<anonymous>)
    at e.onTransportChunk (dh-internal.js:1:41179)
    at Object.$onMessage (dh-core.js:21090:35)
    at MultiplexedWebsocketTransport$3methodref$onMessage$Type.handleEvent_2 [as handleEvent] (dh-core.js:21234:10)
```

**Versions**

Engine Version: 0.35.0-SNAPSHOT
Web UI Version: 0.79.0
Java Version: 21.0.3
Barrage Version: 0.6.0
Browser Name: Chrome 125
OS Name: Linux
@deephaven/js-plugin-matplotlib: 0.4.1
@deephaven/js-plugin-ui: 0.14.0
@deephaven/js-plugin-plotly-express: 0.8.0



Discussion / hints:
Will need this fix to be in 0.15.x

Fix patch:
diff --git a/plugins/ui/src/js/src/errors/MixedPanelsError.ts b/plugins/ui/src/js/src/errors/MixedPanelsError.ts
index f1f48a332..1202e46d3 100644
--- a/plugins/ui/src/js/src/errors/MixedPanelsError.ts
+++ b/plugins/ui/src/js/src/errors/MixedPanelsError.ts
@@ -1,4 +1,6 @@
 export class MixedPanelsError extends Error {
+  name = 'MixedPanelsError';
+
   isMixedPanelsError = true;
 }
 
diff --git a/plugins/ui/src/js/src/errors/NoChildrenError.ts b/plugins/ui/src/js/src/errors/NoChildrenError.ts
index ef66f2718..6d0c7a906 100644
--- a/plugins/ui/src/js/src/errors/NoChildrenError.ts
+++ b/plugins/ui/src/js/src/errors/NoChildrenError.ts
@@ -1,4 +1,6 @@
 export class NoChildrenError extends Error {
+  name = 'NoChildrenError';
+
   isNoChildrenError = true;
 }
 
diff --git a/plugins/ui/src/js/src/widget/DocumentHandler.tsx b/plugins/ui/src/js/src/widget/DocumentHandler.tsx
index 7d4f3c400..db30efd14 100644
--- a/plugins/ui/src/js/src/widget/DocumentHandler.tsx
+++ b/plugins/ui/src/js/src/widget/DocumentHandler.tsx
@@ -1,4 +1,10 @@
-import React, { useCallback, useMemo, useRef, useState } from 'react';
+import React, {
+  useCallback,
+  useEffect,
+  useMemo,
+  useRef,
+  useState,
+} from 'react';
 import { nanoid } from 'nanoid';
 import { WidgetDescriptor } from '@deephaven/dashboard';
 import Log from '@deephaven/log';
@@ -46,7 +52,6 @@ function DocumentHandler({
   onClose,
 }: DocumentHandlerProps): JSX.Element {
   log.debug('Rendering document', widget);
-  const panelOpenCountRef = useRef(0);
   const panelIdIndex = useRef(0);
 
   // Using `useState` here to initialize the data only once.
@@ -57,43 +62,69 @@ function DocumentHandler({
 
   // panelIds that are currently opened within this document. This list is tracked by the `onOpen`/`onClose` call on the `ReactPanelManager` from a child component.
   // Note that the initial widget data provided will be the `panelIds` for this document to use; this array is what is actually opened currently.
-  const [panelIds] = useState<string[]>([]);
+  const panelIds = useRef<string[]>([]);
+
+  // Flag to signal the panel counts have changed in the last render
+  // We may need to check if we need to close this widget if all panels are closed
+  const [isPanelsDirty, setPanelsDirty] = useState(false);
 
   const handleOpen = useCallback(
     (panelId: string) => {
-      if (panelIds.includes(panelId)) {
+      if (panelIds.current.includes(panelId)) {
         throw new Error('Duplicate panel opens received');
       }
 
-      panelOpenCountRef.current += 1;
-      log.debug('Panel opened, open count', panelOpenCountRef.current);
+      panelIds.current.push(panelId);
+      log.debug('Panel opened, open count', panelIds.current.length);
 
-      panelIds.push(panelId);
-      onDataChange({ ...widgetData, panelIds });
+      setPanelsDirty(true);
     },
-    [onDataChange, panelIds, widgetData]
+    [panelIds]
   );
 
   const handleClose = useCallback(
     (panelId: string) => {
-      const panelIndex = panelIds.indexOf(panelId);
+      const panelIndex = panelIds.current.indexOf(panelId);
       if (panelIndex === -1) {
         throw new Error('Panel close received for unknown panel');
       }
-      panelOpenCountRef.current -= 1;
-      if (panelOpenCountRef.current < 0) {
-        throw new Error('Panel open count is negative');
-      }
-      log.debug('Panel closed, open count', panelOpenCountRef.current);
-      if (panelOpenCountRef.current === 0) {
-        onClose?.();
+
+      panelIds.current.splice(panelIndex, 1);
+      log.debug('Panel closed, open count', panelIds.current.length);
+
+      setPanelsDirty(true);
+    },
+    [panelIds]
+  );
+
+  /**
+   * When there are changes made to panels in a render cycle, check if they've all been closed and fire an `onClose` event if they are.
+   * Otherwise, fire an `onDataChange` event with the updated panelIds that are open.
+   */
+  useEffect(
+    function syncOpenPanels() {
+      if (!isPanelsDirty) {
         return;
       }
 
-      panelIds.splice(panelIndex, 1);
-      onDataChange({ ...widgetData, panelIds });
+      setPanelsDirty(false);
+
+      // Check if all the panels in this widget are closed
+      // We do it outside of the `handleClose` function in case a new panel opens up in the same render cycle
+      log.debug2(
+        'Widget',
+        widget.id,
+        'open panel count',
+        panelIds.current.length
+      );
+      if (panelIds.current.length === 0) {
+        log.debug('Widget', widget.id, 'closed all panels, triggering onClose');
+        onClose?.();
+      } else {
+        onDataChange({ ...widgetData, panelIds: panelIds.current });
+      }
     },
-    [onClose, onDataChange, panelIds, widgetData]
+    [isPanelsDirty, widget.id, onClose, onDataChange, widgetData]
   );
 
   const getPanelId = useCallback(() => {
diff --git a/plugins/ui/src/js/src/widget/WidgetErrorUtils.ts b/plugins/ui/src/js/src/widget/WidgetErrorUtils.ts
new file mode 100644
index 000000000..9d6391a0e
--- /dev/null
+++ b/plugins/ui/src/js/src/widget/WidgetErrorUtils.ts
@@ -0,0 +1,74 @@
+import { WidgetAction, isWidgetError } from './WidgetTypes';
+
+/**
+ * Get the name of an error type
+ * @param error Name of an error
+ * @returns The name of the error
+ */
+export function getErrorName(error: NonNullable<unknown>): string {
+  if (isWidgetError(error)) {
+    return error.name;
+  }
+  const errorType =
+    typeof error === 'object' ? error.constructor.name ?? '' : typeof error;
+  if (errorType.endsWith('Error')) {
+    return errorType;
+  }
+  return 'Unknown error';
+}
+
+/**
+ * Get the message of an error
+ * @param error Error object
+ * @returns The error message
+ */
+export function getErrorMessage(error: NonNullable<unknown>): string {
+  if (
+    typeof error === 'object' &&
+    'message' in error &&
+    typeof error.message === 'string'
+  ) {
+    return error.message.trim();
+  }
+  if (typeof error === 'string') {
+    return error.trim();
+  }
+  return 'Unknown error';
+}
+
+/**
+ * Get the short message of an error. Just the first line of the error message.
+ * @param error Error object
+ * @returns The error short message
+ */
+export function getErrorShortMessage(error: NonNullable<unknown>): string {
+  const message = getErrorMessage(error);
+  const lines = message.split('\n');
+  return lines[0].trim();
+}
+
+/**
+ * Get the stack trace of an error
+ * @param error Error object
+ * @returns The error stack trace
+ */
+export function getErrorStack(error: NonNullable<unknown>): string {
+  if (isWidgetError(error)) {
+    return error.stack ?? '';
+  }
+  return '';
+}
+
+/**
+ * Get the action from an error object if it exists
+ * @param error Error object
+ * @returns The action from the error, if it exists
+ */
+export function getErrorAction(
+  error: NonNullable<unknown>
+): WidgetAction | null {
+  if (isWidgetError(error)) {
+    return error.action ?? null;
+  }
+  return null;
+}
diff --git a/plugins/ui/src/js/src/widget/WidgetErrorView.tsx b/plugins/ui/src/js/src/widget/WidgetErrorView.tsx
index 6cf78847a..2a1634612 100644
--- a/plugins/ui/src/js/src/widget/WidgetErrorView.tsx
+++ b/plugins/ui/src/js/src/widget/WidgetErrorView.tsx
@@ -18,7 +18,7 @@ import {
   getErrorName,
   getErrorShortMessage,
   getErrorStack,
-} from './WidgetUtils';
+} from './WidgetErrorUtils';
 
 /** Component that display an error message. Will automatically show a button for more info and an action button if the error has an Action defined */
 export function WidgetErrorView({
diff --git a/plugins/ui/src/js/src/widget/WidgetUtils.tsx b/plugins/ui/src/js/src/widget/WidgetUtils.tsx
index 97cf89a52..d85db4902 100644
--- a/plugins/ui/src/js/src/widget/WidgetUtils.tsx
+++ b/plugins/ui/src/js/src/widget/WidgetUtils.tsx
@@ -26,7 +26,7 @@ import {
 } from '@deephaven/components';
 import { ValueOf } from '@deephaven/utils';
 import Log from '@deephaven/log';
-import { ReadonlyWidgetData, WidgetAction, isWidgetError } from './WidgetTypes';
+import { ReadonlyWidgetData } from './WidgetTypes';
 import {
   ElementNode,
   ELEMENT_KEY,
@@ -235,62 +235,3 @@ export function wrapCallable(
 
   return callable;
 }
-
-/**
- * Get the name of an error type
- * @param error Name of an error
- * @returns The name of the error
- */
-export function getErrorName(error: unknown): string {
-  if (isWidgetError(error)) {
-    return error.name;
-  }
-  return 'Unknown error';
-}
-
-/**
- * Get the message of an error
- * @param error Error object
- * @returns The error message
- */
-export function getErrorMessage(error: unknown): string {
-  if (isWidgetError(error)) {
-    return error.message.trim();
-  }
-  return 'Unknown error';
-}
-
-/**
- * Get the short message of an error. Just the first line of the error message.
- * @param error Error object
- * @returns The error short message
- */
-export function getErrorShortMessage(error: unknown): string {
-  const message = getErrorMessage(error);
-  const lines = message.split('\n');
-  return lines[0].trim();
-}
-
-/**
- * Get the stack trace of an error
- * @param error Error object
- * @returns The error stack trace
- */
-export function getErrorStack(error: unknown): string {
-  if (isWidgetError(error)) {
-    return error.stack ?? '';
-  }
-  return '';
-}
-
-/**
- * Get the action from an error object if it exists
- * @param error Error object
- * @returns The action from the error, if it exists
- */
-export function getErrorAction(error: unknown): WidgetAction | null {
-  if (isWidgetError(error)) {
-    return error.action ?? null;
-  }
-  return null;
-}


Test patch:
diff --git a/plugins/ui/src/js/src/widget/WidgetErrorUtils.test.ts b/plugins/ui/src/js/src/widget/WidgetErrorUtils.test.ts
new file mode 100644
index 000000000..0f5f302e6
--- /dev/null
+++ b/plugins/ui/src/js/src/widget/WidgetErrorUtils.test.ts
@@ -0,0 +1,109 @@
+import { MixedPanelsError } from '../errors';
+import {
+  getErrorName,
+  getErrorMessage,
+  getErrorShortMessage,
+  getErrorStack,
+  getErrorAction,
+} from './WidgetErrorUtils';
+import { WidgetError } from './WidgetTypes';
+
+const longMessage =
+  'This is a test error message\nWith a really long message\nThat spans multiple lines';
+const testAction = jest.fn();
+const error = new Error('Test error');
+const widgetError = { name: 'WidgetError', message: 'Widget test error' };
+const widgetErrorWithStack: WidgetError = {
+  name: 'WidgetStackError',
+  message: longMessage,
+  stack: 'Test stack',
+};
+const errorWithAction = {
+  name: 'ActionError',
+  message: 'Action test error',
+  action: {
+    title: 'Widget Action',
+    action: testAction,
+  },
+};
+const mixedPanelsError = new MixedPanelsError('Mixed error');
+const emptyObject = {};
+const strError = 'String Error';
+
+describe('getErrorName', () => {
+  it('returns the name of the error', () => {
+    expect(getErrorName(error)).toBe('Error');
+    expect(getErrorName(widgetError)).toBe('WidgetError');
+    expect(getErrorName(widgetErrorWithStack)).toBe('WidgetStackError');
+    expect(getErrorName(errorWithAction)).toBe('ActionError');
+    expect(getErrorName(mixedPanelsError)).toBe('MixedPanelsError');
+  });
+
+  it('returns "Unknown error" for an unknown error', () => {
+    expect(getErrorName(emptyObject)).toBe('Unknown error');
+    expect(getErrorName(strError)).toBe('Unknown error');
+  });
+});
+
+describe('getErrorMessage', () => {
+  it('returns the message of the error', () => {
+    expect(getErrorMessage(error)).toBe('Test error');
+    expect(getErrorMessage(widgetError)).toBe('Widget test error');
+    expect(getErrorMessage(widgetErrorWithStack)).toBe(longMessage);
+    expect(getErrorMessage(errorWithAction)).toBe('Action test error');
+    expect(getErrorMessage(mixedPanelsError)).toBe('Mixed error');
+    expect(getErrorMessage(strError)).toBe('String Error');
+  });
+
+  it('returns "Unknown error" for an unknown error', () => {
+    expect(getErrorMessage(emptyObject)).toBe('Unknown error');
+  });
+});
+
+describe('getErrorShortMessage', () => {
+  it('returns the short message of the error', () => {
+    expect(getErrorShortMessage(error)).toBe('Test error');
+    expect(getErrorShortMessage(widgetError)).toBe('Widget test error');
+    expect(getErrorShortMessage(widgetErrorWithStack)).toBe(
+      'This is a test error message'
+    );
+    expect(getErrorShortMessage(errorWithAction)).toBe('Action test error');
+    expect(getErrorShortMessage(mixedPanelsError)).toBe('Mixed error');
+    expect(getErrorShortMessage(strError)).toBe('String Error');
+  });
+
+  it('returns "Unknown error" for an unknown error', () => {
+    expect(getErrorShortMessage(emptyObject)).toBe('Unknown error');
+  });
+});
+
+describe('getErrorStack', () => {
+  it('returns the stack of the error', () => {
+    expect(getErrorStack(error)).toContain(
+      'Error: Test error\n    at Object.<anonymous>'
+    );
+    expect(getErrorStack(widgetError)).toBe('');
+    expect(getErrorStack(widgetErrorWithStack)).toBe('Test stack');
+    expect(getErrorStack(errorWithAction)).toBe('');
+    expect(getErrorStack(mixedPanelsError)).toContain(
+      'MixedPanelsError: Mixed error\n    at Object.<anonymous>'
+    );
+    expect(getErrorStack(strError)).toBe('');
+    expect(getErrorStack(emptyObject)).toBe('');
+  });
+});
+
+describe('getErrorAction', () => {
+  it('returns the action of the error', () => {
+    expect(getErrorAction(error)).toBeNull();
+    expect(getErrorAction(widgetError)).toBeNull();
+    expect(getErrorAction(widgetErrorWithStack)).toBeNull();
+    expect(getErrorAction(errorWithAction)).toEqual({
+      title: 'Widget Action',
+      action: testAction,
+    });
+    expect(getErrorAction(mixedPanelsError)).toBeNull();
+    expect(getErrorAction(strError)).toBeNull();
+    expect(getErrorAction(emptyObject)).toBeNull();
+  });
+});