fix: Handle undefined DashboardData props (#1726)

- Handle undefined props on DashboardData. Don't render console panel if
no session
- Fixed a set state on unmounted HeapUsage bug
- Removed chatty async interval debug logging

fixes #1684

**Testing**
- Run server with PSK enabled
- Login and open any file
- Logout should no longer show "ConsolePanel.tsx:372 Uncaught TypeError:
Cannot destructure property 'config' of 'sessionWrapper' as it is
undefined." error in debug console (Note that login will still fail
until #1685 is done)

Author: bmingles

packages/code-studio/src/main/AppMainContainer.tsx

@@ -123,7 +123,7 @@ interface AppMainContainerProps {
   match: {
     params: { notebookPath: string };
   };
-  connection: IdeConnection;
+  connection?: IdeConnection;
   session?: IdeSession;
   sessionConfig?: SessionConfig;
   setActiveTool: (tool: string) => void;
@@ -255,6 +255,10 @@ export class AppMainContainer extends Component<
 
   initWidgets(): void {
     const { connection } = this.props;
+    if (connection == null) {
+      return;
+    }
+
     if (connection.subscribeToFieldUpdates == null) {
       log.warn(
         'subscribeToFieldUpdates not supported, not initializing widgets'
@@ -614,6 +618,10 @@ export class AppMainContainer extends Component<
 
   startListeningForDisconnect(): void {
     const { connection } = this.props;
+    if (connection == null) {
+      return;
+    }
+
     connection.addEventListener(
       dh.IdeConnection.EVENT_DISCONNECT,
       this.handleDisconnect
@@ -630,6 +638,10 @@ export class AppMainContainer extends Component<
 
   stopListeningForDisconnect(): void {
     const { connection } = this.props;
+    if (connection == null) {
+      return;
+    }
+
     connection.removeEventListener(
       dh.IdeConnection.EVENT_DISCONNECT,
       this.handleDisconnect
@@ -650,6 +662,7 @@ export class AppMainContainer extends Component<
   ): DehydratedDashboardPanelProps & { fetch?: () => Promise<unknown> } {
     const { connection } = this.props;
     const { metadata } = props;
+
     if (
       metadata?.type != null &&
       (metadata?.id != null || metadata?.name != null)
@@ -666,12 +679,14 @@ export class AppMainContainer extends Component<
               name: metadata.name,
               title: metadata.name,
             };
+
       return {
-        fetch: () => connection.getObject(widget),
+        fetch: async () => connection?.getObject(widget),
         ...props,
         localDashboardId: id,
       };
     }
+
     return DashboardUtils.hydrate(props, id);
   }
 
@@ -684,7 +699,7 @@ export class AppMainContainer extends Component<
     const { connection } = this.props;
     this.emitLayoutEvent(PanelEvent.OPEN, {
       dragEvent,
-      fetch: () => connection.getObject(widget),
+      fetch: async () => connection?.getObject(widget),
       widget,
     });
   }

packages/console/src/HeapUsage.tsx

@@ -4,7 +4,7 @@ import { Tooltip } from '@deephaven/components';
 import type { QueryConnectable } from '@deephaven/jsapi-types';
 import { Plot, useChartTheme } from '@deephaven/chart';
 import Log from '@deephaven/log';
-import { useAsyncInterval } from '@deephaven/react-hooks';
+import { useAsyncInterval, useIsMountedRef } from '@deephaven/react-hooks';
 import './HeapUsage.scss';
 
 const log = Log.module('HeapUsage');
@@ -41,9 +41,16 @@ function HeapUsage({
     usages: [],
   });
 
+  const isMountedRef = useIsMountedRef();
+
   const setUsageUpdateInterval = useCallback(async () => {
     try {
       const newUsage = await connection.getWorkerHeapInfo();
+
+      if (!isMountedRef.current) {
+        return;
+      }
+
       setMemoryUsage(newUsage);
 
       if (bgMonitoring || isOpen) {
@@ -69,7 +76,7 @@ function HeapUsage({
     } catch (e) {
       log.warn('Unable to get heap usage', e);
     }
-  }, [isOpen, connection, monitorDuration, bgMonitoring]);
+  }, [connection, isMountedRef, bgMonitoring, isOpen, monitorDuration]);
 
   useAsyncInterval(
     setUsageUpdateInterval,

packages/dashboard-core-plugins/src/panels/ConsolePanel.test.tsx

@@ -1,20 +1,24 @@
 import React from 'react';
 import { render } from '@testing-library/react';
 import { CommandHistoryStorage } from '@deephaven/console';
-import type { Container } from '@deephaven/golden-layout';
+import type { Container, EventEmitter } from '@deephaven/golden-layout';
 import type { IdeConnection, IdeSession } from '@deephaven/jsapi-types';
+import { dh } from '@deephaven/jsapi-shim';
 import { SessionConfig, SessionWrapper } from '@deephaven/jsapi-utils';
+import { TestUtils } from '@deephaven/utils';
 import { ConsolePanel } from './ConsolePanel';
 
-const mockConsole = jest.fn(() => null);
+type IdeSessionConstructor = new (language: string) => IdeSession;
+
+const mockConsole = jest.fn((_props: unknown) => null);
 jest.mock('@deephaven/console', () => ({
   ...(jest.requireActual('@deephaven/console') as Record<string, unknown>),
   Console: props => mockConsole(props),
   default: props => mockConsole(props),
 }));
 
 function makeSession(language = 'TEST_LANG'): IdeSession {
-  return new dh.IdeSession(language) as unknown as IdeSession;
+  return new (dh.IdeSession as unknown as IdeSessionConstructor)(language);
 }
 
 function makeConnection({
@@ -42,31 +46,15 @@ function makeSessionWrapper({
   return { session, connection, config, dh };
 }
 
-function makeEventHub() {
-  return {
-    emit: jest.fn(),
-    on: jest.fn(),
-    off: jest.fn(),
-    trigger: jest.fn(),
-    unbind: jest.fn(),
-  };
-}
-
-function makeGlContainer(): Container {
-  return {
-    emit: jest.fn(),
-    on: jest.fn(),
-    off: jest.fn(),
-  } as unknown as Container;
-}
-
 function makeCommandHistoryStorage(): CommandHistoryStorage {
   return {} as CommandHistoryStorage;
 }
 
 function renderConsolePanel({
-  eventHub = makeEventHub(),
-  container = makeGlContainer(),
+  eventHub = TestUtils.createMockProxy<EventEmitter>(),
+  container = TestUtils.createMockProxy<Container>({
+    tab: undefined,
+  }),
   commandHistoryStorage = makeCommandHistoryStorage(),
   timeZone = 'MockTimeZone',
   sessionWrapper = makeSessionWrapper(),
@@ -78,11 +66,18 @@ function renderConsolePanel({
       commandHistoryStorage={commandHistoryStorage}
       timeZone={timeZone}
       sessionWrapper={sessionWrapper}
+      localDashboardId="mock-localDashboardId"
+      plugins={new Map()}
     />
   );
 }
 
 beforeEach(() => {
+  // Mocking the Console component causes it to be treated as a functional
+  // component which causes React to log an error about passing refs. Disable
+  // logging to supress this
+  TestUtils.disableConsoleOutput('error');
+
   mockConsole.mockClear();
 });
 

packages/dashboard-core-plugins/src/panels/ConsolePanel.tsx

@@ -4,6 +4,7 @@ import React, { PureComponent, ReactElement, RefObject } from 'react';
 import shortid from 'shortid';
 import debounce from 'lodash.debounce';
 import { connect } from 'react-redux';
+import { LoadingOverlay } from '@deephaven/components';
 import {
   CommandHistoryStorage,
   Console,
@@ -63,7 +64,7 @@ interface ConsolePanelProps extends DashboardPanelProps {
 
   panelState?: PanelState;
 
-  sessionWrapper: SessionWrapper;
+  sessionWrapper?: SessionWrapper;
 
   timeZone: string;
   unzip?: (file: File) => Promise<JSZipObject[]>;
@@ -159,6 +160,10 @@ export class ConsolePanel extends PureComponent<
 
   subscribeToFieldUpdates(): void {
     const { sessionWrapper } = this.props;
+    if (sessionWrapper == null) {
+      return;
+    }
+
     const { session } = sessionWrapper;
 
     this.objectSubscriptionCleanup = session.subscribeToFieldUpdates(
@@ -244,6 +249,9 @@ export class ConsolePanel extends PureComponent<
 
   handleOpenObject(object: VariableDefinition, forceOpen = true): void {
     const { sessionWrapper } = this.props;
+    if (sessionWrapper == null) {
+      return;
+    }
     const { session } = sessionWrapper;
     const { root } = this.context;
     const oldPanelId =
@@ -359,7 +367,7 @@ export class ConsolePanel extends PureComponent<
     return <ObjectIcon type={type} />;
   }
 
-  render(): ReactElement {
+  render(): ReactElement | null {
     const {
       commandHistoryStorage,
       glContainer,
@@ -368,6 +376,13 @@ export class ConsolePanel extends PureComponent<
       timeZone,
       unzip,
     } = this.props;
+
+    if (sessionWrapper == null) {
+      return (
+        <LoadingOverlay isLoading={false} errorMessage="Console is disabled." />
+      );
+    }
+
     const { consoleSettings, error, objectMap } = this.state;
     const { config, session, connection, details = {}, dh } = sessionWrapper;
     const { workerName, processInfoId } = details;

packages/dashboard-core-plugins/src/panels/FileExplorerPanel.tsx

@@ -21,7 +21,7 @@ const log = Log.module('FileExplorerPanel');
 
 type StateProps = {
   fileStorage: FileStorage;
-  language: string;
+  language?: string;
   session?: IdeSession;
 };
 

packages/dashboard-core-plugins/src/panels/LogPanel.tsx

@@ -14,11 +14,11 @@ import { getDashboardSessionWrapper } from '../redux';
 const log = Log.module('LogPanel');
 
 interface LogPanelProps extends DashboardPanelProps {
-  session: IdeSession;
+  session?: IdeSession;
 }
 
 interface LogPanelState {
-  session: IdeSession;
+  session?: IdeSession;
 }
 
 class LogPanel extends PureComponent<LogPanelProps, LogPanelState> {

packages/dashboard-core-plugins/src/panels/NotebookPanel.tsx

@@ -64,7 +64,7 @@ interface Metadata extends PanelMetadata {
   id: string;
 }
 interface NotebookSetting {
-  isMinimapEnabled: boolean;
+  isMinimapEnabled?: boolean;
 }
 
 interface FileMetadata {
@@ -78,16 +78,21 @@ interface PanelState {
   fileMetadata: FileMetadata | null;
 }
 
-interface NotebookPanelProps extends DashboardPanelProps {
+interface NotebookPanelMappedProps {
+  defaultNotebookSettings: NotebookSetting;
   fileStorage: FileStorage;
+  session?: IdeSession;
+  sessionLanguage?: string;
+}
+
+interface NotebookPanelProps
+  extends DashboardPanelProps,
+    NotebookPanelMappedProps {
   isDashboardActive: boolean;
   isPreview: boolean;
   metadata: Metadata;
-  session: IdeSession;
-  sessionLanguage: string;
   panelState: PanelState;
   notebooksUrl: string;
-  defaultNotebookSettings: NotebookSetting;
   updateSettings: (settings: Partial<WorkspaceSettings>) => void;
 }
 
@@ -790,7 +795,7 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
     const { defaultNotebookSettings, updateSettings } = this.props;
     const newSettings = {
       defaultNotebookSettings: {
-        isMinimapEnabled: !defaultNotebookSettings.isMinimapEnabled,
+        isMinimapEnabled: !(defaultNotebookSettings.isMinimapEnabled ?? false),
       },
     };
     updateSettings(newSettings);
@@ -1176,10 +1181,10 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
     const { defaultNotebookSettings } = this.props;
     const { settings: initialSettings } = this.state;
     return this.getOverflowActions(
-      defaultNotebookSettings.isMinimapEnabled,
+      defaultNotebookSettings.isMinimapEnabled ?? false,
       this.getSettings(
         initialSettings,
-        defaultNotebookSettings.isMinimapEnabled
+        defaultNotebookSettings.isMinimapEnabled ?? false
       ).wordWrap === 'on'
     );
   }
@@ -1216,7 +1221,7 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
     const isExistingItem = fileMetadata?.id != null;
     const settings = this.getSettings(
       initialSettings,
-      defaultNotebookSettings.isMinimapEnabled
+      defaultNotebookSettings.isMinimapEnabled ?? false
     );
     const isSessionConnected = session != null;
     const isLanguageMatching = sessionLanguage === settings.language;
@@ -1428,10 +1433,7 @@ class NotebookPanel extends Component<NotebookPanelProps, NotebookPanelState> {
 const mapStateToProps = (
   state: RootState,
   ownProps: { localDashboardId: string }
-): Pick<
-  NotebookPanelProps,
-  'defaultNotebookSettings' | 'fileStorage' | 'session' | 'sessionLanguage'
-> => {
+): NotebookPanelMappedProps => {
   const fileStorage = getFileStorage(state);
   const defaultNotebookSettings = getDefaultNotebookSettings(state);
   const sessionWrapper = getDashboardSessionWrapper(
@@ -1443,7 +1445,7 @@ const mapStateToProps = (
   const { type: sessionLanguage } = sessionConfig ?? {};
   return {
     fileStorage,
-    defaultNotebookSettings: defaultNotebookSettings as NotebookSetting,
+    defaultNotebookSettings,
     session,
     sessionLanguage,
   };