From ae7cc72e60cfb8bd33da7885b36aa5543ccc5575 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 15:58:55 -0600 Subject: [PATCH 1/2] fix(server): report failed preview navigation URLs - Preserve the failed destination URL in preview status and snapshots - Reject opens when navigation fails before automation attaches --- apps/server/src/preview/ServerBrowser.test.ts | 67 +++++++++++++++++-- apps/server/src/preview/ServerBrowser.ts | 49 +++++++++----- 2 files changed, 95 insertions(+), 21 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 3af55fefbc0e2..217ac4b4028b5 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2649,8 +2649,14 @@ it.live("reports why a page failed to load while the tab shows the error page", page.emit("request", request); page.emit("requestfailed", request); page.url = () => "chrome-error://chromewebdata/"; - expect((yield* status()).loadError).toEqual(certificateError); - expect((yield* snapshot()).loadError).toEqual(certificateError); + expect(yield* status()).toMatchObject({ + url: certificateError.url, + loadError: certificateError, + }); + expect(yield* snapshot()).toMatchObject({ + url: certificateError.url, + loadError: certificateError, + }); const subframeRequest = { ...request, @@ -2659,7 +2665,10 @@ it.live("reports why a page failed to load while the tab shows the error page", }; page.emit("request", subframeRequest); page.emit("requestfailed", subframeRequest); - expect((yield* status()).loadError).toEqual(certificateError); + expect(yield* status()).toMatchObject({ + url: certificateError.url, + loadError: certificateError, + }); // Clearing must happen before the next navigation replaces the error page. page.emit("request", { ...request, url: () => "https://device.local/login" }); @@ -2670,6 +2679,48 @@ it.live("reports why a page failed to load while the tab shows the error page", page.emit("request", dataRequest); page.emit("requestfailed", dataRequest); expect((yield* status()).loadError?.url).toBe("data:text/html,…"); + initialNavigation = async () => { + page.emit("request", request); + page.emit("requestfailed", request); + }; + const error = yield* broker + .invoke({ + scope, + tabId, + operation: "navigate", + input: { url: certificateError.url }, + }) + .pipe(Effect.flip); + expect(error).toMatchObject({ + _tag: "PreviewAutomationExecutionError", + reason: `Navigation to ${certificateError.url} failed: ${certificateError.description}`, + }); + }), + ).pipe(Effect.provide(layer)), +); + +it.live("preview_open rejects a desktop navigation that failed before automation attached", () => + Effect.scoped( + Effect.gen(function* () { + const { broker, tabId } = yield* ready; + const manager = yield* Manager.PreviewManager; + const channel = yield* DesktopChannel.DesktopBrowserChannel; + desktopRendersNext = true; + desktopPageUrl = "chrome-error://chromewebdata/"; + using _failure = vi.spyOn(channel, "loadFailure").mockReturnValue(certificateError); + const error = yield* broker + .invoke({ + scope, + operation: "open", + input: { url: certificateError.url, reuseExistingTab: false, open: false }, + }) + .pipe(Effect.flip); + expect(error).toMatchObject({ + _tag: "PreviewAutomationExecutionError", + reason: `Navigation to ${certificateError.url} failed: ${certificateError.description}`, + }); + const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); + expect(sessions.map((session) => session.tabId)).toEqual([tabId]); }), ).pipe(Effect.provide(layer)), ); @@ -2701,8 +2752,14 @@ it.live("reports a desktop tab's load failure from before the server connected", input: {}, tabId: PreviewTabId.make(opened.tabId), }); - expect((yield* status()).loadError).toEqual(certificateError); - expect((yield* snapshot()).loadError).toEqual(certificateError); + expect(yield* status()).toMatchObject({ + url: certificateError.url, + loadError: certificateError, + }); + expect(yield* snapshot()).toMatchObject({ + url: certificateError.url, + loadError: certificateError, + }); desktopLoadFailures.set(opened.tabId, { ...certificateError, url: dataUrl }); expect((yield* status()).loadError?.url).toBe("data:text/html,…"); // A navigation the server sees supersedes it, even before the error page goes. diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 18d08e65acd08..ed53efd77a65e 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1624,6 +1624,17 @@ const make = Effect.gen(function* () { return reported && { ...reported, url: ServerBrowserPage.withoutDataPayload(reported.url) }; }; + const assertNavigationSucceeded = (tab: ServerTab) => { + const failure = loadErrorOf(tab); + if (failure) { + throw new ServerBrowserPage.ServerBrowserOperationError({ + tag: "PreviewAutomationExecutionError", + message: `Navigation to ${failure.url} failed: ${failure.description}`, + detail: failure, + }); + } + }; + const statusWithTitle = async ( tab: ServerTab | undefined, agentSessionId?: string, @@ -1646,7 +1657,7 @@ const make = Effect.gen(function* () { available: true, visible: isVisible(tab), tabId: tab.tabId, - url: url === "about:blank" ? null : url, + url: loadError?.url ?? (url === "about:blank" ? null : url), title: null, loading: tab.loading, ...(loadError ? { loadError } : {}), @@ -1668,7 +1679,8 @@ const make = Effect.gen(function* () { url: candidate.page.url() === "about:blank" ? null - : ServerBrowserPage.withoutDataPayload(candidate.page.url()), + : (loadErrorOf(candidate)?.url ?? + ServerBrowserPage.withoutDataPayload(candidate.page.url())), ...(candidate.openerTabId === undefined ? {} : { openerTabId: candidate.openerTabId }), owner: tabOwner(candidate), ownedByCaller: candidate.control.agentId === agentSessionId, @@ -1732,6 +1744,7 @@ const make = Effect.gen(function* () { } throw ServerBrowserPage.toOperationError(cause, timeout); }); + assertNavigationSucceeded(tab); }; const withCaptureLock = (tab: ServerTab, operation: () => Promise): Promise => { @@ -2092,15 +2105,24 @@ const make = Effect.gen(function* () { if (url) await navigate(tab, url, "load", navigationTimeout); } else { // A failed open returns no tabId; a leftover hidden tab would block later reuse. - await tab.initialNavigation?.catch(async (cause: unknown) => { - // Playwright rejects downloads too; their tab must survive to save the file. - if (cause instanceof Error && cause.message.includes("Download is starting")) { + await Promise.resolve(tab.initialNavigation) + .then(async () => { + if (url) { + await tab.page + .waitForLoadState("load", { timeout: navigationTimeout }) + .catch(constVoid); + } + assertNavigationSucceeded(tab); + }) + .catch(async (cause: unknown) => { + // Playwright rejects downloads too; their tab must survive to save the file. + if (cause instanceof Error && cause.message.includes("Download is starting")) { + throw cause; + } + await runPromise(manager.close({ threadId: tab.threadId, tabId: tab.tabId })); + dropTab(tab, false); throw cause; - } - await runPromise(manager.close({ threadId: tab.threadId, tabId: tab.tabId })); - dropTab(tab, false); - throw cause; - }); + }); } const reveal = open.open ?? open.show; if (reveal !== false) { @@ -2112,11 +2134,6 @@ const make = Effect.gen(function* () { }), ); } - if (!existing && url) { - await tab.page - .waitForLoadState("load", { timeout: navigationTimeout }) - .catch(constVoid); - } return statusWithTitle(tab, request.agentSessionId); }, deadlineMs, @@ -2270,7 +2287,7 @@ const make = Effect.gen(function* () { }), ); const loadError = loadErrorOf(tab); - return loadError ? { ...snapshot, loadError } : snapshot; + return loadError ? { ...snapshot, url: loadError.url, loadError } : snapshot; } case "click": { const clickInput = input as PreviewAutomationClickInput; From 0c140a7870c1238e18cc923971e785c28a8c1184 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Wed, 11 Nov 2026 16:14:35 -0600 Subject: [PATCH 2/2] fix(server): sanitize preview failure diagnostics --- apps/server/src/preview/ServerBrowser.test.ts | 11 ++++++++--- apps/server/src/preview/ServerBrowser.ts | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 217ac4b4028b5..9b6b2d6dd7bdf 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2699,7 +2699,7 @@ it.live("reports why a page failed to load while the tab shows the error page", ).pipe(Effect.provide(layer)), ); -it.live("preview_open rejects a desktop navigation that failed before automation attached", () => +it.effect("preview_open rejects a desktop navigation that failed before automation attached", () => Effect.scoped( Effect.gen(function* () { const { broker, tabId } = yield* ready; @@ -2707,18 +2707,23 @@ it.live("preview_open rejects a desktop navigation that failed before automation const channel = yield* DesktopChannel.DesktopBrowserChannel; desktopRendersNext = true; desktopPageUrl = "chrome-error://chromewebdata/"; - using _failure = vi.spyOn(channel, "loadFailure").mockReturnValue(certificateError); + const failure = { + ...certificateError, + url: "https://user:secret@device.local/?token=private#fragment", + }; + using _failure = vi.spyOn(channel, "loadFailure").mockReturnValue(failure); const error = yield* broker .invoke({ scope, operation: "open", - input: { url: certificateError.url, reuseExistingTab: false, open: false }, + input: { url: failure.url, reuseExistingTab: false, open: false }, }) .pipe(Effect.flip); expect(error).toMatchObject({ _tag: "PreviewAutomationExecutionError", reason: `Navigation to ${certificateError.url} failed: ${certificateError.description}`, }); + expect(JSON.stringify(error)).not.toMatch(/secret|private|fragment/); const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); expect(sessions.map((session) => session.tabId)).toEqual([tabId]); }), diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index ed53efd77a65e..4b7d9c2805bb1 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1627,10 +1627,20 @@ const make = Effect.gen(function* () { const assertNavigationSucceeded = (tab: ServerTab) => { const failure = loadErrorOf(tab); if (failure) { + const diagnosticUrl = URL.parse(failure.url); + if (diagnosticUrl) { + diagnosticUrl.username = ""; + diagnosticUrl.password = ""; + diagnosticUrl.search = ""; + diagnosticUrl.hash = ""; + } + const url = diagnosticUrl + ? ServerBrowserPage.withoutDataPayload(diagnosticUrl.href) + : "the requested page"; throw new ServerBrowserPage.ServerBrowserOperationError({ tag: "PreviewAutomationExecutionError", - message: `Navigation to ${failure.url} failed: ${failure.description}`, - detail: failure, + message: `Navigation to ${url} failed: ${failure.description}`, + detail: { ...failure, url }, }); } };