mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(browser): retry relay navigation after frame detach
Retry browser navigate once after transient frame-detached/target-closed errors by forcing a clean Playwright reconnect, so extension-relay sessions stay controllable across navigation swaps. Closes #29431
This commit is contained in:
committed by
Peter Steinberger
parent
376a52a5ba
commit
7c9d2c1d48
@@ -39,9 +39,40 @@ describe("pw-tools-core.snapshot navigate guard", () => {
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
url: "https://example.com",
|
||||
timeoutMs: 10,
|
||||
ssrfPolicy: { allowPrivateNetwork: true },
|
||||
});
|
||||
|
||||
expect(goto).toHaveBeenCalledWith("https://example.com", { timeout: 1000 });
|
||||
expect(result.url).toBe("https://example.com");
|
||||
});
|
||||
|
||||
it("reconnects and retries once when navigation detaches frame", async () => {
|
||||
const goto = vi
|
||||
.fn<(...args: unknown[]) => Promise<void>>()
|
||||
.mockRejectedValueOnce(new Error("page.goto: Frame has been detached"))
|
||||
.mockResolvedValueOnce(undefined);
|
||||
setPwToolsCoreCurrentPage({
|
||||
goto,
|
||||
url: vi.fn(() => "https://example.com/recovered"),
|
||||
});
|
||||
|
||||
const result = await mod.navigateViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "tab-1",
|
||||
url: "https://example.com/recovered",
|
||||
ssrfPolicy: { allowPrivateNetwork: true },
|
||||
});
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().getPageForTargetId).toHaveBeenCalledTimes(2);
|
||||
expect(getPwToolsCoreSessionMocks().forceDisconnectPlaywrightForTarget).toHaveBeenCalledTimes(
|
||||
1,
|
||||
);
|
||||
expect(getPwToolsCoreSessionMocks().forceDisconnectPlaywrightForTarget).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "tab-1",
|
||||
reason: "retry navigate after detached frame",
|
||||
});
|
||||
expect(goto).toHaveBeenCalledTimes(2);
|
||||
expect(result.url).toBe("https://example.com/recovered");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
} from "./pw-role-snapshot.js";
|
||||
import {
|
||||
ensurePageState,
|
||||
forceDisconnectPlaywrightForTarget,
|
||||
getPageForTargetId,
|
||||
storeRoleRefsForTarget,
|
||||
type WithSnapshotForAI,
|
||||
@@ -166,6 +167,19 @@ export async function navigateViaPlaywright(opts: {
|
||||
timeoutMs?: number;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
}): Promise<{ url: string }> {
|
||||
const isRetryableNavigateError = (err: unknown): boolean => {
|
||||
const msg =
|
||||
typeof err === "string"
|
||||
? err.toLowerCase()
|
||||
: err instanceof Error
|
||||
? err.message.toLowerCase()
|
||||
: "";
|
||||
return (
|
||||
msg.includes("frame has been detached") ||
|
||||
msg.includes("target page, context or browser has been closed")
|
||||
);
|
||||
};
|
||||
|
||||
const url = String(opts.url ?? "").trim();
|
||||
if (!url) {
|
||||
throw new Error("url is required");
|
||||
@@ -174,11 +188,26 @@ export async function navigateViaPlaywright(opts: {
|
||||
url,
|
||||
...withBrowserNavigationPolicy(opts.ssrfPolicy),
|
||||
});
|
||||
const page = await getPageForTargetId(opts);
|
||||
const timeout = Math.max(1000, Math.min(120_000, opts.timeoutMs ?? 20_000));
|
||||
let page = await getPageForTargetId(opts);
|
||||
ensurePageState(page);
|
||||
await page.goto(url, {
|
||||
timeout: Math.max(1000, Math.min(120_000, opts.timeoutMs ?? 20_000)),
|
||||
});
|
||||
try {
|
||||
await page.goto(url, { timeout });
|
||||
} catch (err) {
|
||||
if (!isRetryableNavigateError(err)) {
|
||||
throw err;
|
||||
}
|
||||
// Extension relays can briefly drop CDP during renderer swaps/navigation.
|
||||
// Force a clean reconnect, then retry once on the refreshed page handle.
|
||||
await forceDisconnectPlaywrightForTarget({
|
||||
cdpUrl: opts.cdpUrl,
|
||||
targetId: opts.targetId,
|
||||
reason: "retry navigate after detached frame",
|
||||
}).catch(() => {});
|
||||
page = await getPageForTargetId(opts);
|
||||
ensurePageState(page);
|
||||
await page.goto(url, { timeout });
|
||||
}
|
||||
const finalUrl = page.url();
|
||||
await assertBrowserNavigationResultAllowed({
|
||||
url: finalUrl,
|
||||
|
||||
@@ -22,7 +22,9 @@ const sessionMocks = vi.hoisted(() => ({
|
||||
return currentPage;
|
||||
}),
|
||||
ensurePageState: vi.fn(() => pageState),
|
||||
forceDisconnectPlaywrightForTarget: vi.fn(async () => {}),
|
||||
restoreRoleRefsForTarget: vi.fn(() => {}),
|
||||
storeRoleRefsForTarget: vi.fn(() => {}),
|
||||
refLocator: vi.fn(() => {
|
||||
if (!currentRefLocator) {
|
||||
throw new Error("missing locator");
|
||||
|
||||
Reference in New Issue
Block a user