diff --git a/src/browser/pw-tools-core.interactions.ts b/src/browser/pw-tools-core.interactions.ts index f3eec30c1d1..852b11bb6dc 100644 --- a/src/browser/pw-tools-core.interactions.ts +++ b/src/browser/pw-tools-core.interactions.ts @@ -10,14 +10,44 @@ import { } from "./pw-session.js"; import { normalizeTimeoutMs, requireRef, toAIFriendlyError } from "./pw-tools-core.shared.js"; +type TargetOpts = { + cdpUrl: string; + targetId?: string; +}; + +async function getRestoredPageForTarget(opts: TargetOpts) { + const page = await getPageForTargetId(opts); + ensurePageState(page); + restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + return page; +} + +function resolveInteractionTimeoutMs(timeoutMs?: number): number { + return Math.max(500, Math.min(60_000, Math.floor(timeoutMs ?? 8000))); +} + +async function awaitEvalWithAbort( + evalPromise: Promise, + abortPromise?: Promise, +): Promise { + if (!abortPromise) { + return await evalPromise; + } + try { + return await Promise.race([evalPromise, abortPromise]); + } catch (err) { + // If abort wins the race, evaluate may reject later; avoid unhandled rejections. + void evalPromise.catch(() => {}); + throw err; + } +} + export async function highlightViaPlaywright(opts: { cdpUrl: string; targetId?: string; ref: string; }): Promise { - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); const ref = requireRef(opts.ref); try { await refLocator(page, ref).highlight(); @@ -35,15 +65,10 @@ export async function clickViaPlaywright(opts: { modifiers?: Array<"Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift">; timeoutMs?: number; }): Promise { - const page = await getPageForTargetId({ - cdpUrl: opts.cdpUrl, - targetId: opts.targetId, - }); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); const ref = requireRef(opts.ref); const locator = refLocator(page, ref); - const timeout = Math.max(500, Math.min(60_000, Math.floor(opts.timeoutMs ?? 8000))); + const timeout = resolveInteractionTimeoutMs(opts.timeoutMs); try { if (opts.doubleClick) { await locator.dblclick({ @@ -70,12 +95,10 @@ export async function hoverViaPlaywright(opts: { timeoutMs?: number; }): Promise { const ref = requireRef(opts.ref); - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); try { await refLocator(page, ref).hover({ - timeout: Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)), + timeout: resolveInteractionTimeoutMs(opts.timeoutMs), }); } catch (err) { throw toAIFriendlyError(err, ref); @@ -94,12 +117,10 @@ export async function dragViaPlaywright(opts: { if (!startRef || !endRef) { throw new Error("startRef and endRef are required"); } - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); try { await refLocator(page, startRef).dragTo(refLocator(page, endRef), { - timeout: Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)), + timeout: resolveInteractionTimeoutMs(opts.timeoutMs), }); } catch (err) { throw toAIFriendlyError(err, `${startRef} -> ${endRef}`); @@ -117,12 +138,10 @@ export async function selectOptionViaPlaywright(opts: { if (!opts.values?.length) { throw new Error("values are required"); } - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); try { await refLocator(page, ref).selectOption(opts.values, { - timeout: Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)), + timeout: resolveInteractionTimeoutMs(opts.timeoutMs), }); } catch (err) { throw toAIFriendlyError(err, ref); @@ -156,12 +175,10 @@ export async function typeViaPlaywright(opts: { timeoutMs?: number; }): Promise { const text = String(opts.text ?? ""); - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); const ref = requireRef(opts.ref); const locator = refLocator(page, ref); - const timeout = Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)); + const timeout = resolveInteractionTimeoutMs(opts.timeoutMs); try { if (opts.slowly) { await locator.click({ timeout }); @@ -183,10 +200,8 @@ export async function fillFormViaPlaywright(opts: { fields: BrowserFormField[]; timeoutMs?: number; }): Promise { - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); - const timeout = Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)); + const page = await getRestoredPageForTarget(opts); + const timeout = resolveInteractionTimeoutMs(opts.timeoutMs); for (const field of opts.fields) { const ref = field.ref.trim(); const type = (field.type || DEFAULT_FILL_FIELD_TYPE).trim() || DEFAULT_FILL_FIELD_TYPE; @@ -231,9 +246,7 @@ export async function evaluateViaPlaywright(opts: { if (!fnText) { throw new Error("function is required"); } - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); // Clamp evaluate timeout to prevent permanently blocking Playwright's command queue. // Without this, a long-running async evaluate blocks all subsequent page operations // because Playwright serializes CDP commands per page. @@ -313,17 +326,7 @@ export async function evaluateViaPlaywright(opts: { fnBody: fnText, timeoutMs: evaluateTimeout, }); - if (!abortPromise) { - return await evalPromise; - } - try { - return await Promise.race([evalPromise, abortPromise]); - } catch (err) { - // If abort wins the race, the underlying evaluate may reject later; ensure we don't - // surface it as an unhandled rejection. - void evalPromise.catch(() => {}); - throw err; - } + return await awaitEvalWithAbort(evalPromise, abortPromise); } // eslint-disable-next-line @typescript-eslint/no-implied-eval -- required for browser-context eval @@ -353,15 +356,7 @@ export async function evaluateViaPlaywright(opts: { fnBody: fnText, timeoutMs: evaluateTimeout, }); - if (!abortPromise) { - return await evalPromise; - } - try { - return await Promise.race([evalPromise, abortPromise]); - } catch (err) { - void evalPromise.catch(() => {}); - throw err; - } + return await awaitEvalWithAbort(evalPromise, abortPromise); } finally { if (signal && abortListener) { signal.removeEventListener("abort", abortListener); @@ -375,9 +370,7 @@ export async function scrollIntoViewViaPlaywright(opts: { ref: string; timeoutMs?: number; }): Promise { - const page = await getPageForTargetId(opts); - ensurePageState(page); - restoreRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId: opts.targetId, page }); + const page = await getRestoredPageForTarget(opts); const timeout = normalizeTimeoutMs(opts.timeoutMs, 20_000); const ref = requireRef(opts.ref);