Fix xterm dimensions error with explicit checks instead of try-catch (#12095)

Co-authored-by: openhands <openhands@all-hands.dev>

frontend/__tests__/hooks/use-terminal.test.tsx

@@ -1,4 +1,3 @@
-/* eslint-disable max-classes-per-file */
 import { beforeAll, describe, expect, it, vi, afterEach } from "vitest";
 import { useTerminal } from "#/hooks/use-terminal";
 import { Command, useCommandStore } from "#/stores/command-store";
@@ -43,6 +42,11 @@ describe("useTerminal", () => {
     write: vi.fn(),
     writeln: vi.fn(),
     dispose: vi.fn(),
+    element: document.createElement("div"),
+  }));
+
+  const mockFitAddon = vi.hoisted(() => ({
+    fit: vi.fn(),
   }));
 
   beforeAll(() => {
@@ -68,6 +72,15 @@ describe("useTerminal", () => {
         writeln = mockTerminal.writeln;
 
         dispose = mockTerminal.dispose;
+
+        element = mockTerminal.element;
+      },
+    }));
+
+    // mock FitAddon
+    vi.mock("@xterm/addon-fit", () => ({
+      FitAddon: class {
+        fit = mockFitAddon.fit;
       },
     }));
   });
@@ -96,4 +109,18 @@ describe("useTerminal", () => {
     expect(mockTerminal.writeln).toHaveBeenNthCalledWith(1, "echo hello");
     expect(mockTerminal.writeln).toHaveBeenNthCalledWith(2, "hello");
   });
+
+  it("should not call fit() when terminal.element is null", () => {
+    // Temporarily set element to null to simulate terminal not being opened
+    const originalElement = mockTerminal.element;
+    mockTerminal.element = null as unknown as HTMLDivElement;
+
+    renderWithProviders(<TestTerminalComponent />);
+
+    // fit() should not be called because terminal.element is null
+    expect(mockFitAddon.fit).not.toHaveBeenCalled();
+
+    // Restore original element
+    mockTerminal.element = originalElement;
+  });
 });

frontend/src/hooks/use-terminal.ts

@@ -29,6 +29,47 @@ const renderCommand = (
   }
 };
 
+/**
+ * Check if the terminal is ready for fit operations.
+ * This prevents the "Cannot read properties of undefined (reading 'dimensions')" error
+ * that occurs when fit() is called on a terminal that is hidden, disposed, or not fully initialized.
+ */
+const canFitTerminal = (
+  terminalInstance: Terminal | null,
+  fitAddonInstance: FitAddon | null,
+  containerElement: HTMLDivElement | null,
+): boolean => {
+  // Check terminal and fitAddon exist
+  if (!terminalInstance || !fitAddonInstance) {
+    return false;
+  }
+
+  // Check container element exists
+  if (!containerElement) {
+    return false;
+  }
+
+  // Check element is visible (not display: none)
+  // When display is none, offsetParent is null (except for fixed/body elements)
+  const computedStyle = window.getComputedStyle(containerElement);
+  if (computedStyle.display === "none") {
+    return false;
+  }
+
+  // Check element has dimensions
+  const { clientWidth, clientHeight } = containerElement;
+  if (clientWidth === 0 || clientHeight === 0) {
+    return false;
+  }
+
+  // Check terminal has been opened (element property is set after open())
+  if (!terminalInstance.element) {
+    return false;
+  }
+
+  return true;
+};
+
 // Create a persistent reference that survives component unmounts
 // This ensures terminal history is preserved when navigating away and back
 const persistentLastCommandIndex = { current: 0 };
@@ -39,6 +80,7 @@ export const useTerminal = () => {
   const fitAddon = React.useRef<FitAddon | null>(null);
   const ref = React.useRef<HTMLDivElement>(null);
   const lastCommandIndex = persistentLastCommandIndex; // Use the persistent reference
+  const isDisposed = React.useRef(false);
 
   const createTerminal = () =>
     new Terminal({
@@ -55,6 +97,15 @@ export const useTerminal = () => {
       },
     });
 
+  const fitTerminalSafely = React.useCallback(() => {
+    if (isDisposed.current) {
+      return;
+    }
+    if (canFitTerminal(terminal.current, fitAddon.current, ref.current)) {
+      fitAddon.current!.fit();
+    }
+  }, []);
+
   const initializeTerminal = () => {
     if (terminal.current) {
       if (fitAddon.current) terminal.current.loadAddon(fitAddon.current);
@@ -62,13 +113,14 @@ export const useTerminal = () => {
         terminal.current.open(ref.current);
         // Hide cursor for read-only terminal using ANSI escape sequence
         terminal.current.write("\x1b[?25l");
-        fitAddon.current?.fit();
+        fitTerminalSafely();
       }
     }
   };
 
   // Initialize terminal and handle cleanup
   React.useEffect(() => {
+    isDisposed.current = false;
     terminal.current = createTerminal();
     fitAddon.current = new FitAddon();
 
@@ -91,6 +143,7 @@ export const useTerminal = () => {
     }
 
     return () => {
+      isDisposed.current = true;
       terminal.current?.dispose();
       lastCommandIndex.current = 0;
     };
@@ -118,7 +171,10 @@ export const useTerminal = () => {
     let resizeObserver: ResizeObserver | null = null;
 
     resizeObserver = new ResizeObserver(() => {
-      fitAddon.current?.fit();
+      // Use requestAnimationFrame to debounce resize events and ensure DOM is ready
+      requestAnimationFrame(() => {
+        fitTerminalSafely();
+      });
     });
 
     if (ref.current) {
@@ -128,7 +184,7 @@ export const useTerminal = () => {
     return () => {
       resizeObserver?.disconnect();
     };
-  }, []);
+  }, [fitTerminalSafely]);
 
   return ref;
 };