fix(sessions): auto-recover + report the chat SessionView render-loop crash

packages/ui/src/features/task-detail/components/TaskLogsPanel.tsx

@@ -3,7 +3,7 @@ import type { Task } from "@posthog/shared/domain-types";
 import { Box, Flex } from "@radix-ui/themes";
 import { useCallback, useEffect } from "react";
 import { BackgroundWrapper } from "../../../primitives/BackgroundWrapper";
-import { ErrorBoundary } from "../../../primitives/ErrorBoundary";
+import { ErrorBoundary } from "../../../shell/ErrorBoundary";
 import { useHostCapabilities } from "../../../shell/useHostCapabilities";
 import { useFolders } from "../../folders/useFolders";
 import { useDraftStore } from "../../message-editor/draftStore";
@@ -162,7 +162,11 @@ export function TaskLogsPanel({ taskId, task, hideInput }: TaskLogsPanelProps) {
     <BackgroundWrapper>
       <Flex direction="column" height="100%" width="100%">
         <Box className="min-h-0 flex-1">
-          <ErrorBoundary name="SessionView" resetKey={taskId}>
+          {/* The chat subtree has historically tripped a transient React #185
+              render loop (see issue #2165) that clears on remount — reported via
+              telemetry (shell wrapper) and auto-recovered so a re-viewed task
+              heals itself instead of stranding the user on the error fallback. */}
+          <ErrorBoundary name="SessionView" resetKey={taskId} autoRecover>
             <SessionView
               events={events}
               taskId={taskId}

packages/ui/src/primitives/ErrorBoundary.test.tsx

@@ -0,0 +1,104 @@
+import { Theme } from "@radix-ui/themes";
+import { render, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import type { ReactNode } from "react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { ErrorBoundary, type ErrorBoundaryProps } from "./ErrorBoundary";
+
+// External control so the child's throwing behaviour can change across the
+// boundary's OWN internal re-render — children don't re-render from a parent
+// while the fallback is showing, so a plain prop wouldn't take effect. This
+// models a transient render loop (React #185) that trips once and then clears
+// on the next mount, exactly the shape issue #2165 describes ("Try again
+// recovers it"). Uses real timers so the actual setTimeout-driven recovery is
+// exercised end to end.
+const control = { shouldThrow: true };
+
+function Flaky() {
+  if (control.shouldThrow) throw new Error("Maximum update depth exceeded");
+  return <div>recovered</div>;
+}
+
+function boundary(props: Partial<ErrorBoundaryProps>): ReactNode {
+  return (
+    <Theme>
+      <ErrorBoundary {...props}>
+        <Flaky />
+      </ErrorBoundary>
+    </Theme>
+  );
+}
+
+beforeEach(() => {
+  control.shouldThrow = true;
+  // React logs caught errors to console.error; silence the expected noise.
+  vi.spyOn(console, "error").mockImplementation(() => {});
+});
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe("ErrorBoundary autoRecover", () => {
+  it("replicates the defect: without autoRecover the fallback stays even after the transient condition clears (no resetKey change, no manual retry)", () => {
+    const { rerender } = render(boundary({ resetKey: "task-a" }));
+    expect(screen.getByText("Something went wrong")).toBeInTheDocument();
+
+    // The transient loop condition clears, but navigating back to the SAME
+    // task doesn't change resetKey — the boundary is stuck on the error.
+    control.shouldThrow = false;
+    rerender(boundary({ resetKey: "task-a" }));
+
+    expect(screen.getByText("Something went wrong")).toBeInTheDocument();
+    expect(screen.queryByText("recovered")).not.toBeInTheDocument();
+  });
+
+  it("auto-recovers a transient error without a manual retry, and reports it first", async () => {
+    const onError = vi.fn();
+    render(boundary({ autoRecover: true, onError }));
+
+    // While the bounded recovery is pending it renders nothing — no flash of
+    // the scary error UI for a one-frame transient — but it HAS reported it.
+    expect(screen.queryByText("Something went wrong")).not.toBeInTheDocument();
+    expect(screen.queryByText("recovered")).not.toBeInTheDocument();
+    expect(onError).toHaveBeenCalledTimes(1);
+
+    // The transient condition clears; the scheduled reset then re-renders the
+    // healthy child on its own.
+    control.shouldThrow = false;
+    expect(await screen.findByText("recovered")).toBeInTheDocument();
+    expect(screen.queryByText("Something went wrong")).not.toBeInTheDocument();
+  });
+
+  it("is bounded: a persistent loop lands on the manual fallback instead of retrying forever", async () => {
+    const onError = vi.fn();
+    render(boundary({ autoRecover: true, onError }));
+
+    // The child keeps throwing; after the bounded retries are spent the manual
+    // fallback appears rather than an unbounded catch/retry storm.
+    expect(await screen.findByText("Something went wrong")).toBeInTheDocument();
+    expect(screen.queryByText("recovered")).not.toBeInTheDocument();
+    // 1 initial catch + at most MAX_AUTO_RECOVERIES (2) re-throws.
+    expect(onError.mock.calls.length).toBeGreaterThanOrEqual(1);
+    expect(onError.mock.calls.length).toBeLessThanOrEqual(3);
+
+    // And it stays settled on the fallback — no further retries fire.
+    const callsAfterSettle = onError.mock.calls.length;
+    await new Promise((resolve) => setTimeout(resolve, 500));
+    expect(onError.mock.calls.length).toBe(callsAfterSettle);
+    expect(screen.getByText("Something went wrong")).toBeInTheDocument();
+  });
+
+  it("manual retry still recovers after auto-recovery is exhausted", async () => {
+    const user = userEvent.setup();
+    render(boundary({ autoRecover: true }));
+    expect(await screen.findByText("Something went wrong")).toBeInTheDocument();
+
+    control.shouldThrow = false;
+    await user.click(screen.getByRole("button", { name: /try again/i }));
+
+    await waitFor(() =>
+      expect(screen.getByText("recovered")).toBeInTheDocument(),
+    );
+  });
+});

packages/ui/src/primitives/ErrorBoundary.tsx

@@ -2,13 +2,37 @@ import { Warning } from "@phosphor-icons/react";
 import { Box, Button, Callout, Flex, Text } from "@radix-ui/themes";
 import { Component, type ErrorInfo, type ReactNode } from "react";
 
+// A transient render-loop error (React #185 "Maximum update depth exceeded" from
+// a layout-timing setState cycle) trips once, then clears on the next mount —
+// which is why clicking "Try again" has always recovered it. `autoRecover`
+// replays that click for the user: after catching, it schedules a bounded number
+// of resets so a transient error heals itself, while a persistent one still
+// lands on the manual fallback instead of thrashing. The error is always
+// reported via `onError` first, so root-cause visibility is unaffected.
+const MAX_AUTO_RECOVERIES = 2;
+const AUTO_RECOVER_BASE_DELAY_MS = 150;
+// Once the subtree has rendered cleanly for this long, treat it as healthy and
+// refill the recovery budget so a later, unrelated transient error gets its own
+// retries rather than being starved by an earlier one.
+const RECOVERY_STABLE_RESET_MS = 4000;
+
 export interface ErrorBoundaryProps {
   children: ReactNode;
   fallback?: ReactNode;
   /** Optional name to identify which boundary caught the error */
   name?: string;
   /** When this value changes, the boundary clears its error state. */
   resetKey?: unknown;
+  /**
+   * Auto-clear a caught error a bounded number of times before showing the
+   * manual retry fallback. Use for subtrees prone to transient render-loop
+   * errors that clear on remount (e.g. the chat SessionView, whose #185 loop
+   * has always been recoverable via "Try again"). While an auto-recovery is
+   * pending the boundary renders nothing rather than flashing the error UI. The
+   * error is still reported via `onError` on every catch, so observability is
+   * unaffected. Off by default.
+   */
+  autoRecover?: boolean;
   /**
    * If returns true for a caught error, the boundary renders nothing,
    * skips the fallback UI, and waits for `resetKey` to change before
@@ -30,10 +54,21 @@ export interface ErrorBoundaryProps {
 interface State {
   error: Error | null;
   lastResetKey: unknown;
+  /** How many times autoRecover has cleared an error since the last healthy
+   * stretch / resetKey change. Caps auto-recovery so a persistent loop can't
+   * thrash the boundary. */
+  recoveryAttempts: number;
 }
 
 export class ErrorBoundary extends Component<ErrorBoundaryProps, State> {
-  state: State = { error: null, lastResetKey: this.props.resetKey };
+  state: State = {
+    error: null,
+    lastResetKey: this.props.resetKey,
+    recoveryAttempts: 0,
+  };
+
+  private recoverTimer: ReturnType<typeof setTimeout> | null = null;
+  private stableTimer: ReturnType<typeof setTimeout> | null = null;
 
   static getDerivedStateFromError(error: Error): Partial<State> {
     return { error };
@@ -44,7 +79,9 @@ export class ErrorBoundary extends Component<ErrorBoundaryProps, State> {
     state: State,
   ): Partial<State> | null {
     if (props.resetKey === state.lastResetKey) return null;
-    return { error: null, lastResetKey: props.resetKey };
+    // A genuine new context (e.g. a different task): clear the error and refill
+    // the auto-recovery budget.
+    return { error: null, lastResetKey: props.resetKey, recoveryAttempts: 0 };
   }
 
   componentDidCatch(error: Error, errorInfo: ErrorInfo) {
@@ -53,16 +90,79 @@ export class ErrorBoundary extends Component<ErrorBoundaryProps, State> {
       componentStack: errorInfo.componentStack,
       suppressed,
     });
+    if (suppressed) return;
+    // A fresh error means the subtree isn't healthy yet — cancel any pending
+    // budget refill and, if enabled, schedule the next bounded auto-recovery.
+    this.clearStableTimer();
+    if (this.props.autoRecover) this.scheduleAutoRecover();
+  }
+
+  componentDidUpdate(_prevProps: ErrorBoundaryProps, prevState: State) {
+    // The error just cleared (auto-recovery or manual retry). If it doesn't
+    // recur within the stability window, refill the recovery budget so a later
+    // unrelated transient error isn't starved of retries.
+    if (this.props.autoRecover && prevState.error && !this.state.error) {
+      this.clearStableTimer();
+      this.stableTimer = setTimeout(() => {
+        this.stableTimer = null;
+        if (this.state.recoveryAttempts !== 0) {
+          this.setState({ recoveryAttempts: 0 });
+        }
+      }, RECOVERY_STABLE_RESET_MS);
+    }
+  }
+
+  componentWillUnmount() {
+    this.clearRecoverTimer();
+    this.clearStableTimer();
+  }
+
+  private scheduleAutoRecover(): void {
+    if (this.recoverTimer !== null) return;
+    if (this.state.recoveryAttempts >= MAX_AUTO_RECOVERIES) return;
+    // Back off a little per attempt so a loop that needs a beat to settle gets
+    // progressively more time before we give up to the manual fallback.
+    const delay =
+      AUTO_RECOVER_BASE_DELAY_MS * (this.state.recoveryAttempts + 1);
+    this.recoverTimer = setTimeout(() => {
+      this.recoverTimer = null;
+      this.setState((s) =>
+        s.error
+          ? { error: null, recoveryAttempts: s.recoveryAttempts + 1 }
+          : null,
+      );
+    }, delay);
+  }
+
+  private clearRecoverTimer(): void {
+    if (this.recoverTimer !== null) {
+      clearTimeout(this.recoverTimer);
+      this.recoverTimer = null;
+    }
+  }
+
+  private clearStableTimer(): void {
+    if (this.stableTimer !== null) {
+      clearTimeout(this.stableTimer);
+      this.stableTimer = null;
+    }
   }
 
   handleRetry = () => {
-    this.setState({ error: null });
+    this.clearRecoverTimer();
+    this.setState({ error: null, recoveryAttempts: 0 });
   };
 
   render() {
-    const { error } = this.state;
+    const { error, recoveryAttempts } = this.state;
     if (!error) return this.props.children;
     if (this.props.shouldSuppress?.(error)) return null;
+    // While a bounded auto-recovery is still pending, render nothing rather than
+    // flashing the error UI for what is (usually) a one-frame transient loop.
+    // componentDidCatch has scheduled the reset that will re-render the children.
+    if (this.props.autoRecover && recoveryAttempts < MAX_AUTO_RECOVERIES) {
+      return null;
+    }
     if (this.props.fallback) return this.props.fallback;
 
     return (