fix(browser): honor long passive observation windows (#5605)
Reuse the cancellable segmented wait and settle observation termination through one cleanup owner. Preserve caller cancellation and release timers/listeners across early target end. Fixes #3929.
This commit is contained in:
1 parent
90e433df06
commit
c3d7d0aafc
3 files changed
+148
-18
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<EndReason> =>
|
||||
await new Promise<EndReason>((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 (
|
||||
|
||||
@@ -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<number>((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;
|
||||
|
||||
Reference in new issue
Block a user