fix(browser): release scenario listeners and allow bounded Chrome close (#2441)
* fix(browser): release scenario event listeners on finish * fix(browser): allow Chrome cleanup to complete within bounded deadline * style(browser): format cleanup deadline regression * test(browser): expect popup cleanup listener --------- Co-authored-by: LcokHcak <268763621+LcokHcak@users.noreply.github.com> Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
LcokHcak
GitHub
parent
9b08c2b1cb
commit
c9982ad8bf
|
|
@ -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([]);
|
||||
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
|
@ -54,10 +54,11 @@ export class PlaywrightScenarioEvents {
|
|||
private readonly enabled: ReadonlySet<EventName>;
|
||||
private readonly secrets: BrowserScenarioSecrets;
|
||||
private readonly ownsContext: boolean;
|
||||
private readonly pages = new Set<Page>();
|
||||
private pages = new WeakSet<Page>();
|
||||
private readonly incompleteFamilies = new Set<EventName>();
|
||||
private readonly network: PlaywrightScenarioNetwork | undefined;
|
||||
private stopNetwork: (() => void) | undefined;
|
||||
private readonly stopListeners = new Map<object, Set<() => 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<void> {
|
||||
this.stopNetwork?.();
|
||||
this.stopNetwork = undefined;
|
||||
for (const owner of this.stopListeners.keys()) this.releaseListeners(owner);
|
||||
this.pages = new WeakSet<Page>();
|
||||
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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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, {});
|
||||
|
|
|
|||
Loading…
Reference in New Issue