From 6f196815519efad64c2b6d71cbd73a8cb5ffee96 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 09:26:29 +0800 Subject: [PATCH] fix(browser): type DOM listener, metrics and emulation CDP replies (#10641) * fix(browser): type CDP command replies and parse frame trees once CdpConnection.send is typed by method name. A command registered in src/browser/protocol/CdpCommands.ts has its parameters typed from the protocol table Playwright ships and its reply parsed once by its own result schema; a mismatch rejects that command with CdpMalformedReply (protocol_error, command, JSON pointer) without poisoning the connection. Playwright storage sessions admit replies through the same table. Page.getFrameTree and Page.getResourceTree are the first registered commands. Their one parser keeps per-frame facts tolerant (malformed child entries remain explicit omissions) and replaces the per-site readers: observeMainFrameUrl, mainFrameUrl, walkFrameTrees, frameTreeResources, runtimeFrameTreeSchema and the scenario storage frame schema. A reply without a frame tree used to be retried for two seconds and reported as target_not_allowed; it is now one protocol_error. A guard test counts literal CDP command names without a result schema per source file; the baseline can only shrink. * fix(browser): consume parsed CDP facts across boundaries * refactor(browser): register target, storage and acknowledgement CDP replies Register result schemas for Target.attachToTarget, Target.getTargetInfo, Storage.getStorageKey, Page.addScriptToEvaluateOnNewDocument, Debugger.setBreakpointByUrl, Debugger.evaluateOnCallFrame and fourteen commands whose reply REA reads only as success. Their callers use the parsed reply; the per-site readers are removed (attachedSessionId and the scenario storage and service-worker zod schemas, which threw raw ZodError). Playwright's Target.getTargetInfo in scenario attach now goes through the same admission. The untyped-command baseline drops from 96 to 48. * fix(browser): report malformed script source and coverage replies as protocol errors * fix(browser): type DOM listener, metrics and emulation CDP replies * fix(browser): report malformed script source and coverage replies as protocol errors (#10638) --- .../CdpCaptureDocuments.baseUrl.test.ts | 2 - src/browser/CdpCaptureDomNamespaces.ts | 26 +++------ src/browser/CdpObservationSession.ts | 14 ++--- src/browser/CdpPageCapture.ts | 4 +- src/browser/execution/CdpEventListeners.ts | 57 +++++++------------ src/browser/execution/CdpListenerSelector.ts | 3 +- src/browser/execution/CdpRuntimeProtocol.ts | 14 ----- src/browser/protocol/CdpCommands.ts | 42 ++++++++++++++ .../CdpCommands.untyped-baseline.json | 21 ------- .../browser/cdpDomSvgIdentity.test.ts | 15 ++++- tests/boundary/browser/cdpWebRuntime.test.ts | 30 ++++++++++ tests/fixtures/webRuntime.ts | 4 +- 12 files changed, 129 insertions(+), 103 deletions(-) diff --git a/src/browser/CdpCaptureDocuments.baseUrl.test.ts b/src/browser/CdpCaptureDocuments.baseUrl.test.ts index 71adb9f679..642bb7837b 100644 --- a/src/browser/CdpCaptureDocuments.baseUrl.test.ts +++ b/src/browser/CdpCaptureDocuments.baseUrl.test.ts @@ -472,11 +472,9 @@ describe("browser SVG identity", () => { ).toBe(false); expect(capturedSvgNodeKind(query, { node })).toBe(false); for (const invalid of [ - {}, { node: { ...node, backendNodeId: 43, isSVG: true } }, { node: { ...node, nodeName: "A", isSVG: true } }, { node: { ...node, nodeType: 3, isSVG: true } }, - { node: { ...node, isSVG: "true" } }, ]) expect(capturedSvgNodeKind(query, invalid)).toBeUndefined(); }); diff --git a/src/browser/CdpCaptureDomNamespaces.ts b/src/browser/CdpCaptureDomNamespaces.ts index ab329a48a7..e06c963c63 100644 --- a/src/browser/CdpCaptureDomNamespaces.ts +++ b/src/browser/CdpCaptureDomNamespaces.ts @@ -1,4 +1,4 @@ -import { recordValue } from "./CdpCaptureValues.js"; +import type { CdpCommandResult } from "./protocol/CdpCommands.js"; import type { SnapshotDocumentNodes, SnapshotAttributes, @@ -33,22 +33,14 @@ export interface DomSvgNodeQuery { /** Bind the browser's SVG flag to the exact snapshotted element. */ export const capturedSvgNodeKind = ( query: DomSvgNodeQuery, - result: unknown, -): boolean | undefined => { - const node = recordValue(recordValue(result)?.node); - if ( - node?.backendNodeId !== query.backendNodeId || - node.nodeName !== query.nodeName || - node.nodeType !== 1 - ) - return undefined; - // CDP omits isSVG for non-SVG nodes. Other values are malformed, not false. - return node.isSVG === undefined - ? false - : typeof node.isSVG === "boolean" - ? node.isSVG - : undefined; -}; + { node }: CdpCommandResult<"DOM.describeNode">, +): boolean | undefined => + node.backendNodeId !== query.backendNodeId || + node.nodeName !== query.nodeName || + node.nodeType !== 1 + ? undefined + : // CDP omits isSVG for non-SVG nodes. + (node.isSVG ?? false); interface DomXmlElement { readonly svgElement: boolean; diff --git a/src/browser/CdpObservationSession.ts b/src/browser/CdpObservationSession.ts index 9f79856459..226d46f6a1 100644 --- a/src/browser/CdpObservationSession.ts +++ b/src/browser/CdpObservationSession.ts @@ -20,7 +20,6 @@ import { isHttpUrl, numberValue, recordValue, - recordsValue, cdpStringValue, type UnknownRecord, } from "./CdpCaptureValues.js"; @@ -32,6 +31,7 @@ import { selectCdpFrameIdentity, cdpFrameEventFacts, } from "./CdpFrameIdentity.js"; +import type { CdpCommandResult } from "./protocol/CdpCommands.js"; import { observedFrameUrl } from "./protocol/CdpFrameTree.js"; import { cdpTargetEventMatches } from "./CdpTargetEvents.js"; @@ -739,12 +739,12 @@ class TimelineCapture { } } -const performanceTimestamp = (value: unknown): number => { - const metrics = recordsValue(recordValue(value)?.metrics).filter( - ({ name }) => name === "Timestamp", - ); - const timestamp = numberValue(metrics[0]?.value); - if (metrics.length !== 1 || timestamp === undefined || timestamp < 0) +const performanceTimestamp = ({ + metrics, +}: CdpCommandResult<"Performance.getMetrics">): number => { + const timestamps = metrics.filter(({ name }) => name === "Timestamp"); + const timestamp = timestamps[0]?.value; + if (timestamps.length !== 1 || timestamp === undefined || timestamp < 0) throw new BrowserObservationError("observe_web_session", "protocol_error", { detail: "Performance.getMetrics returned no valid monotonic Timestamp metric.", diff --git a/src/browser/CdpPageCapture.ts b/src/browser/CdpPageCapture.ts index 1f2f8d5b92..2f34a6e639 100644 --- a/src/browser/CdpPageCapture.ts +++ b/src/browser/CdpPageCapture.ts @@ -368,8 +368,10 @@ const capturePageDom = async (state: CaptureState): Promise => { ), ); } catch (cause: unknown) { - // A detached node can disappear between snapshot and description. + // A detached node can disappear between snapshot and description, and + // one malformed description leaves only that node's kind unknown. // Preserve partial metadata, but propagate transport/cancellation errors. + if (cause instanceof CdpMalformedReply) return undefined; if (!(cause instanceof CdpCommandRejection)) throw cause; if (cause.code === -32_601) descriptionUnavailable = true; else if (cause.code !== -32_000) throw cause; diff --git a/src/browser/execution/CdpEventListeners.ts b/src/browser/execution/CdpEventListeners.ts index c6baa210cf..ca366d5073 100644 --- a/src/browser/execution/CdpEventListeners.ts +++ b/src/browser/execution/CdpEventListeners.ts @@ -1,11 +1,10 @@ import { randomUUID } from "node:crypto"; -import { z } from "zod"; import type { InspectWebEventListenersInput, WebEventListenersObservation, } from "../../domain/webEventListeners.js"; import type { CdpRuntimeSources } from "./CdpRuntimeSources.js"; -import { runtimeListenersSchema } from "./CdpRuntimeProtocol.js"; +import type { CdpCommandResult } from "../protocol/CdpCommands.js"; import { queryListenerSelector } from "./CdpListenerSelector.js"; /** Inspect native DOM listener descriptors without evaluating JavaScript or invoking handlers. */ @@ -23,44 +22,28 @@ export const captureWebEventListeners = async ( for (const domain of ["Page", "Runtime", "Debugger", "DOM"]) await session.enable(domain); await session.assertDocument(); - const document = z - .object({ root: z.object({ nodeId: z.int().min(1) }) }) - .parse(await session.command("DOM.getDocument", { depth: 0 })); - const selected = z - .object({ nodeId: z.int().min(0) }) - .parse( - await queryListenerSelector( - session, - document.root.nodeId, - input.selector, - ), - ); + const document = await session.command("DOM.getDocument", { depth: 0 }); + const selected = await queryListenerSelector( + session, + document.root.nodeId, + input.selector, + ); let backendNodeId: number | null = null; - let descriptors: z.infer["listeners"] = []; + let descriptors: CdpCommandResult<"DOMDebugger.getEventListeners">["listeners"] = + []; if (selected.nodeId !== 0) { - const described = z - .object({ node: z.object({ backendNodeId: z.int().min(1) }) }) - .parse( - await session.command("DOM.describeNode", { - nodeId: selected.nodeId, - }), - ); + const described = await session.command("DOM.describeNode", { + nodeId: selected.nodeId, + }); backendNodeId = described.node.backendNodeId; - const resolved = z - .object({ object: z.object({ objectId: z.string().min(1) }) }) - .parse( - await session.command("DOM.resolveNode", { - nodeId: selected.nodeId, - objectGroup: group, - }), - ); - descriptors = runtimeListenersSchema.parse( - await session.command("DOMDebugger.getEventListeners", { - objectId: resolved.object.objectId, - depth: 0, - pierce: false, - }), - ).listeners; + const resolved = await session.command("DOM.resolveNode", { + nodeId: selected.nodeId, + objectGroup: group, + }); + ({ listeners: descriptors } = await session.command( + "DOMDebugger.getEventListeners", + { objectId: resolved.object.objectId, depth: 0, pierce: false }, + )); } const retained = await sources.read( descriptors diff --git a/src/browser/execution/CdpListenerSelector.ts b/src/browser/execution/CdpListenerSelector.ts index f540b7b3ea..26dc9d3467 100644 --- a/src/browser/execution/CdpListenerSelector.ts +++ b/src/browser/execution/CdpListenerSelector.ts @@ -1,5 +1,6 @@ import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; import { CdpCommandRejection } from "../CdpCommandRejection.js"; +import type { CdpCommandResult } from "../protocol/CdpCommands.js"; import type { CdpRuntimeSession } from "./CdpRuntimeSession.js"; /** Classify Chromium's selector syntax rejection without changing other command/transport failures. */ @@ -7,7 +8,7 @@ export const queryListenerSelector = async ( session: CdpRuntimeSession, nodeId: number, selector: string, -): Promise => { +): Promise> => { try { return await session.command("DOM.querySelector", { nodeId, selector }); } catch (cause: unknown) { diff --git a/src/browser/execution/CdpRuntimeProtocol.ts b/src/browser/execution/CdpRuntimeProtocol.ts index 0b8ebea5fa..b8311de2e9 100644 --- a/src/browser/execution/CdpRuntimeProtocol.ts +++ b/src/browser/execution/CdpRuntimeProtocol.ts @@ -18,17 +18,3 @@ export const runtimeStackSchema = z.object({ .object({ id: z.string(), debuggerId: z.string().optional() }) .optional(), }); -/** Native listener descriptors omit live remote handles from portable evidence. */ -export const runtimeListenersSchema = z.object({ - listeners: z.array( - z.object({ - type: z.string(), - useCapture: z.boolean(), - passive: z.boolean(), - once: z.boolean(), - scriptId: z.string(), - lineNumber: integer, - columnNumber: cdpResourceColumnSchema, - }), - ), -}); diff --git a/src/browser/protocol/CdpCommands.ts b/src/browser/protocol/CdpCommands.ts index 2cb08f7401..5c923f556f 100644 --- a/src/browser/protocol/CdpCommands.ts +++ b/src/browser/protocol/CdpCommands.ts @@ -2,6 +2,7 @@ import type { CDPSession } from "playwright-core"; import { z } from "zod"; import { BrowserObservationError } from "../../domain/browserObservationError.js"; +import { cdpResourceColumnSchema } from "../../domain/cdpObservationSchemas.js"; import type { BrowserObservationOperation } from "../../domain/browserObservationErrors.js"; import { err, ok, type Result } from "../../domain/result.js"; import { @@ -21,6 +22,22 @@ export type CdpProtocolMethod = Parameters[0]; const acknowledgement = z.object({}); const coverageOffset = z.int().min(0); +const nodeId = z.int().min(1); + +/** Native listener descriptors; live remote handles are not read. */ +const eventListenersReply = z.object({ + listeners: z.array( + z.object({ + type: z.string(), + useCapture: z.boolean(), + passive: z.boolean(), + once: z.boolean(), + scriptId: z.string(), + lineNumber: z.int().min(0), + columnNumber: cdpResourceColumnSchema, + }), + ), +}); /** Precise coverage keeps every nested producer range; counts are not flattened. */ const preciseCoverageReply = z.object({ @@ -51,6 +68,22 @@ const preciseCoverageReply = z.object({ * that command's raw reply; consumers receive its parsed output. */ const CDP_COMMAND_REPLIES = { + "DOM.describeNode": z.object({ + node: z.object({ + backendNodeId: nodeId, + nodeName: z.string(), + nodeType: z.int(), + // CDP omits isSVG for non-SVG nodes. + isSVG: z.boolean().optional(), + }), + }), + "DOM.getDocument": z.object({ root: z.object({ nodeId }) }), + // Zero is the protocol's "no match". + "DOM.querySelector": z.object({ nodeId: z.int().min(0) }), + "DOM.resolveNode": z.object({ + object: z.object({ objectId: z.string().min(1) }), + }), + "DOMDebugger.getEventListeners": eventListenersReply, "Debugger.enable": acknowledgement, "Debugger.evaluateOnCallFrame": z.object({ result: z.looseObject({}), @@ -59,6 +92,12 @@ const CDP_COMMAND_REPLIES = { "Debugger.getScriptSource": z.object({ scriptSource: z.string() }), "Debugger.resume": acknowledgement, "Debugger.setBreakpointByUrl": z.object({ breakpointId: z.string() }), + "Emulation.clearDeviceMetricsOverride": acknowledgement, + "Emulation.setDeviceMetricsOverride": acknowledgement, + "Emulation.setEmulatedMedia": acknowledgement, + "Emulation.setFocusEmulationEnabled": acknowledgement, + "Emulation.setLocaleOverride": acknowledgement, + "Emulation.setTimezoneOverride": acknowledgement, "Network.enable": acknowledgement, "Page.addScriptToEvaluateOnNewDocument": z.object({ identifier: z.string(), @@ -70,6 +109,9 @@ const CDP_COMMAND_REPLIES = { "Page.setLifecycleEventsEnabled": acknowledgement, "Page.stopLoading": acknowledgement, "Performance.enable": acknowledgement, + "Performance.getMetrics": z.object({ + metrics: z.array(z.object({ name: z.string(), value: z.number() })), + }), "Profiler.startPreciseCoverage": z.object({ timestamp: z.number().min(0) }), "Profiler.stopPreciseCoverage": acknowledgement, "Profiler.takePreciseCoverage": preciseCoverageReply, diff --git a/src/browser/protocol/CdpCommands.untyped-baseline.json b/src/browser/protocol/CdpCommands.untyped-baseline.json index 7a97126d77..848691238b 100644 --- a/src/browser/protocol/CdpCommands.untyped-baseline.json +++ b/src/browser/protocol/CdpCommands.untyped-baseline.json @@ -22,13 +22,9 @@ "Browser.getVersion": 1, "Target.getTargets": 1 }, - "src/browser/CdpObservationSession.ts": { - "Performance.getMetrics": 1 - }, "src/browser/CdpPageCapture.ts": { "Network.getResponseBody": 2, "DOMSnapshot.captureSnapshot": 1, - "DOM.describeNode": 1, "Accessibility.getFullAXTree": 1 }, "src/browser/CdpPageCaptureWorkers.ts": { @@ -41,27 +37,10 @@ "src/browser/CdpWebMcpDiscovery.ts": { "WebMCP.enable": 1 }, - "src/browser/PlaywrightScenarioBrowser.ts": { - "Emulation.setEmulatedMedia": 1, - "Emulation.setDeviceMetricsOverride": 2, - "Emulation.setLocaleOverride": 3, - "Emulation.setTimezoneOverride": 3, - "Emulation.setFocusEmulationEnabled": 3, - "Emulation.clearDeviceMetricsOverride": 1 - }, "src/browser/PlaywrightScenarioServiceWorkers.ts": { "Runtime.evaluate": 1 }, "src/browser/PlaywrightScenarioStorage.ts": { "Target.sendMessageToTarget": 1 - }, - "src/browser/execution/CdpEventListeners.ts": { - "DOM.getDocument": 1, - "DOM.describeNode": 1, - "DOM.resolveNode": 1, - "DOMDebugger.getEventListeners": 1 - }, - "src/browser/execution/CdpListenerSelector.ts": { - "DOM.querySelector": 2 } } diff --git a/tests/boundary/browser/cdpDomSvgIdentity.test.ts b/tests/boundary/browser/cdpDomSvgIdentity.test.ts index eeaebf2e7a..2280257b02 100644 --- a/tests/boundary/browser/cdpDomSvgIdentity.test.ts +++ b/tests/boundary/browser/cdpDomSvgIdentity.test.ts @@ -94,7 +94,7 @@ describe("CDP SVG identity at the snapshot boundary", () => { ]); }); - it.each(["removed", "mismatched"])( + it.each(["removed", "mismatched", "malformed"])( "retains partial metadata when a candidate is %s", async (failure) => { const browser = await startBrowser({ @@ -116,7 +116,18 @@ describe("CDP SVG identity at the snapshot boundary", () => { isSVG: true, }, } - : undefined, + : failure === "malformed" && + command.method === "DOM.describeNode" && + command.params.backendNodeId === 3 + ? { + node: { + backendNodeId: 3, + nodeType: 1, + nodeName: "a", + isSVG: "true", + }, + } + : undefined, }); const result = await inspect(browser); if (!result.ok) throw result.error; diff --git a/tests/boundary/browser/cdpWebRuntime.test.ts b/tests/boundary/browser/cdpWebRuntime.test.ts index 1ba2d5079a..5a04d1cc22 100644 --- a/tests/boundary/browser/cdpWebRuntime.test.ts +++ b/tests/boundary/browser/cdpWebRuntime.test.ts @@ -372,6 +372,36 @@ describe("runtime source and listener identity", () => { ).toBe(false); }); + it.each([ + ["DOM.getDocument", { root: {} }, "/root/nodeId"], + ["DOM.querySelector", { nodeId: -1 }, "/nodeId"], + ["DOM.describeNode", { node: { backendNodeId: 22 } }, "/node/nodeName"], + ["DOM.resolveNode", { object: { objectId: "" } }, "/object/objectId"], + ["DOMDebugger.getEventListeners", { listeners: "none" }, "/listeners"], + ] as const)( + "reports a malformed %s reply as a protocol error and releases the session", + async (method, reply, pointer) => { + const { browser, provider, listeners } = await fixture({ + commandResult: (command) => + command.method === method ? reply : undefined, + }); + const result = await provider.inspectEventListeners(listeners); + if (result.ok) throw new Error("A malformed DOM reply must fail"); + expect(result.error).toMatchObject({ + _tag: "BrowserObservationError", + operation: "inspect_web_event_listeners", + reason: "protocol_error", + command: method, + pointer, + }); + expect( + browser.commands.some( + (command) => command.method === "Target.detachFromTarget", + ), + ).toBe(true); + }, + ); + it("inspects listener source and releases only its owned object group", async () => { const { browser, provider, listeners } = await fixture(); const result = await provider.inspectEventListeners(listeners); diff --git a/tests/fixtures/webRuntime.ts b/tests/fixtures/webRuntime.ts index 9d7dcc1db0..3cc091c5d9 100644 --- a/tests/fixtures/webRuntime.ts +++ b/tests/fixtures/webRuntime.ts @@ -96,7 +96,9 @@ const runtimeResult = ( case "DOM.querySelector": return { nodeId: command.params.selector === "#missing" ? 0 : 2 }; case "DOM.describeNode": - return { node: { backendNodeId: 22 } }; + return { + node: { backendNodeId: 22, nodeType: 1, nodeName: "BUTTON" }, + }; case "DOM.resolveNode": return { object: { objectId: "selected-node-object" } }; case "DOMDebugger.getEventListeners":