diff --git a/src/browser/PlaywrightScenarioBrowser.test.ts b/src/browser/PlaywrightScenarioBrowser.test.ts index 79e903975..ffd2dba95 100644 --- a/src/browser/PlaywrightScenarioBrowser.test.ts +++ b/src/browser/PlaywrightScenarioBrowser.test.ts @@ -164,7 +164,15 @@ it("keeps attached-target ownership after the cleanup response timeout", async ( cleanup: { reason: expect.stringContaining("timed out") }, }); await closeStarted.promise; - await vi.advanceTimersByTimeAsync(1_000); + let responseSettled = false; + void timedOutClose + .finally(() => { + responseSettled = true; + }) + .catch(() => {}); + await vi.advanceTimersByTimeAsync(29_999); + expect(responseSettled).toBe(false); + await vi.advanceTimersByTimeAsync(1); await timeoutExpectation; expect(settled).toEqual([]); diff --git a/src/browser/PlaywrightScenarioBrowser.ts b/src/browser/PlaywrightScenarioBrowser.ts index 615ad055e..3af9ade9c 100644 --- a/src/browser/PlaywrightScenarioBrowser.ts +++ b/src/browser/PlaywrightScenarioBrowser.ts @@ -11,6 +11,9 @@ import { BrowserObservationError } from "../domain/browserObservationError.js"; import { withPlaywrightExecutionBoundary } from "./PlaywrightExecutionBoundary.js"; const OPERATION = "capture_browser_scenario" as const; +// Closing a real Chrome persistent context can take several seconds even when +// it succeeds. The shorter operation deadline must not misreport cleanup. +const CLEANUP_CONFIRMATION_TIMEOUT_MS = 30_000; const throwIfScenarioCancelled = (signal: AbortSignal | undefined): void => { if (signal?.aborted === true) @@ -71,7 +74,11 @@ export class PlaywrightScenarioBrowserCleanupOwner { signal?: AbortSignal, ): Promise { try { - await withPlaywrightExecutionBoundary(() => cleanup, 1_000, signal); + await withPlaywrightExecutionBoundary( + () => cleanup, + CLEANUP_CONFIRMATION_TIMEOUT_MS, + signal, + ); } catch (cause: unknown) { throw new BrowserObservationError(OPERATION, "cleanup_failed", { cause, diff --git a/src/browser/PlaywrightScenarioEvents.sharedContext.test.ts b/src/browser/PlaywrightScenarioEvents.sharedContext.test.ts new file mode 100644 index 000000000..8e20a83ae --- /dev/null +++ b/src/browser/PlaywrightScenarioEvents.sharedContext.test.ts @@ -0,0 +1,130 @@ +import { EventEmitter } from "node:events"; +import type { Page } from "playwright-core"; +import { expect, it } from "vitest"; + +import { browserScenarioSchema } from "../domain/browserScenario.js"; +import { BrowserScenarioSecrets } from "./BrowserScenarioSecrets.js"; +import { PlaywrightScenarioEvents } from "./PlaywrightScenarioEvents.js"; + +it("omits a request without a frame in a shared context", () => { + const context = new EventEmitter(); + const page = new EventEmitter() as unknown as Page; + const scenario = browserScenarioSchema.parse({ + browser: { mode: "launch", executable_path: "/opt/chromium" }, + start_url: { url: "https://app.example.test/" }, + actions: [{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }], + capture: { events: ["network"] }, + }); + const secrets = BrowserScenarioSecrets.resolve(scenario, {}); + if (secrets === undefined) throw new Error("Expected resolved secrets"); + const events = new PlaywrightScenarioEvents({ + page, + context: context as never, + ownsContext: false, + enabled: new Set(["network"]), + secrets, + }); + const unknownRequest = { + frame: () => { + throw new Error("Frame for this navigation request is unavailable"); + }, + url: () => "https://unrelated.example.test/private", + }; + context.emit("request", unknownRequest); + expect(events.result().items).toEqual([]); + expect(events.limitations()).toContain( + "Shared-context network capture excludes requests without a known root or descendant Page frame.", + ); +}); + +it("detaches root and popup listeners when a scenario finishes", async () => { + const context = new EventEmitter(); + const page = new EventEmitter() as unknown as Page; + const popup = Object.assign(new EventEmitter(), { + url: () => "https://app.example.test/popup", + }); + const scenario = browserScenarioSchema.parse({ + browser: { mode: "launch", executable_path: "/opt/chromium" }, + start_url: { url: "https://app.example.test/" }, + actions: [{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }], + capture: { events: ["console", "popups"] }, + }); + const secrets = BrowserScenarioSecrets.resolve(scenario, {}); + if (secrets === undefined) throw new Error("Expected resolved secrets"); + const events = new PlaywrightScenarioEvents({ + page, + context: context as never, + ownsContext: false, + enabled: new Set(["console", "popups"]), + secrets, + }); + (page as unknown as EventEmitter).emit("popup", popup); + expect((page as unknown as EventEmitter).listenerCount("console")).toBe(1); + expect(popup.listenerCount("console")).toBe(1); + // Popup closure is recorded, then its page-owned listeners are released. + expect(popup.listenerCount("close")).toBe(2); + await events.finish(); + expect((page as unknown as EventEmitter).listenerCount("console")).toBe(0); + expect((page as unknown as EventEmitter).listenerCount("popup")).toBe(0); + expect(popup.listenerCount("console")).toBe(0); + expect(popup.listenerCount("popup")).toBe(0); + expect(popup.listenerCount("close")).toBe(0); +}); + +it("detaches worker and websocket listeners when a scenario finishes", async () => { + const context = new EventEmitter(); + const page = new EventEmitter() as unknown as Page; + const socket = Object.assign(new EventEmitter(), { + url: () => "wss://app.example.test/socket", + }); + const worker = Object.assign(new EventEmitter(), { + url: () => "https://app.example.test/worker.js", + }); + const scenario = browserScenarioSchema.parse({ + browser: { mode: "launch", executable_path: "/opt/chromium" }, + start_url: { url: "https://app.example.test/" }, + actions: [{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }], + capture: { events: ["websockets", "workers"] }, + }); + const secrets = BrowserScenarioSecrets.resolve(scenario, {}); + if (secrets === undefined) throw new Error("Expected resolved secrets"); + const events = new PlaywrightScenarioEvents({ + page, + context: context as never, + ownsContext: false, + enabled: new Set(["websockets", "workers"]), + secrets, + }); + (page as unknown as EventEmitter).emit("websocket", socket); + (page as unknown as EventEmitter).emit("worker", worker); + expect(socket.listenerCount("framesent")).toBe(1); + expect(worker.listenerCount("close")).toBe(1); + await events.finish(); + expect(socket.listenerCount("framesent")).toBe(0); + expect(socket.listenerCount("framereceived")).toBe(0); + expect(socket.listenerCount("close")).toBe(0); + expect(worker.listenerCount("close")).toBe(0); +}); + +it("detaches owned-context popup discovery when a scenario finishes", async () => { + const context = new EventEmitter(); + const page = new EventEmitter() as unknown as Page; + const scenario = browserScenarioSchema.parse({ + browser: { mode: "launch", executable_path: "/opt/chromium" }, + start_url: { url: "https://app.example.test/" }, + actions: [{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }], + capture: { events: ["console"] }, + }); + const secrets = BrowserScenarioSecrets.resolve(scenario, {}); + if (secrets === undefined) throw new Error("Expected resolved secrets"); + const events = new PlaywrightScenarioEvents({ + page, + context: context as never, + ownsContext: true, + enabled: new Set(["console"]), + secrets, + }); + expect(context.listenerCount("page")).toBe(1); + await events.finish(); + expect(context.listenerCount("page")).toBe(0); +}); diff --git a/src/browser/PlaywrightScenarioEvents.ts b/src/browser/PlaywrightScenarioEvents.ts index 18e1f0e3b..aee9db630 100644 --- a/src/browser/PlaywrightScenarioEvents.ts +++ b/src/browser/PlaywrightScenarioEvents.ts @@ -54,10 +54,11 @@ export class PlaywrightScenarioEvents { private readonly enabled: ReadonlySet; private readonly secrets: BrowserScenarioSecrets; private readonly ownsContext: boolean; - private readonly pages = new Set(); + private pages = new WeakSet(); private readonly incompleteFamilies = new Set(); private readonly network: PlaywrightScenarioNetwork | undefined; private stopNetwork: (() => void) | undefined; + private readonly stopListeners = new Map void>>(); private sealed = false; constructor(options: EventCaptureOptions) { @@ -72,10 +73,12 @@ export class PlaywrightScenarioEvents { this.secrets = options.secrets; this.ownsContext = options.ownsContext === true; this.pages.add(options.page); - if (this.ownsContext && this.enabled.size > 0) - (options.context ?? options.page.context()).on("page", (page) => - this.popup(page), - ); + if (this.ownsContext && this.enabled.size > 0) { + const context = options.context ?? options.page.context(); + const onPage = (page: Page) => this.popup(page); + context.on("page", onPage); + this.retainListener(context, () => context.off("page", onPage)); + } if (this.enabled.has("network")) { const context = options.context ?? options.page.context(); const collector = new PlaywrightScenarioNetwork({ @@ -138,6 +141,8 @@ export class PlaywrightScenarioEvents { async finish(): Promise { this.stopNetwork?.(); this.stopNetwork = undefined; + for (const owner of this.stopListeners.keys()) this.releaseListeners(owner); + this.pages = new WeakSet(); try { await this.network?.finish(); } finally { @@ -184,34 +189,81 @@ export class PlaywrightScenarioEvents { return this.sequence; } + private retainListener(owner: object, stop: () => void): void { + let stops = this.stopListeners.get(owner); + if (stops === undefined) { + stops = new Set(); + this.stopListeners.set(owner, stops); + } + stops.add(stop); + } + + private releaseListeners(owner: object): void { + const stops = this.stopListeners.get(owner); + this.stopListeners.delete(owner); + for (const stop of stops ?? []) stop(); + } + private observePage(page: Page): void { - if (this.enabled.has("console")) - page.on("console", (message) => this.console(message)); - if (this.enabled.has("page-errors")) - page.on("pageerror", (error) => { + if (this.enabled.size > 0) { + const onClose = () => { + this.releaseListeners(page); + }; + page.on("close", onClose); + this.retainListener(page, () => page.off("close", onClose)); + } + + if (this.enabled.has("console")) { + const onConsole = (message: ConsoleMessage) => this.console(message); + page.on("console", onConsole); + this.retainListener(page, () => page.off("console", onConsole)); + } + if (this.enabled.has("page-errors")) { + const onError = (error: Error) => { this.push({ kind: "page-error", message: this.secrets.redact(error.message), stack: error.stack === undefined ? null : this.secrets.redact(error.stack), }); - }); - if (this.enabled.has("websockets")) - page.on("websocket", (socket) => this.webSocket(socket)); - if (this.enabled.has("frames")) { - page.on("frameattached", (frame) => this.frame("frame-attached", frame)); - page.on("framedetached", (frame) => this.frame("frame-detached", frame)); - page.on("framenavigated", (frame) => - this.frame("frame-navigated", frame), - ); + }; + page.on("pageerror", onError); + this.retainListener(page, () => page.off("pageerror", onError)); + } + if (this.enabled.has("websockets")) { + const onWebSocket = (socket: WebSocket) => this.webSocket(socket); + page.on("websocket", onWebSocket); + this.retainListener(page, () => page.off("websocket", onWebSocket)); + } + if (this.enabled.has("frames")) { + const onAttached = (frame: Frame) => this.frame("frame-attached", frame); + const onDetached = (frame: Frame) => this.frame("frame-detached", frame); + const onNavigated = (frame: Frame) => + this.frame("frame-navigated", frame); + page.on("frameattached", onAttached); + page.on("framedetached", onDetached); + page.on("framenavigated", onNavigated); + this.retainListener(page, () => page.off("frameattached", onAttached)); + this.retainListener(page, () => page.off("framedetached", onDetached)); + this.retainListener(page, () => page.off("framenavigated", onNavigated)); + } + if (this.enabled.has("workers")) { + const onWorker = (worker: Worker) => this.worker(worker); + page.on("worker", onWorker); + this.retainListener(page, () => page.off("worker", onWorker)); } - if (this.enabled.has("workers")) - page.on("worker", (worker) => this.worker(worker)); // Any selected family needs descendant discovery in shared contexts. // Popup lifecycle records remain gated on their own selector below. - if (this.enabled.size > 0) page.on("popup", (popup) => this.popup(popup)); - if (this.enabled.has("downloads")) - page.on("download", (download) => this.download(download)); + if (this.enabled.size > 0) { + const onPopup = (popup: Page) => this.popup(popup); + page.on("popup", onPopup); + this.retainListener(page, () => page.off("popup", onPopup)); + } + if (this.enabled.has("downloads")) { + const onDownload = (download: Download) => this.download(download); + page.on("download", onDownload); + this.retainListener(page, () => page.off("download", onDownload)); + } } private safeUrl(value: string) { @@ -231,13 +283,20 @@ export class PlaywrightScenarioEvents { private webSocket(socket: WebSocket): void { const url = this.safeUrl(socket.url()); this.push({ kind: "websocket-opened", url }); - socket.on("framesent", ({ payload }) => - this.webSocketFrame("websocket-frame-sent", url, payload), - ); - socket.on("framereceived", ({ payload }) => - this.webSocketFrame("websocket-frame-received", url, payload), - ); - socket.on("close", () => this.push({ kind: "websocket-closed", url })); + const onSent = ({ payload }: { payload: string | Buffer }) => + this.webSocketFrame("websocket-frame-sent", url, payload); + const onReceived = ({ payload }: { payload: string | Buffer }) => + this.webSocketFrame("websocket-frame-received", url, payload); + const onClose = () => { + this.push({ kind: "websocket-closed", url }); + this.releaseListeners(socket); + }; + socket.on("framesent", onSent); + socket.on("framereceived", onReceived); + socket.on("close", onClose); + this.retainListener(socket, () => socket.off("framesent", onSent)); + this.retainListener(socket, () => socket.off("framereceived", onReceived)); + this.retainListener(socket, () => socket.off("close", onClose)); } private webSocketFrame( @@ -286,7 +345,12 @@ export class PlaywrightScenarioEvents { name: null, }; this.push({ kind: "worker-created", ...details }); - worker.on("close", () => this.push({ kind: "worker-closed", ...details })); + const onClose = () => { + this.push({ kind: "worker-closed", ...details }); + this.releaseListeners(worker); + }; + worker.on("close", onClose); + this.retainListener(worker, () => worker.off("close", onClose)); } private popup(page: Page): void { @@ -305,13 +369,14 @@ export class PlaywrightScenarioEvents { }; if (this.enabled.has("popups")) { this.push({ kind: "popup-opened", ...opened }); - page.on("close", () => + const onClose = () => this.push({ kind: "popup-closed", url: page.url() === "" ? null : this.safeUrl(page.url()), name: null, - }), - ); + }); + page.on("close", onClose); + this.retainListener(page, () => page.off("close", onClose)); } this.observePage(page); } diff --git a/tests/process-global/browserScenarioCleanup.test.ts b/tests/process-global/browserScenarioCleanup.test.ts index b2acfd02e..498940e53 100644 --- a/tests/process-global/browserScenarioCleanup.test.ts +++ b/tests/process-global/browserScenarioCleanup.test.ts @@ -1,4 +1,5 @@ import { Cli } from "incur"; +import { resolve } from "node:path"; import { expect, it, vi } from "vitest"; import type { BrowserScenarioCapturePort } from "../../src/application/BrowserScenarioCapturePort.js"; @@ -32,7 +33,7 @@ vi.mock("../../src/composition/browserScenario.js", () => ({ })); const scenario = browserScenarioSchema.parse({ - browser: { mode: "launch", executable_path: "/fixture/chromium" }, + browser: { mode: "launch", executable_path: resolve("/fixture/chromium") }, start_url: { url: "https://app.example.test/" }, actions: [{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }], storage: {}, @@ -111,6 +112,22 @@ class RetrySession implements BrowserScenarioSessionPort { } } +it("waits for a slow but successful browser close before removing its profile", async () => { + const stages: string[] = []; + const cleanup = new PlaywrightScenarioBrowserCleanupOwner({ + closeBrowser: async () => { + await new Promise((resolve) => setTimeout(resolve, 1_250)); + stages.push("browser-closed"); + }, + removeProfile: async () => { + stages.push("profile-removed"); + }, + }); + + await cleanup.close(); + expect(stages).toEqual(["browser-closed", "profile-removed"]); +}); + const serveScenario = async () => { const cli = Cli.create("browser-cleanup-test", { sync: false }); registerBrowserScenarioCommands(cli, silentLogger, {});