fix(desktop): reduce app crashes when a preview tab's renderer dies during agent screenshots

apps/desktop/src/preview/DesktopBrowserHost.test.ts

@@ -3,6 +3,7 @@ import { describe, expect, it } from "@effect/vitest";
 import { DesktopBrowserEvent } from "@t3tools/contracts";
 import * as Effect from "effect/Effect";
 import * as Fiber from "effect/Fiber";
+import * as Queue from "effect/Queue";
 import * as Schema from "effect/Schema";
 import * as Stream from "effect/Stream";
 import * as NodeEvents from "node:events";
@@ -15,32 +16,54 @@ const decodeCdpReply = Schema.decodeUnknownSync(
   Schema.fromJsonString(Schema.Struct({ id: Schema.Number })),
 );
 const key = { threadId: "thread-1", tabId: "tab-1" };
+/** Sends a page command whose id the fake debugger records as it receives it. */
+const command = (
+  host: DesktopBrowserHost.DesktopBrowserHost["Service"],
+  id: number,
+  method: string,
+) =>
+  host.handleCommandLine(
+    encodeJson({
+      type: "cdp",
+      ...key,
+      message: encodeJson({ id, method, params: { id }, sessionId: "t3-preview-page" }),
+    }),
+  );
 
 const makeDebuggee = () => {
   const emitter = new NodeEvents.EventEmitter();
   const pending: Array<() => void> = [];
+  const sent: Array<unknown> = [];
+  const sendWaiters: Array<() => void> = [];
   const debuggee = Object.assign(emitter, {
-    sendCommand: (method: string) =>
+    sendCommand: (method: string, params?: { id?: number }) =>
       new Promise((resolve) => {
         if (method === "Target.getTargetInfo") {
           resolve({ targetInfo: { targetId: "GUEST" } });
           return;
         }
+        sent.push(params?.id);
         pending.push(() => resolve({ method }));
+        sendWaiters.shift()?.();
       }),
   });
-  const webContents = {
+  const webContents = Object.assign(new NodeEvents.EventEmitter(), {
     getURL: () => "http://localhost/",
     getTitle: () => "Page",
     getUserAgent: () => "Electron",
-  };
+  });
   return {
     tab: {
       webContents: webContents as unknown as Electron.WebContents,
       debugger: debuggee as unknown as Electron.Debugger,
     },
     emit: (method: string, params: unknown) => emitter.emit("message", {}, method, params, ""),
+    navigate: () => webContents.emit("did-navigate"),
+    /** Ids of the commands the debugger received, in order. */
+    sent,
+    nextSend: () => new Promise<void>((resolve) => sendWaiters.push(resolve)),
     release: () => pending.splice(0).forEach((resolve) => resolve()),
+    releaseFirst: () => pending.shift()?.(),
   };
 };
 
@@ -51,6 +74,25 @@ const takeEvents = (host: DesktopBrowserHost.DesktopBrowserHost["Service"], coun
     Effect.map((lines) => lines.map((line) => decodeEvent(new TextDecoder().decode(line)))),
   );
 
+/**
+ * Ids of the replies the host sends, in order. A screenshot's reply leaves after its turn
+ * has passed on, so by then the next waiting screenshot, if any, has been sent. Subscribe
+ * before sending commands: a new subscriber releases the tab's relay.
+ */
+const watchReplies = (host: DesktopBrowserHost.DesktopBrowserHost["Service"]) =>
+  Effect.gen(function* () {
+    const replies = yield* Queue.unbounded<number>();
+    yield* host.events.pipe(
+      Stream.map((line) => decodeEvent(new TextDecoder().decode(line))),
+      Stream.runForEach((event) =>
+        event.type === "cdp" ? Queue.offer(replies, decodeCdpReply(event.message).id) : Effect.void,
+      ),
+      Effect.forkScoped,
+    );
+    yield* Effect.yieldNow;
+    return replies;
+  });
+
 describe("DesktopBrowserHost", () => {
   it.effect("announces tabs already attached to a backend that starts later", () =>
     Effect.gen(function* () {
@@ -89,6 +131,67 @@ describe("DesktopBrowserHost", () => {
     }).pipe(Effect.scoped),
   );
 
+  it.effect("sends a page one screenshot at a time", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      host.attach(key, debuggee.tab);
+      const replies = yield* watchReplies(host);
+
+      yield* command(host, 1, "Page.captureScreenshot");
+      yield* command(host, 2, "Page.captureScreenshot");
+      yield* command(host, 3, "DOM.enable");
+      expect(debuggee.sent).toEqual([1, 3]);
+
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(1);
+      expect(debuggee.sent).toEqual([1, 3, 2]);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("stops waiting on a screenshot the page navigated away from", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      host.attach(key, debuggee.tab);
+      const replies = yield* watchReplies(host);
+
+      yield* command(host, 1, "Page.captureScreenshot");
+      yield* command(host, 2, "Page.captureScreenshot");
+      yield* command(host, 3, "Page.captureScreenshot");
+      debuggee.navigate();
+      expect(debuggee.sent).toEqual([1, 2]);
+
+      // The superseded screenshot answering late must not start a third beside the second.
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(1);
+      expect(debuggee.sent).toEqual([1, 2]);
+
+      debuggee.releaseFirst();
+      expect(yield* Queue.take(replies)).toBe(2);
+      expect(debuggee.sent).toEqual([1, 2, 3]);
+    }).pipe(Effect.scoped),
+  );
+
+  it.effect("drops screenshots a released relay left waiting", () =>
+    Effect.gen(function* () {
+      const host = yield* DesktopBrowserHost.make;
+      const debuggee = makeDebuggee();
+      host.attach(key, debuggee.tab);
+
+      yield* command(host, 1, "Page.captureScreenshot");
+      yield* command(host, 2, "Page.captureScreenshot");
+      yield* host.handleCommandLine(encodeJson({ type: "release", ...key }));
+      yield* command(host, 3, "Page.captureScreenshot");
+      expect(debuggee.sent).toEqual([1]);
+
+      const sent = debuggee.nextSend();
+      debuggee.release();
+      yield* Effect.promise(() => sent);
+      expect(debuggee.sent).toEqual([1, 3]);
+    }),
+  );
+
   it.effect("saves a server tab's download under its CDP guid where the server asked", () =>
     Effect.gen(function* () {
       const host = yield* DesktopBrowserHost.make;

apps/desktop/src/preview/DesktopBrowserHost.ts

@@ -43,6 +43,8 @@ interface AttachedTab {
   readonly key: DesktopBrowserTabKey;
   readonly debuggee: DesktopBrowserTabDebugger;
   relay: CdpRelayConnection | null;
+  screenshotTurn: object | null;
+  readonly waitingScreenshots: Array<() => void>;
   downloadDirectory: string | null;
   pendingDownloadGuid: string | null;
   agentInputAt: number;
@@ -52,6 +54,7 @@ interface AttachedTab {
     params: unknown,
     sessionId: string,
   ) => void;
+  readonly onNewDocument: () => void;
 }
 
 export class DesktopBrowserHost extends Context.Service<
@@ -84,15 +87,54 @@ export const make = Effect.gen(function* () {
   const tabs = new Map<string, AttachedTab>();
   const emit = (event: DesktopBrowserEventType) => runFork(PubSub.publish(outbox, event));
 
+  const nextScreenshot = (tab: AttachedTab) => {
+    tab.screenshotTurn = null;
+    tab.waitingScreenshots.shift()?.();
+  };
+
+  /**
+   * Chromium below 157 crashes the whole app when a guest's renderer exits with a
+   * screenshot copy in flight (crbug.com/567321049). Overlapping screenshots make that
+   * far likelier, and one the server gave up on stays pending in Chromium, so each guest
+   * gets one at a time. One queued by a relay released since then is dropped unsent.
+   */
+  const takeScreenshot = (
+    tab: AttachedTab,
+    relay: CdpRelayConnection,
+    capture: () => Promise<unknown>,
+  ) =>
+    new Promise<unknown>((resolve, reject) => {
+      const start = () => {
+        if (tab.relay !== relay) {
+          reject(new Error("The preview connection that asked for this screenshot closed."));
+          nextScreenshot(tab);
+          return;
+        }
+        const turn = {};
+        tab.screenshotTurn = turn;
+        capture()
+          .finally(() => {
+            if (tab.screenshotTurn === turn) nextScreenshot(tab);
+          })
+          .then(resolve, reject);
+      };
+      if (tab.screenshotTurn === null) start();
+      else tab.waitingScreenshots.push(start);
+    });
+
   const relayFor = (tab: AttachedTab) => {
     if (tab.relay) return tab.relay;
     const { webContents, debugger: debuggee } = tab.debuggee;
+    const send = (method: string, params: Record<string, unknown>, sessionId?: string) =>
+      sessionId === undefined
+        ? sendPageCommand(webContents, debuggee, method, params)
+        : debuggee.sendCommand(method, params, sessionId);
     const relay: CdpRelayConnection = createCdpRelayConnection(
       {
         send: (method, params, sessionId) =>
-          sessionId === undefined
-            ? sendPageCommand(webContents, debuggee, method, params)
-            : debuggee.sendCommand(method, params, sessionId),
+          method === "Page.captureScreenshot"
+            ? takeScreenshot(tab, relay, () => send(method, params, sessionId))
+            : send(method, params, sessionId),
         targetId: () =>
           debuggee
             .sendCommand("Target.getTargetInfo")
@@ -133,7 +175,9 @@ export const make = Effect.gen(function* () {
     const tab = tabs.get(id);
     if (!tab) return;
     tabs.delete(id);
+    tab.relay = null;
     tab.debuggee.debugger.off("message", tab.onMessage);
+    tab.debuggee.webContents.off("did-navigate", tab.onNewDocument);
     emit({ type: "detached", ...key });
   };
 
@@ -145,6 +189,8 @@ export const make = Effect.gen(function* () {
       key,
       debuggee,
       relay: null,
+      screenshotTurn: null,
+      waitingScreenshots: [],
       downloadDirectory: null,
       pendingDownloadGuid: null,
       agentInputAt: Number.NEGATIVE_INFINITY,
@@ -155,9 +201,15 @@ export const make = Effect.gen(function* () {
         }
         tab.relay?.event(method, params, sessionId);
       },
+      // A screenshot pending when the page loads a new document, a reload included, can
+      // stay unanswered. A crashed page lands here once it reloads, well after its renderer died.
+      onNewDocument: () => {
+        if (tab.screenshotTurn !== null) nextScreenshot(tab);
+      },
     };
     tabs.set(id, tab);
     debuggee.debugger.on("message", tab.onMessage);
+    debuggee.webContents.on("did-navigate", tab.onNewDocument);
     emit({ type: "attached", ...key });
   };