fix: browser remote tab ops harden (#1057) (thanks @mukhtharcm)
This commit is contained in:
@@ -394,9 +394,7 @@ export async function closePlaywrightBrowserConnection(): Promise<void> {
|
||||
* List all pages/tabs from the persistent Playwright connection.
|
||||
* Used for remote profiles where HTTP-based /json/list is ephemeral.
|
||||
*/
|
||||
export async function listPagesViaPlaywright(opts: {
|
||||
cdpUrl: string;
|
||||
}): Promise<
|
||||
export async function listPagesViaPlaywright(opts: { cdpUrl: string }): Promise<
|
||||
Array<{
|
||||
targetId: string;
|
||||
title: string;
|
||||
@@ -432,25 +430,15 @@ export async function listPagesViaPlaywright(opts: {
|
||||
* Used for remote profiles where HTTP-based /json/new is ephemeral.
|
||||
* Returns the new page's targetId and metadata.
|
||||
*/
|
||||
export async function createPageViaPlaywright(opts: {
|
||||
cdpUrl: string;
|
||||
url: string;
|
||||
}): Promise<{
|
||||
export async function createPageViaPlaywright(opts: { cdpUrl: string; url: string }): Promise<{
|
||||
targetId: string;
|
||||
title: string;
|
||||
url: string;
|
||||
type: string;
|
||||
}> {
|
||||
const { browser } = await connectBrowser(opts.cdpUrl);
|
||||
const contexts = browser.contexts();
|
||||
// Use the first context if available, otherwise this is a fresh connection
|
||||
// and we need to use the default context that Browserless provides
|
||||
let context = contexts[0];
|
||||
if (!context) {
|
||||
// For Browserless over CDP, there should be at least one context
|
||||
// If not, we can try accessing pages directly from contexts
|
||||
throw new Error("No browser context available for creating a new page");
|
||||
}
|
||||
const context = browser.contexts()[0] ?? (await browser.newContext());
|
||||
ensureContextState(context);
|
||||
|
||||
const page = await context.newPage();
|
||||
ensurePageState(page);
|
||||
|
||||
197
src/browser/server-context.remote-tab-ops.test.ts
Normal file
197
src/browser/server-context.remote-tab-ops.test.ts
Normal file
@@ -0,0 +1,197 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import type { BrowserServerState } from "./server-context.js";
|
||||
|
||||
vi.mock("./chrome.js", () => ({
|
||||
isChromeCdpReady: vi.fn(async () => true),
|
||||
isChromeReachable: vi.fn(async () => true),
|
||||
launchClawdChrome: vi.fn(async () => {
|
||||
throw new Error("unexpected launch");
|
||||
}),
|
||||
resolveClawdUserDataDir: vi.fn(() => "/tmp/clawd"),
|
||||
stopClawdChrome: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
function makeState(
|
||||
profile: "remote" | "clawd",
|
||||
): BrowserServerState & { profiles: Map<string, { lastTargetId?: string | null }> } {
|
||||
return {
|
||||
// biome-ignore lint/suspicious/noExplicitAny: test stub
|
||||
server: null as any,
|
||||
port: 0,
|
||||
resolved: {
|
||||
enabled: true,
|
||||
controlUrl: "http://127.0.0.1:18791",
|
||||
controlHost: "127.0.0.1",
|
||||
controlPort: 18791,
|
||||
cdpProtocol: profile === "remote" ? "https" : "http",
|
||||
cdpHost: profile === "remote" ? "browserless.example" : "127.0.0.1",
|
||||
cdpIsLoopback: profile !== "remote",
|
||||
remoteCdpTimeoutMs: 1500,
|
||||
remoteCdpHandshakeTimeoutMs: 3000,
|
||||
color: "#FF4500",
|
||||
headless: true,
|
||||
noSandbox: false,
|
||||
attachOnly: false,
|
||||
defaultProfile: profile,
|
||||
profiles: {
|
||||
remote: {
|
||||
cdpUrl: "https://browserless.example/chrome?token=abc",
|
||||
cdpPort: 443,
|
||||
color: "#00AA00",
|
||||
},
|
||||
clawd: { cdpPort: 18800, color: "#FF4500" },
|
||||
},
|
||||
},
|
||||
profiles: new Map(),
|
||||
};
|
||||
}
|
||||
|
||||
describe("browser server-context remote profile tab operations", () => {
|
||||
it("uses Playwright tab operations when available", async () => {
|
||||
vi.resetModules();
|
||||
const listPagesViaPlaywright = vi.fn(async () => [
|
||||
{ targetId: "T1", title: "Tab 1", url: "https://a.example", type: "page" },
|
||||
]);
|
||||
const createPageViaPlaywright = vi.fn(async () => ({
|
||||
targetId: "T2",
|
||||
title: "Tab 2",
|
||||
url: "https://b.example",
|
||||
type: "page",
|
||||
}));
|
||||
const closePageByTargetIdViaPlaywright = vi.fn(async () => {});
|
||||
|
||||
vi.doMock("./pw-ai.js", () => ({
|
||||
listPagesViaPlaywright,
|
||||
createPageViaPlaywright,
|
||||
closePageByTargetIdViaPlaywright,
|
||||
}));
|
||||
|
||||
const fetchMock = vi.fn(async () => {
|
||||
throw new Error("unexpected fetch");
|
||||
});
|
||||
// @ts-expect-error test override
|
||||
global.fetch = fetchMock;
|
||||
|
||||
const { createBrowserRouteContext } = await import("./server-context.js");
|
||||
const state = makeState("remote");
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const remote = ctx.forProfile("remote");
|
||||
|
||||
const tabs = await remote.listTabs();
|
||||
expect(tabs.map((t) => t.targetId)).toEqual(["T1"]);
|
||||
|
||||
const opened = await remote.openTab("https://b.example");
|
||||
expect(opened.targetId).toBe("T2");
|
||||
expect(state.profiles.get("remote")?.lastTargetId).toBe("T2");
|
||||
|
||||
await remote.closeTab("T1");
|
||||
expect(closePageByTargetIdViaPlaywright).toHaveBeenCalledWith({
|
||||
cdpUrl: "https://browserless.example/chrome?token=abc",
|
||||
targetId: "T1",
|
||||
});
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not swallow Playwright runtime errors for remote profiles", async () => {
|
||||
vi.resetModules();
|
||||
vi.doMock("./pw-ai.js", () => ({
|
||||
listPagesViaPlaywright: vi.fn(async () => {
|
||||
throw new Error("boom");
|
||||
}),
|
||||
}));
|
||||
|
||||
const fetchMock = vi.fn(async () => {
|
||||
throw new Error("unexpected fetch");
|
||||
});
|
||||
// @ts-expect-error test override
|
||||
global.fetch = fetchMock;
|
||||
|
||||
const { createBrowserRouteContext } = await import("./server-context.js");
|
||||
const state = makeState("remote");
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const remote = ctx.forProfile("remote");
|
||||
|
||||
await expect(remote.listTabs()).rejects.toThrow(/boom/);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls back to /json/list when Playwright is not available", async () => {
|
||||
vi.resetModules();
|
||||
vi.doMock("./pw-ai.js", () => ({
|
||||
listPagesViaPlaywright: undefined,
|
||||
createPageViaPlaywright: undefined,
|
||||
closePageByTargetIdViaPlaywright: undefined,
|
||||
}));
|
||||
|
||||
const fetchMock = vi.fn(async (url: unknown) => {
|
||||
const u = String(url);
|
||||
if (!u.includes("/json/list")) throw new Error(`unexpected fetch: ${u}`);
|
||||
return {
|
||||
ok: true,
|
||||
json: async () => [
|
||||
{
|
||||
id: "T1",
|
||||
title: "Tab 1",
|
||||
url: "https://a.example",
|
||||
webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1",
|
||||
type: "page",
|
||||
},
|
||||
],
|
||||
} as unknown as Response;
|
||||
});
|
||||
// @ts-expect-error test override
|
||||
global.fetch = fetchMock;
|
||||
|
||||
const { createBrowserRouteContext } = await import("./server-context.js");
|
||||
const state = makeState("remote");
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const remote = ctx.forProfile("remote");
|
||||
|
||||
const tabs = await remote.listTabs();
|
||||
expect(tabs.map((t) => t.targetId)).toEqual(["T1"]);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("browser server-context tab selection state", () => {
|
||||
it("updates lastTargetId when openTab is created via CDP", async () => {
|
||||
vi.resetModules();
|
||||
vi.doUnmock("./pw-ai.js");
|
||||
vi.doMock("./cdp.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("./cdp.js")>("./cdp.js");
|
||||
return {
|
||||
...actual,
|
||||
createTargetViaCdp: vi.fn(async () => ({ targetId: "CREATED" })),
|
||||
};
|
||||
});
|
||||
|
||||
const fetchMock = vi.fn(async (url: unknown) => {
|
||||
const u = String(url);
|
||||
if (!u.includes("/json/list")) throw new Error(`unexpected fetch: ${u}`);
|
||||
return {
|
||||
ok: true,
|
||||
json: async () => [
|
||||
{
|
||||
id: "CREATED",
|
||||
title: "New Tab",
|
||||
url: "https://created.example",
|
||||
webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/CREATED",
|
||||
type: "page",
|
||||
},
|
||||
],
|
||||
} as unknown as Response;
|
||||
});
|
||||
// @ts-expect-error test override
|
||||
global.fetch = fetchMock;
|
||||
|
||||
const { createBrowserRouteContext } = await import("./server-context.js");
|
||||
const state = makeState("clawd");
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const clawd = ctx.forProfile("clawd");
|
||||
|
||||
const opened = await clawd.openTab("https://created.example");
|
||||
expect(opened.targetId).toBe("CREATED");
|
||||
expect(state.profiles.get("clawd")?.lastTargetId).toBe("CREATED");
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,7 @@
|
||||
import fs from "node:fs";
|
||||
|
||||
import { extractErrorCode, formatErrorMessage } from "../infra/errors.js";
|
||||
|
||||
import { appendCdpPath, createTargetViaCdp, getHeadersWithAuth, normalizeCdpWsUrl } from "./cdp.js";
|
||||
import {
|
||||
isChromeCdpReady,
|
||||
@@ -25,6 +27,36 @@ import {
|
||||
import { resolveTargetIdFromTabs } from "./target-id.js";
|
||||
import { movePathToTrash } from "./trash.js";
|
||||
|
||||
type PwAiModule = typeof import("./pw-ai.js");
|
||||
|
||||
let pwAiModule: Promise<PwAiModule | null> | null = null;
|
||||
|
||||
function isModuleNotFoundError(err: unknown): boolean {
|
||||
const code = extractErrorCode(err);
|
||||
if (code === "ERR_MODULE_NOT_FOUND") return true;
|
||||
const msg = formatErrorMessage(err);
|
||||
return (
|
||||
msg.includes("Cannot find module") ||
|
||||
msg.includes("Cannot find package") ||
|
||||
msg.includes("Failed to resolve import") ||
|
||||
msg.includes("Failed to resolve entry for package") ||
|
||||
msg.includes("Failed to load url")
|
||||
);
|
||||
}
|
||||
|
||||
async function getPwAiModule(): Promise<PwAiModule | null> {
|
||||
if (pwAiModule) return pwAiModule;
|
||||
pwAiModule = (async () => {
|
||||
try {
|
||||
return await import("./pw-ai.js");
|
||||
} catch (err) {
|
||||
if (isModuleNotFoundError(err)) return null;
|
||||
throw err;
|
||||
}
|
||||
})();
|
||||
return pwAiModule;
|
||||
}
|
||||
|
||||
export type {
|
||||
BrowserRouteContext,
|
||||
BrowserServerState,
|
||||
@@ -102,20 +134,19 @@ function createProfileContext(
|
||||
const listTabs = async (): Promise<BrowserTab[]> => {
|
||||
// For remote profiles, use Playwright's persistent connection to avoid ephemeral sessions
|
||||
if (!profile.cdpIsLoopback) {
|
||||
try {
|
||||
const mod = await import("./pw-ai.js");
|
||||
const pages = await mod.listPagesViaPlaywright({ cdpUrl: profile.cdpUrl });
|
||||
const mod = await getPwAiModule();
|
||||
const listPagesViaPlaywright = (mod as Partial<PwAiModule> | null)?.listPagesViaPlaywright;
|
||||
if (typeof listPagesViaPlaywright === "function") {
|
||||
const pages = await listPagesViaPlaywright({ cdpUrl: profile.cdpUrl });
|
||||
return pages.map((p) => ({
|
||||
targetId: p.targetId,
|
||||
title: p.title,
|
||||
url: p.url,
|
||||
type: p.type,
|
||||
}));
|
||||
} catch {
|
||||
// Fall back to HTTP-based listing if Playwright is not available
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
const raw = await fetchJson<
|
||||
Array<{
|
||||
id?: string;
|
||||
@@ -140,9 +171,10 @@ function createProfileContext(
|
||||
// For remote profiles, use Playwright's persistent connection to create tabs
|
||||
// This ensures the tab persists beyond a single request
|
||||
if (!profile.cdpIsLoopback) {
|
||||
try {
|
||||
const mod = await import("./pw-ai.js");
|
||||
const page = await mod.createPageViaPlaywright({ cdpUrl: profile.cdpUrl, url });
|
||||
const mod = await getPwAiModule();
|
||||
const createPageViaPlaywright = (mod as Partial<PwAiModule> | null)?.createPageViaPlaywright;
|
||||
if (typeof createPageViaPlaywright === "function") {
|
||||
const page = await createPageViaPlaywright({ cdpUrl: profile.cdpUrl, url });
|
||||
const profileState = getProfileState();
|
||||
profileState.lastTargetId = page.targetId;
|
||||
return {
|
||||
@@ -151,16 +183,9 @@ function createProfileContext(
|
||||
url: page.url,
|
||||
type: page.type,
|
||||
};
|
||||
} catch (err) {
|
||||
// Fall back to HTTP-based tab creation if Playwright is not available
|
||||
// (though it will likely be ephemeral for remote profiles)
|
||||
if (String(err).includes("No browser context")) {
|
||||
// This is a real error, not a missing Playwright issue
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
const createdViaCdp = await createTargetViaCdp({
|
||||
cdpUrl: profile.cdpUrl,
|
||||
url,
|
||||
@@ -169,6 +194,8 @@ function createProfileContext(
|
||||
.catch(() => null);
|
||||
|
||||
if (createdViaCdp) {
|
||||
const profileState = getProfileState();
|
||||
profileState.lastTargetId = createdViaCdp;
|
||||
const deadline = Date.now() + 2000;
|
||||
while (Date.now() < deadline) {
|
||||
const tabs = await listTabs().catch(() => [] as BrowserTab[]);
|
||||
@@ -363,9 +390,10 @@ function createProfileContext(
|
||||
const tabs = await listTabs();
|
||||
// For remote profiles using Playwright's persistent connection, we don't need wsUrl
|
||||
// because we access pages directly through Playwright, not via individual WebSocket URLs.
|
||||
const candidates = profile.driver === "extension" || !profile.cdpIsLoopback
|
||||
? tabs
|
||||
: tabs.filter((t) => Boolean(t.wsUrl));
|
||||
const candidates =
|
||||
profile.driver === "extension" || !profile.cdpIsLoopback
|
||||
? tabs
|
||||
: tabs.filter((t) => Boolean(t.wsUrl));
|
||||
|
||||
const resolveById = (raw: string) => {
|
||||
const resolved = resolveTargetIdFromTabs(raw, candidates);
|
||||
@@ -423,21 +451,21 @@ function createProfileContext(
|
||||
}
|
||||
throw new Error("tab not found");
|
||||
}
|
||||
|
||||
|
||||
// For remote profiles, use Playwright's persistent connection to close tabs
|
||||
if (!profile.cdpIsLoopback) {
|
||||
try {
|
||||
const mod = await import("./pw-ai.js");
|
||||
await mod.closePageByTargetIdViaPlaywright({
|
||||
const mod = await getPwAiModule();
|
||||
const closePageByTargetIdViaPlaywright = (mod as Partial<PwAiModule> | null)
|
||||
?.closePageByTargetIdViaPlaywright;
|
||||
if (typeof closePageByTargetIdViaPlaywright === "function") {
|
||||
await closePageByTargetIdViaPlaywright({
|
||||
cdpUrl: profile.cdpUrl,
|
||||
targetId: resolved.targetId,
|
||||
});
|
||||
return;
|
||||
} catch {
|
||||
// Fall back to HTTP-based close if Playwright is not available
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
await fetchOk(appendCdpPath(profile.cdpUrl, `/json/close/${resolved.targetId}`));
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user