diff --git a/docs/browser-observation.md b/docs/browser-observation.md index 3ea8b51012..6916f96e83 100644 --- a/docs/browser-observation.md +++ b/docs/browser-observation.md @@ -201,6 +201,11 @@ rea inspect-web-page http://127.0.0.1:9222 TARGET_ID \ --json ``` +Passive observation honors the requested duration using cancellable waits within +the runtime's timer range. Long windows stay pending across successive waits; +caller cancellation, target termination or leaving the allowed origins ends +collection early. `window_elapsed` identifies completion of the requested wait. + Static bundle analysis and a user-driven observation window are separate operations: ```bash diff --git a/src/browser/CdpObservationSession.ts b/src/browser/CdpObservationSession.ts index 61b65b3997..5374573644 100644 --- a/src/browser/CdpObservationSession.ts +++ b/src/browser/CdpObservationSession.ts @@ -11,6 +11,7 @@ import { CdpCaptureCompleteness } from "./CdpCaptureCompleteness.js"; import { authorizedMainFrame } from "./CdpAuthorizedMainFrame.js"; import { allowedSanitizedUrl, + delayWithCancellation, isHttpUrl, numberValue, recordValue, @@ -356,32 +357,38 @@ const waitForWindow = async ( signal?: AbortSignal, ): Promise => await new Promise((resolve, reject) => { + const timerController = new AbortController(); let settled = false; let removeEnd = (): void => {}; - const finish = (reason: EndReason): void => { + const settle = (complete: () => void): void => { if (settled) return; settled = true; - clearTimeout(timer); + timerController.abort(); signal?.removeEventListener("abort", onAbort); removeEnd(); - resolve(reason); + complete(); }; - const onAbort = (): void => { - if (settled) return; - settled = true; - clearTimeout(timer); - signal?.removeEventListener("abort", onAbort); - removeEnd(); - reject( - new AnalysisCancelledError("observe_web_session", { - cause: signal?.reason, - }), - ); - }; - const timer = setTimeout(() => finish("window_elapsed"), durationMs); - removeEnd = capture.onEnd(finish); + const finish = (reason: EndReason): void => settle(() => resolve(reason)); + const onAbort = (): void => + settle(() => { + reject( + new AnalysisCancelledError("observe_web_session", { + cause: signal?.reason, + }), + ); + }); signal?.addEventListener("abort", onAbort, { once: true }); if (signal?.aborted === true) onAbort(); + if (!settled) removeEnd = capture.onEnd(finish); + if (settled) return; + void delayWithCancellation( + durationMs, + "observe_web_session", + timerController.signal, + ).then( + () => finish("window_elapsed"), + (cause: unknown) => settle(() => reject(cause)), + ); }); const captureFinalUrl = async ( diff --git a/tests/boundary/browser/cdpBrowserProvider.timelineEvents.test.ts b/tests/boundary/browser/cdpBrowserProvider.timelineEvents.test.ts index 4a707c1ab7..89f4c77320 100644 --- a/tests/boundary/browser/cdpBrowserProvider.timelineEvents.test.ts +++ b/tests/boundary/browser/cdpBrowserProvider.timelineEvents.test.ts @@ -1,10 +1,128 @@ -import { describe, expect, it } from "vitest"; +import { getEventListeners } from "node:events"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js"; import { observeWebSessionInputSchema } from "../../../src/domain/browserSession.js"; import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js"; import { trackBrowser } from "./cdpBrowserProvider.support.js"; +afterEach(() => vi.restoreAllMocks()); + +it.each([2_147_483_648, 3_000_000_000])( + "keeps a long observation pending with a representable timer: %i", + async (duration) => { + const browser = await startFakeCdpBrowser(); + trackBrowser(browser); + const controller = new AbortController(); + const reason = { stop: "long observation" }; + const provider = new CdpBrowserProvider(); + const nativeTimeout = globalThis.setTimeout; + let scheduled!: (delay: number) => void; + const scheduledDelay = new Promise((resolve) => { + scheduled = resolve; + }); + vi.spyOn(globalThis, "setTimeout").mockImplementation( + (callback, delay, ...args) => { + if (delay === duration || delay === 2_147_483_647) scheduled(delay); + return nativeTimeout(callback, delay, ...args); + }, + ); + const pending = provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: duration, + }), + { signal: controller.signal }, + ); + const observedDelay = await scheduledDelay; + controller.abort(reason); + const result = await pending; + await provider.close(); + expect(observedDelay).toBe(2_147_483_647); + if (result.ok) throw new Error("Long window completed before cancellation"); + expect(result.error).toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(result.error.cause).toBe(reason); + expect(getEventListeners(controller.signal, "abort")).toHaveLength(0); + await expect.poll(() => browser.activeConnectionCount()).toBe(0); + }, +); + +it("does not retain a caller listener when the target ends before the wait starts", async () => { + const browser = await startFakeCdpBrowser({ + eventsBeforeReply: ["Page.setLifecycleEventsEnabled"], + commandEvents: ({ method, sessionId }) => + method === "Page.setLifecycleEventsEnabled" + ? [ + { + method: "Page.frameNavigated", + ...(sessionId === undefined ? {} : { sessionId }), + params: { + frame: { id: "frame-main", url: "https://outside.test/" }, + }, + }, + ] + : undefined, + }); + trackBrowser(browser); + const controller = new AbortController(); + const provider = new CdpBrowserProvider(); + const result = await provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 3_000_000_000, + }), + { signal: controller.signal }, + ); + if (!result.ok) throw result.error; + expect(result.value.window).toMatchObject({ + requested_ms: 3_000_000_000, + end_reason: "target_left_scope", + }); + expect(getEventListeners(controller.signal, "abort")).toHaveLength(0); + await expect.poll(() => browser.activeConnectionCount()).toBe(0); + await provider.close(); +}); + +it("preserves cancellation at the armed handshake without starting a window timer", async () => { + const browser = await startFakeCdpBrowser(); + trackBrowser(browser); + const controller = new AbortController(); + const reason = { stop: "at armed handshake" }; + const provider = new CdpBrowserProvider(); + const timers = vi.spyOn(globalThis, "setTimeout"); + const result = await provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 3_000_000_000, + }), + { + signal: controller.signal, + progress: { + async report(update) { + if (update.phase === "browser_observation" && update.completed === 1) + controller.abort(reason); + }, + }, + }, + ); + if (result.ok) throw new Error("Expected armed cancellation"); + expect(result.error.cause).toBe(reason); + expect( + timers.mock.calls.filter( + ([, delay]) => delay === 3_000_000_000 || delay === 2_147_483_647, + ), + ).toHaveLength(0); + expect(getEventListeners(controller.signal, "abort")).toHaveLength(0); + await expect.poll(() => browser.activeConnectionCount()).toBe(0); + await provider.close(); +}); + describe("CdpBrowserProvider navigation timeline", () => { it("returns every event beyond the former default collection cap", async () => { const additionalEvents = 2_001;