fix(preview): keep load failures tied to the current page

Author: juliusmarminge

apps/desktop/src/preview/DesktopBrowserHost.ts

@@ -7,6 +7,7 @@ import {
   DesktopBrowserCommand,
   DesktopBrowserEvent,
   type DesktopBrowserEvent as DesktopBrowserEventType,
+  type PreviewAutomationLoadError,
 } from "@t3tools/contracts";
 import * as Context from "effect/Context";
 import * as NodePath from "node:path";
@@ -61,11 +62,7 @@ export class DesktopBrowserHost extends Context.Service<
     readonly handleCommandLine: (line: string) => Effect.Effect<void>;
     readonly attach: (key: DesktopBrowserTabKey, debuggee: DesktopBrowserTabDebugger) => void;
     readonly detach: (key: DesktopBrowserTabKey) => void;
-    /** Tells the server an attached tab's page failed to load; ignored for other tabs. */
-    readonly loadFailed: (
-      key: DesktopBrowserTabKey,
-      failure: { readonly url: string; readonly code: number; readonly description: string },
-    ) => void;
+    readonly loadFailed: (key: DesktopBrowserTabKey, failure: PreviewAutomationLoadError) => void;
     readonly humanStartedDownload: (source: Electron.WebContents) => boolean;
     readonly placeDownload: (source: Electron.WebContents, item: Electron.DownloadItem) => boolean;
     readonly pointers: Stream.Stream<{

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

@@ -3071,22 +3071,31 @@ describe("PreviewManager", () => {
             },
           },
         );
+        const emit = (event: string, ...args: unknown[]) => {
+          for (const listener of listeners.get(event) ?? []) listener(...args);
+        };
         return {
           wc,
-          fail: (url: string) => {
-            for (const listener of listeners.get("did-fail-load") ?? []) {
-              listener({}, -202, "ERR_CERT_AUTHORITY_INVALID", url, true);
-            }
-          },
+          emit,
+          fail: (url: string) =>
+            emit("did-fail-load", {}, -202, "ERR_CERT_AUTHORITY_INVALID", url, true),
         };
       };
       const initial = makeGuest(42);
       const replacement = makeGuest(43);
       const manager = yield* PreviewManager.PreviewManager;
+      const statuses: PreviewManager.PreviewNavStatus[] = [];
+      yield* manager.subscribeStateChanges((_tabId, state) =>
+        Effect.sync(() => {
+          statuses.push(state.navStatus);
+        }),
+      );
       yield* manager.createTab("tab_1", { serverTab });
       yield* manager.prepareWebview(initial.wc);
       yield* manager.prepareWebview(replacement.wc);
       initial.fail("https://first.local/");
+      initial.emit("did-start-navigation", { isMainFrame: false, isSameDocument: false });
+      initial.emit("did-start-loading");
       replacement.fail("https://second.local/");
       const failure = (url: string) => ({
         type: "loadFailed",
@@ -3111,6 +3120,17 @@ describe("PreviewManager", () => {
       initial.fail("https://stale.local/");
       replacement.fail("https://current.local/");
       expect(JSON.parse(yield* Queue.take(lines))).toEqual(failure("https://current.local/"));
+      expect(statuses).not.toContainEqual(expect.objectContaining({ url: "https://stale.local/" }));
+
+      const retried = makeGuest(44);
+      yield* manager.prepareWebview(retried.wc);
+      retried.fail("https://retry.local/");
+      retried.emit("did-start-navigation", { isMainFrame: true, isSameDocument: false });
+      fromId.mockReturnValue(retried.wc);
+      yield* manager.registerWebview("tab_1", 44);
+      expect(JSON.parse(yield* Queue.take(lines))).toEqual({ type: "detached", ...serverTab });
+      expect(JSON.parse(yield* Queue.take(lines))).toEqual({ type: "attached", ...serverTab });
+      expect(statuses.at(-1)?.kind).toBe("Success");
     }).pipe(Effect.provide(managerLayer()), Effect.scoped),
   );
 

apps/desktop/src/preview/Manager.ts

@@ -25,6 +25,7 @@ import type {
   DesktopPreviewScreenshotArtifact,
   DesktopPreviewTabDefaults,
   PreviewForwardedShortcut,
+  PreviewAutomationLoadError,
 } from "@t3tools/contracts";
 import * as HostProcess from "@t3tools/shared/HostProcess";
 import { matchesKeybindingShortcut } from "@t3tools/shared/keybindings";
@@ -603,15 +604,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
     { readonly semaphore: Semaphore.Semaphore; users: number }
   >();
   const tabLifecycleGenerations = new Map<string, number>();
-  /**
-   * The main-frame failure of each guest's current load, by webContents id.
-   * Watched from attach, because a guest starts loading before registerWebview
-   * installs its tab's listeners.
-   */
-  const guestLoadFailures = new Map<
-    number,
-    { readonly url: string; readonly code: number; readonly description: string }
-  >();
+  // A guest can fail its first load before registerWebview installs its tab listeners.
+  const guestLoadFailures = new Map<number, PreviewAutomationLoadError>();
 
   const attempt = <A>(errorContext: PreviewOperationContext, evaluate: () => A) =>
     Effect.try({
@@ -1460,23 +1454,40 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
       validatedUrl: string,
       isMainFrame: boolean,
     ): void => {
-      if (code === -3 || !isMainFrame) return;
+      if (code === -3 || !isMainFrame || wc.isDestroyed()) return;
       const url = validatedUrl || wc.getURL();
       runFork(
-        update(tabId, {
-          navStatus: { kind: "LoadFailed", url, title: wc.getTitle(), code, description },
-        }).pipe(
-          Effect.andThen(SynchronizedRef.get(tabsRef)),
-          Effect.tap((tabs) =>
-            Effect.sync(() => {
-              const tab = tabs.get(tabId);
-              // A replaced guest's failure is not the current page's.
-              if (tab?.serverTab && tab.webContentsId === wc.id) {
-                browserHost.loadFailed(tab.serverTab, { url, code, description });
-              }
-            }),
-          ),
-        ),
+        Effect.gen(function* () {
+          const updatedAt = yield* currentIso;
+          const next = yield* SynchronizedRef.modify(tabsRef, (tabs) => {
+            const current = tabs.get(tabId);
+            if (
+              !current ||
+              current.webContentsId !== wc.id ||
+              wc.isDestroyed() ||
+              webContents.fromId(wc.id) !== wc
+            ) {
+              return [Option.none<PreviewTabState>(), tabs] as const;
+            }
+            const state: PreviewTabState = {
+              ...current,
+              navStatus: { kind: "LoadFailed", url, title: wc.getTitle(), code, description },
+              updatedAt,
+            };
+            return [
+              Option.some(state),
+              replaceMap(tabs, (copy) => copy.set(tabId, state)),
+            ] as const;
+          });
+          if (Option.isNone(next)) return;
+          const current = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
+          if (current?.webContentsId !== wc.id || current.navStatus !== next.value.navStatus)
+            return;
+          if (current.serverTab) {
+            browserHost.loadFailed(current.serverTab, { url, code, description });
+          }
+          yield* emitIfCurrent(tabId, next.value);
+        }),
       );
     };
     const handleHumanInput = Effect.fn("PreviewManager.handleHumanInput")(function* (
@@ -2004,7 +2015,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
   ) {
     yield* rendererHistory.register(wc, { surface: "preview" });
     const webContentsId = wc.id;
-    wc.on("did-start-loading", () => guestLoadFailures.delete(webContentsId));
+    wc.on("did-start-navigation", (event) => {
+      if (event.isMainFrame && !event.isSameDocument) guestLoadFailures.delete(webContentsId);
+    });
     wc.on("did-fail-load", (_event, code, description, validatedUrl, isMainFrame) => {
       if (code === -3 || !isMainFrame) return;
       guestLoadFailures.set(webContentsId, {
@@ -2411,15 +2424,15 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
       const beforeAttach = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
       if (beforeAttach?.webContentsId !== wc.id) return;
       const control = yield* ensureControlSession(wc);
+      if (beforeAttach.serverTab) yield* listenForAgentPointers;
       const afterAttach = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
       if (afterAttach?.webContentsId !== wc.id) {
         yield* detachControlSession(wc.id);
         return;
       }
       if (afterAttach.serverTab) {
-        yield* listenForAgentPointers;
         browserHost.attach(afterAttach.serverTab, { webContents: wc, debugger: control.debugger });
-        // A first load can fail before the attach, while no server is listening.
+        // CDP cannot recover a failure that happened before it attached.
         const { navStatus } = afterAttach;
         if (navStatus.kind === "LoadFailed") {
           browserHost.loadFailed(afterAttach.serverTab, {

apps/server/src/preview/DesktopBrowserChannel.ts

@@ -53,7 +53,6 @@ export class DesktopBrowserChannel extends Context.Service<
     readonly detached: Stream.Stream<DesktopTabKey>;
     readonly attached: Stream.Stream<DesktopTabKey>;
     readonly isAttached: (key: DesktopTabKey) => Effect.Effect<boolean>;
-    /** The latest page-load failure the desktop reported for an attached tab. */
     readonly loadFailure: (
       key: DesktopTabKey,
     ) => Effect.Effect<Option.Option<PreviewAutomationLoadError>>;
@@ -126,15 +125,13 @@ const make = Effect.gen(function* () {
           return PubSub.publish(changes, { key, attached: true });
         case "loadFailed":
           loadFailures.set(id, {
-            // Matches the cap on failures the server observes itself.
             url: event.url.slice(0, 2048),
             code: event.code,
             description: event.description,
           });
           return Effect.void;
         case "detached": {
           attachedTabs.delete(id);
-          // The desktop reports the failure again when the tab reattaches.
           loadFailures.delete(id);
           const queue = inbound.get(id);
           return (queue ? Queue.shutdown(queue) : Effect.void).pipe(

apps/server/src/preview/ServerBrowser.ts

@@ -308,11 +308,7 @@ interface ServerTab {
   colorScheme: PreviewAppearancePreference;
   zoomFactor: number;
   loading: boolean;
-  /**
-   * Why the latest main-frame navigation this server saw failed, null while it
-   * has not failed, and undefined until the server sees a navigation at all.
-   */
-  loadError: PreviewAutomationLoadError | null | undefined;
+  loadError: PreviewAutomationLoadError | undefined;
   closing: boolean;
   recording: Recording | null;
   initialNavigation: Promise<void> | null;
@@ -802,7 +798,7 @@ const make = Effect.gen(function* () {
       navigationGenerations.set(request, ++tab.navigationGeneration);
       clearAbortedNavigation(tab);
       tab.loading = true;
-      tab.loadError = null;
+      tab.loadError = undefined;
       report(tab, { _tag: "Loading", url: request.url().slice(0, 2048), title: "" });
     });
     page.on("load", () => {
@@ -1346,13 +1342,10 @@ const make = Effect.gen(function* () {
     );
   };
 
-  /**
-   * Why the tab shows the browser's error page. A desktop tab's first load can
-   * fail before this server connects, so the desktop's own report fills in.
-   */
   const loadErrorOf = async (tab: ServerTab) => {
     if (!tab.page.url().startsWith("chrome-error://")) return undefined;
-    if (tab.loadError !== undefined || !tab.desktop) return tab.loadError ?? undefined;
+    // A desktop failure from before CDP attached is superseded by any navigation we observe.
+    if (tab.loadError || !tab.desktop || tab.navigationGeneration > 0) return tab.loadError;
     const reported = await Effect.runPromise(
       desktopChannel.loadFailure({ threadId: tab.threadId, tabId: tab.tabId }),
     );