From 5891bfb302e2ce6626495cd4591181ef6d0fdfa7 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:50:32 +0800 Subject: [PATCH] fix(browser): retain precise observation termination facts Distinguish CDP session detachment, observed crashes, and transport loss. Preserve the observed window alongside the original typed coverage-stop failure and unconfirmed cleanup. Validated with 40 focused boundary tests, check:ci, docs:check, and the real Chrome public CLI/MCP verifier on macOS. Browser timing comparison remains incomplete because unchanged-main fixture readiness was intermittent. Fixes #5756 --- docs/web-runtime.md | 4 +- .../lib/browser-runtime-termination-e2e.mjs | 11 +- src/browser/CdpObservationSession.ts | 32 +- src/browser/execution/CdpExecutionCapture.ts | 51 ++- .../execution/CdpExecutionCollection.ts | 4 +- src/browser/execution/CdpExecutionWindow.ts | 11 +- src/domain/analysisErrorBase.ts | 14 +- src/domain/analysisErrorPresentation.ts | 1 + src/domain/analysisErrorProjection.ts | 1 + src/domain/browserObservationError.ts | 2 + src/domain/browserObservationErrors.ts | 1 + src/domain/browserSession.ts | 4 + src/domain/webExecution.ts | 1 + ...BrowserProvider.lifecycle_failures.test.ts | 34 +- .../browser/cdpObservationWindowScope.test.ts | 10 +- .../browser/cdpWebRuntimeTermination.test.ts | 299 +++++++++++++++--- tests/fixtures/fakeCdpBrowser.ts | 2 + tests/fixtures/fakeCdpBrowserEvents.ts | 2 + tests/fixtures/fakeCdpBrowserTypes.ts | 3 +- 19 files changed, 414 insertions(+), 73 deletions(-) diff --git a/docs/web-runtime.md b/docs/web-runtime.md index 9514f66cf1..63b99d6968 100644 --- a/docs/web-runtime.md +++ b/docs/web-runtime.md @@ -32,7 +32,9 @@ A document replacement ends the window; final coverage then remains unavailable. Same-document navigation such as `history.pushState()` remains valid when the frame and known loader identity stay stable and the current origin is allowed. Unknown loader identity retains a conservative URL check; newly reported or replaced loaders are rejected. Malformed producer events end an armed window immediately and trigger cleanup, independently of the selected duration. -Target detachment ends the window even when the browser's root connection stays open. When the closed session cannot acknowledge cleanup, the operation returns a failure with the selected target, failed commands and original producer reason; it does not claim successful cleanup. +Target detachment ends the window with `session_detached`, even when the browser's root connection stays open. An `Inspector.targetCrashed` event ends it with `target_terminated`. A lost transport records `transport_disconnected` in browser observation; execution capture returns the typed transport failure. These outcomes retain the selected target and the observed lifecycle fact. + +When an ended session cannot acknowledge cleanup, the operation returns a failure with the failed commands and original typed producer reason. A coverage-stop failure also retains the observed window's end reason in its diagnostics; cleanup remains unconfirmed until the owning session can resolve it. Child frames, workers, service workers, WebAssembly, chronological execution order and framework-specific causal interpretation are outside this operation. Script ownership comes from frame/context metadata, never URL equality. Repeated URLs and synthetic names therefore retain separate identities. diff --git a/scripts/lib/browser-runtime-termination-e2e.mjs b/scripts/lib/browser-runtime-termination-e2e.mjs index 52dd90d5ac..a57d0716f7 100644 --- a/scripts/lib/browser-runtime-termination-e2e.mjs +++ b/scripts/lib/browser-runtime-termination-e2e.mjs @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { CdpConnection } from "../../dist/browser/CdpConnection.js"; -import { mcpTextValue } from "./mcp-verifier-results.mjs"; +import { mcpTextValue, requireMcpToolError } from "./mcp-verifier-results.mjs"; import { closeVerifierResources } from "./verifier-cleanup.mjs"; /** Close a separately owned fixture page after actual MCP arming and inspect native cleanup diagnostics. */ @@ -60,17 +60,26 @@ export async function verifyRuntimeTargetTermination(client, input, origin) { "Detached instrumentation cannot claim confirmed cleanup", ); const failure = mcpTextValue(response); + const projected = requireMcpToolError(response); assert.match(failure, /cleanup.*confirmed/u); assert.ok( failure.includes(targetId), "Failure must retain the selected target identity", ); assert.match(failure, /Profiler\.stopPreciseCoverage/u); + assert.equal(projected.code, "cleanup_incomplete"); + assert.equal( + projected.details?.diagnostics?.primary_error?.details?.diagnostics + ?.observed_window?.end_reason, + "session_detached", + "Target.closeTarget detaches this CDP session; it does not prove the target crashed", + ); return { target_id: targetId, page_closed_after_arming: true, failure_inline: true, cleanup: "unconfirmed_reported", + observed_window_end_reason: "session_detached", }; } catch (cause) { primaryError = cause; diff --git a/src/browser/CdpObservationSession.ts b/src/browser/CdpObservationSession.ts index 51adfe5612..ccb2d6b1ad 100644 --- a/src/browser/CdpObservationSession.ts +++ b/src/browser/CdpObservationSession.ts @@ -69,7 +69,7 @@ export const observeCdpSession = async ( const capture = new TimelineCapture(allowedOrigins, mainFrameId, initialUrl); let removeListener = (): void => {}; const removeDisconnectListener = context.connection.onDisconnect(() => - capture.targetTerminated(), + capture.transportDisconnected(), ); try { await context.connection.send( @@ -236,9 +236,11 @@ class TimelineCapture { this.#lifecycle(params); break; case "Inspector.targetCrashed": - case "Target.detachedFromTarget": this.targetTerminated(params); break; + case "Target.detachedFromTarget": + this.sessionDetached(params); + break; } } @@ -306,6 +308,32 @@ class TimelineCapture { this.end("target_terminated"); } + sessionDetached(params: UnknownRecord = {}): void { + if (this.#endReason !== undefined) return; + this.#add({ + type: "session_detached", + params, + timestamp: null, + rawUrl: null, + requestId: null, + detail: "session_detached", + }); + this.end("session_detached"); + } + + transportDisconnected(): void { + if (this.#endReason !== undefined) return; + this.#add({ + type: "transport_disconnected", + params: {}, + timestamp: null, + rawUrl: null, + requestId: null, + detail: "transport_disconnected", + }); + this.end("transport_disconnected"); + } + setFinalUrl(rawUrl: string): void { if (this.#scopeExcluded) return; const destination = scopedUrl(rawUrl, this.allowedOrigins); diff --git a/src/browser/execution/CdpExecutionCapture.ts b/src/browser/execution/CdpExecutionCapture.ts index 14c5bb6414..1448ac2296 100644 --- a/src/browser/execution/CdpExecutionCapture.ts +++ b/src/browser/execution/CdpExecutionCapture.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { BrowserObservationError } from "../../domain/browserObservationError.js"; import { AnalysisCancelledError, @@ -37,9 +38,18 @@ export const captureWebExecution = async ( sources.check(); window.check(); if (window.reason !== undefined) - throw new BrowserObservationError(session.operation, "target_changed", { - detail: `Selected target ${session.target.target_id} ended with ${window.reason} before execution observation could be armed.`, - }); + throw new BrowserObservationError( + session.operation, + window.reason === "session_detached" + ? "target_session_detached" + : "target_changed", + { + detail: executionWindowEndDetail( + session.target.target_id, + window.reason, + ), + }, + ); const waiting = window.start(input.observation_ms); // Cancellation may arrive while asynchronous progress is being delivered. void waiting.catch(() => undefined); @@ -64,7 +74,7 @@ export const captureWebExecution = async ( raw = await session.command("Profiler.takePreciseCoverage"); } collection.freezeMetadata(); - await session.command("Profiler.stopPreciseCoverage"); + await stopPreciseCoverage(session, reason); session.preciseCoverageMayBeActive = false; const requestItems = requests.result(); const parsed = @@ -161,3 +171,36 @@ export const captureWebExecution = async ( collection.close(); } }; + +const stopPreciseCoverage = async ( + session: CdpRuntimeSources["session"], + reason: WebExecution["window"]["end_reason"], +): Promise => { + try { + await session.command("Profiler.stopPreciseCoverage"); + } catch (cause: unknown) { + if ( + (reason !== "target_terminated" && reason !== "session_detached") || + !(cause instanceof AnalysisError) + ) + throw cause; + cause.retainDiagnostics({ + observed_window: { + target_id: session.target.target_id, + end_reason: reason, + }, + }); + throw cause; + } +}; + +const executionWindowEndDetail = ( + targetId: string, + reason: WebExecution["window"]["end_reason"], +): string => { + if (reason === "session_detached") + return `The debugging session for selected target ${targetId} detached before execution observation could be armed; the target may still exist.`; + if (reason === "target_terminated") + return `Selected target ${targetId} crashed before execution observation could be armed.`; + return `Selected target ${targetId} changed documents before execution observation could be armed.`; +}; diff --git a/src/browser/execution/CdpExecutionCollection.ts b/src/browser/execution/CdpExecutionCollection.ts index f96c296047..df37bf48c2 100644 --- a/src/browser/execution/CdpExecutionCollection.ts +++ b/src/browser/execution/CdpExecutionCollection.ts @@ -40,8 +40,8 @@ export class CdpExecutionCollection { } this.window.ingest(event); }), - session.transport.connection.onDisconnect(() => - this.window.end("target_terminated"), + session.transport.connection.onDisconnect((error) => + this.window.fail(error), ), session.transport.connection.onProtocolFailure((error) => this.window.fail(error), diff --git a/src/browser/execution/CdpExecutionWindow.ts b/src/browser/execution/CdpExecutionWindow.ts index fddfe10329..ccbb1f9626 100644 --- a/src/browser/execution/CdpExecutionWindow.ts +++ b/src/browser/execution/CdpExecutionWindow.ts @@ -25,10 +25,11 @@ export class CdpExecutionWindow { /** Collection ends on a main-document replacement, never follows source URLs into another context. */ ingest(event: CdpEvent): void { - if ( - event.method === "Target.detachedFromTarget" || - event.method === "Inspector.targetCrashed" - ) { + if (event.method === "Target.detachedFromTarget") { + this.end("session_detached"); + return; + } + if (event.method === "Inspector.targetCrashed") { this.end("target_terminated"); return; } @@ -101,7 +102,7 @@ export class CdpExecutionWindow { }); } - /** Transport loss ends the window; instrumentation cleanup still requires confirmation. */ + /** A producer lifecycle fact ends the window; instrumentation cleanup still requires confirmation. */ end(reason: EndReason): void { if (this.reason !== undefined || this.#failure !== undefined) return; this.reason = reason; diff --git a/src/domain/analysisErrorBase.ts b/src/domain/analysisErrorBase.ts index 1e4ceef10e..f408f471ee 100644 --- a/src/domain/analysisErrorBase.ts +++ b/src/domain/analysisErrorBase.ts @@ -143,17 +143,21 @@ export abstract class AnalysisError extends Error { this.#ownedCleanupResources = [...resources]; } - /** Retain a later lifecycle failure without replacing the original typed error. */ + /** Retain diagnostic facts without replacing the original typed error. */ + retainDiagnostics(diagnostics: Readonly>): this { + Object.assign(this, { + diagnostics: structuredClone({ ...this.diagnostics, ...diagnostics }), + }); + return this; + } + retainSecondaryFailure( failure: AnalysisError, diagnostics?: Readonly>, ): this { // Secondary resources remain owned by their cleanup lifecycle; preserve // the primary error's live cleanup status and original output. - if (diagnostics !== undefined) - Object.assign(this, { - diagnostics: structuredClone({ ...this.diagnostics, ...diagnostics }), - }); + if (diagnostics !== undefined) this.retainDiagnostics(diagnostics); this.#secondaryFailures.push(failure); return this; } diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index 3d3578a887..ac9599e35d 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -204,6 +204,7 @@ const browserErrorCategory = ( reason === "target_not_allowed" || reason === "service_worker_blocked" || reason === "target_changed" || + reason === "target_session_detached" || reason === "endpoint_unreachable" || reason === "disconnected" ) diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index 05bb2aec9a..43cd286d37 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -180,6 +180,7 @@ const BROWSER_ERROR_CODES: Readonly< target_not_allowed: "target_unavailable", service_worker_blocked: "target_unavailable", target_changed: "target_unavailable", + target_session_detached: "target_unavailable", protocol_error: "unreadable_output", disconnected: "provider_unavailable", payload_limit: "truncated", diff --git a/src/domain/browserObservationError.ts b/src/domain/browserObservationError.ts index a519900b79..261ad704b3 100644 --- a/src/domain/browserObservationError.ts +++ b/src/domain/browserObservationError.ts @@ -51,6 +51,8 @@ const observationFailureMessage = ( return `${operation} could not reach the selected loopback debugging endpoint. Verify that the target process is running with debugging enabled and that the endpoint's port matches it, then retry.`; if (reason === "target_not_found") return `${operation} could not find the selected target at the debugging endpoint. Refresh target discovery for that endpoint and retry with a returned target ID.`; + if (reason === "target_session_detached") + return `${operation} lost its debugging session while the target may still exist. Refresh target discovery and attach to the selected target again.`; if (reason === "service_worker_blocked") return `${operation} refused the selected page because it is controlled by or has a registered service worker while the service-worker policy is block. Set environment.service_workers to allow to permit observation, or remove the service worker registration through the browser.`; if (reason === "disconnected") diff --git a/src/domain/browserObservationErrors.ts b/src/domain/browserObservationErrors.ts index 89b2b73e0b..3027553f51 100644 --- a/src/domain/browserObservationErrors.ts +++ b/src/domain/browserObservationErrors.ts @@ -26,6 +26,7 @@ export type BrowserObservationFailureReason = | "target_not_allowed" | "service_worker_blocked" | "target_changed" + | "target_session_detached" | "protocol_error" | "disconnected" | "payload_limit" diff --git a/src/domain/browserSession.ts b/src/domain/browserSession.ts index d36737795c..a8c7b83390 100644 --- a/src/domain/browserSession.ts +++ b/src/domain/browserSession.ts @@ -29,6 +29,8 @@ const timelineEventSchema = z.object({ "load_failed", "lifecycle", "target_terminated", + "session_detached", + "transport_disconnected", ]), timestamp: z .number() @@ -63,6 +65,8 @@ export const webObservationSessionSchema = z.object({ "window_elapsed", "target_left_scope", "target_terminated", + "session_detached", + "transport_disconnected", ]), }), timeline: z.array(timelineEventSchema), diff --git a/src/domain/webExecution.ts b/src/domain/webExecution.ts index b1344e390f..459757c83d 100644 --- a/src/domain/webExecution.ts +++ b/src/domain/webExecution.ts @@ -40,6 +40,7 @@ export const webExecutionSchema = z.object({ "window_elapsed", "document_changed", "target_terminated", + "session_detached", ]), producer_started_seconds: z.number().min(0), producer_sampled_seconds: z.number().min(0).nullable(), diff --git a/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts b/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts index 2dba76b7e5..9b9e12c5d0 100644 --- a/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts +++ b/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts @@ -366,10 +366,10 @@ describe("CdpBrowserProvider: session termination and scope changes", () => { ); if (!result.ok) throw result.error; - expect(result.value.window.end_reason).toBe("target_terminated"); + expect(result.value.window.end_reason).toBe("session_detached"); expect(result.value.timeline.at(-1)).toMatchObject({ - type: "target_terminated", - detail: "target_terminated", + type: "session_detached", + detail: "session_detached", timestamp: null, }); expect(result.value.limitations).toContain( @@ -377,6 +377,28 @@ describe("CdpBrowserProvider: session termination and scope changes", () => { ); }); + it("reports target termination only for an observed target crash", async () => { + const browser = await startFakeCdpBrowser({ + sessionTimeline: "target_crashed", + }); + trackBrowser(browser); + const result = await new CdpBrowserProvider().observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 1_000, + }), + ); + + if (!result.ok) throw result.error; + expect(result.value.window.end_reason).toBe("target_terminated"); + expect(result.value.timeline.at(-1)).toMatchObject({ + type: "target_terminated", + detail: "target_terminated", + }); + }); + it("ends a direct page session when its transport disconnects", async () => { const browser = await startFakeCdpBrowser({ pageScopedVersionWebSocket: true, @@ -394,10 +416,10 @@ describe("CdpBrowserProvider: session termination and scope changes", () => { ); if (!result.ok) throw result.error; - expect(result.value.window.end_reason).toBe("target_terminated"); + expect(result.value.window.end_reason).toBe("transport_disconnected"); expect(result.value.timeline.at(-1)).toMatchObject({ - type: "target_terminated", - detail: "target_terminated", + type: "transport_disconnected", + detail: "transport_disconnected", }); const methods = browser.commands.map(({ method }) => method); expect(methods).not.toContain("Target.attachToTarget"); diff --git a/tests/boundary/browser/cdpObservationWindowScope.test.ts b/tests/boundary/browser/cdpObservationWindowScope.test.ts index 7e76b967c8..8331adc17b 100644 --- a/tests/boundary/browser/cdpObservationWindowScope.test.ts +++ b/tests/boundary/browser/cdpObservationWindowScope.test.ts @@ -268,6 +268,8 @@ it("fails closed on an untimestamped out-of-scope redirect during clock sampling it.each([ [ "Inspector.targetCrashed", + "target_terminated", + "target_terminated", { method: "Inspector.targetCrashed", sessionId: "session-1", @@ -276,6 +278,8 @@ it.each([ ], [ "Target.detachedFromTarget", + "session_detached", + "session_detached", { method: "Target.detachedFromTarget", params: { sessionId: "session-1", targetId: "allowed-page" }, @@ -283,7 +287,7 @@ it.each([ ], ] as const)( "ends on buffered untimestamped %s before collecting later events", - async (_method, terminalEvent) => { + async (_method, endReason, eventType, terminalEvent) => { const { observation } = await observeWithFake({ eventsBeforeReply: ["Performance.getMetrics"], commandResult: (command) => @@ -310,10 +314,10 @@ it.each([ }, }); - expect(observation.window.end_reason).toBe("target_terminated"); + expect(observation.window.end_reason).toBe(endReason); expect(observation.timeline).toContainEqual( expect.objectContaining({ - type: "target_terminated", + type: eventType, timestamp: null, }), ); diff --git a/tests/boundary/browser/cdpWebRuntimeTermination.test.ts b/tests/boundary/browser/cdpWebRuntimeTermination.test.ts index ec2e633a40..588088225b 100644 --- a/tests/boundary/browser/cdpWebRuntimeTermination.test.ts +++ b/tests/boundary/browser/cdpWebRuntimeTermination.test.ts @@ -1,11 +1,12 @@ -import { describe, expect, it, onTestFinished } from "vitest"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; import { CdpWebRuntimeProvider } from "../../../src/browser/execution/CdpWebRuntimeProvider.js"; import type { CdpEvent } from "../../../src/browser/CdpConnection.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; import { observeWebExecutionInputSchema } from "../../../src/domain/webExecution.js"; import { startRuntimeBrowser } from "../../fixtures/webRuntime.js"; describe("runtime flat-session lifecycle", () => { - it("rejects a producer detachment between counter startup and local arming", async () => { + it("rejects session detachment between counter startup and local arming", async () => { let emitEvent: ((event: CdpEvent) => void) | undefined; const browser = await startRuntimeBrowser({ commandResult: (command) => { @@ -37,14 +38,223 @@ describe("runtime flat-session lifecycle", () => { }, ); if (result.ok) - throw new Error("A terminated target cannot arm an execution window"); - expect(result.error).toMatchObject({ reason: "target_changed" }); + throw new Error("A detached session cannot arm an execution window"); + expect(result.error).toMatchObject({ reason: "target_session_detached" }); expect(reportedArmed).toBe(false); }); - it("ends immediately on its root detachment without confusing another session", async () => { + it.each([ + { + case: "matching detach after an unrelated session detaches", + events: [ + { + method: "Target.detachedFromTarget", + params: { sessionId: "another-session" }, + }, + { + method: "Target.detachedFromTarget", + params: { sessionId: "session-1" }, + }, + ] as CdpEvent[], + observationMs: 2_147_483_647, + endReason: "session_detached", + }, + { + case: "detach for another flat session", + events: [ + { + method: "Target.detachedFromTarget", + params: { sessionId: "another-session" }, + }, + ] as CdpEvent[], + observationMs: 30, + endReason: "window_elapsed", + }, + { + case: "the observed crash event", + events: [ + { + method: "Inspector.targetCrashed", + sessionId: "session-1", + params: {}, + }, + ] as CdpEvent[], + observationMs: 2_147_483_647, + endReason: "target_terminated", + }, + ])("preserves lifecycle identity for $case", async (scenario) => { const browser = await startRuntimeBrowser(); onTestFinished(() => browser.close()); + const result = await new CdpWebRuntimeProvider().observeExecution( + observeWebExecutionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "allowed-page", + observation_ms: scenario.observationMs, + }), + { + signal: AbortSignal.timeout(2_000), + progress: { + report: async (update) => { + if (update.completed === 1) + for (const event of scenario.events) browser.emitEvent(event); + }, + }, + }, + ); + if (!result.ok) throw result.error; + expect(result.value.window.end_reason).toBe(scenario.endReason); + if (scenario.endReason !== "window_elapsed") { + expect(result.value.coverage.state).toBe("unavailable"); + expect(result.value.instrumentation.takes_one_resetting_sample).toBe( + false, + ); + } + }); +}); + +describe("runtime stop lifecycle", () => { + it.each(["cleanup failure", "caller cancellation"] as const)( + "retains explicit session detachment during precise-coverage stop with %s", + async (outcome) => { + const controller = new AbortController(); + const reason = { caller: "stop precise coverage" }; + const browser = await startRuntimeBrowser( + outcome === "cleanup failure" + ? { + commandError: (command) => + command.method === "Profiler.stopPreciseCoverage" + ? { code: -32_000, message: "Target session is detached" } + : undefined, + } + : { + commandResult: (command) => { + if (command.method === "Profiler.stopPreciseCoverage") + controller.abort(reason); + return undefined; + }, + }, + ); + onTestFinished(() => browser.close()); + const result = await new CdpWebRuntimeProvider().observeExecution( + observeWebExecutionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "allowed-page", + observation_ms: 2_147_483_647, + }), + { + signal: + outcome === "cleanup failure" + ? AbortSignal.timeout(2_000) + : controller.signal, + progress: { + report: async (update) => { + if (update.completed === 1) + browser.emitEvent({ + method: "Target.detachedFromTarget", + params: { sessionId: "session-1" }, + }); + }, + }, + }, + ); + if (outcome === "caller cancellation") { + if (result.ok) throw new Error("The aborted stop command must fail"); + expect(result.error).toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + }); + return; + } + if (result.ok) + throw new Error("Unconfirmed cleanup must remain a failure"); + + const projection = projectAnalysisError(result.error); + expect(projection.code).toBe("cleanup_incomplete"); + expect(projection.details?.diagnostics).toMatchObject({ + primary_error: { + message: expect.stringContaining("Target session is detached"), + details: { + diagnostics: { + observed_window: { + target_id: "allowed-page", + end_reason: "session_detached", + }, + }, + }, + }, + cleanup_error: { + details: { + diagnostics: { + cleanup_failures: expect.arrayContaining([ + expect.stringContaining("Profiler.stopPreciseCoverage"), + ]), + }, + }, + }, + }); + expect( + browser.commands.filter( + ({ method }) => method === "Profiler.stopPreciseCoverage", + ), + ).toHaveLength(2); + expect(result.error.cleanupPrimary).toMatchObject({ + _tag: "BrowserObservationError", + reason: "protocol_error", + diagnostics: { + observed_window: { + target_id: "allowed-page", + end_reason: "session_detached", + }, + }, + }); + }, + ); +}); + +describe("runtime disconnect ownership", () => { + it("preserves a pending stop-command transport failure after session detachment", async () => { + const browser = await startRuntimeBrowser({ + hangOnMethod: "Profiler.stopPreciseCoverage", + }); + let closed = false; + onTestFinished(async () => { + if (!closed) await browser.close(); + }); + const observed = new CdpWebRuntimeProvider().observeExecution( + observeWebExecutionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "allowed-page", + observation_ms: 1, + }), + { signal: AbortSignal.timeout(2_000) }, + ); + await vi.waitFor(() => { + expect( + browser.commands.some( + ({ method }) => method === "Profiler.stopPreciseCoverage", + ), + ).toBe(true); + }); + browser.emitEvent({ + method: "Target.detachedFromTarget", + params: { sessionId: "session-1" }, + }); + await browser.close(); + closed = true; + const result = await observed; + if (result.ok) throw new Error("The pending stop request must fail"); + expect(result.error.cleanupPrimary).toMatchObject({ + _tag: "BrowserObservationError", + reason: "disconnected", + }); + }); + + it("does not infer target termination from transport disconnect", async () => { + const browser = await startRuntimeBrowser(); + let closed = false; + onTestFinished(async () => { + if (!closed) await browser.close(); + }); const result = await new CdpWebRuntimeProvider().observeExecution( observeWebExecutionInputSchema.parse({ cdp_endpoint: browser.endpoint, @@ -52,52 +262,55 @@ describe("runtime flat-session lifecycle", () => { observation_ms: 2_147_483_647, }), { - signal: AbortSignal.timeout(2_000), progress: { report: async (update) => { - if (update.completed !== 1) return; - browser.emitEvent({ - method: "Target.detachedFromTarget", - params: { sessionId: "another-session" }, - }); - browser.emitEvent({ - method: "Target.detachedFromTarget", - params: { sessionId: "session-1" }, - }); + if (update.completed === 1) { + await browser.close(); + closed = true; + } }, }, }, ); - if (!result.ok) throw result.error; - expect(result.value.window.end_reason).toBe("target_terminated"); - expect(result.value.coverage.state).toBe("unavailable"); - expect(result.value.instrumentation.takes_one_resetting_sample).toBe(false); + if (result.ok) throw new Error("A disconnected transport is not a result"); + expect(result.error.cleanupPrimary).toMatchObject({ + _tag: "BrowserObservationError", + reason: "disconnected", + }); }); +}); - it("ignores root detachment for a different flat session", async () => { - const browser = await startRuntimeBrowser(); - onTestFinished(() => browser.close()); - const result = await new CdpWebRuntimeProvider().observeExecution( - observeWebExecutionInputSchema.parse({ - cdp_endpoint: browser.endpoint, - target_id: "allowed-page", - observation_ms: 30, - }), - { - progress: { - report: async (update) => { - if (update.completed === 1) - browser.emitEvent({ - method: "Target.detachedFromTarget", - params: { sessionId: "another-session" }, - }); - }, - }, - }, - ); - if (!result.ok) throw result.error; - expect(result.value.window.end_reason).toBe("window_elapsed"); - }); +describe("runtime cleanup failure ordering", () => { + it.each([ + ["Profiler.takePreciseCoverage", "Coverage sample failed"], + ["Profiler.stopPreciseCoverage", "Coverage stop failed"], + ] as const)( + "keeps the %s protocol failure primary when transport later closes", + async (method, message) => { + const browser = await startRuntimeBrowser({ + commandError: (command) => + command.method === method ? { code: -32_000, message } : undefined, + closeAfterMethod: method, + }); + onTestFinished(() => browser.close()); + const result = await new CdpWebRuntimeProvider().observeExecution( + observeWebExecutionInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "allowed-page", + observation_ms: 1, + }), + { signal: AbortSignal.timeout(2_000) }, + ); + if (result.ok) throw new Error("The failed CDP command must fail"); + const primary = result.error.cleanupPrimary; + expect(primary).toMatchObject({ + _tag: "BrowserObservationError", + reason: "protocol_error", + }); + if (method === "Profiler.takePreciseCoverage") + expect(primary?.secondaryFailures).toHaveLength(0); + }, + ); }); describe("coverage URL representation", () => { diff --git a/tests/fixtures/fakeCdpBrowser.ts b/tests/fixtures/fakeCdpBrowser.ts index f06300f065..646b6c0360 100644 --- a/tests/fixtures/fakeCdpBrowser.ts +++ b/tests/fixtures/fakeCdpBrowser.ts @@ -60,6 +60,8 @@ export const startFakeCdpBrowser = async ( const commandError = options.commandError?.(command); if (commandError !== undefined) { sendCommandReply(socket, command, { error: commandError }); + if (options.closeAfterMethod === command.method) + setImmediate(() => socket.close()); return; } if (options.closeOnMethod === command.method) { diff --git a/tests/fixtures/fakeCdpBrowserEvents.ts b/tests/fixtures/fakeCdpBrowserEvents.ts index 294c2ad567..20d4ef0733 100644 --- a/tests/fixtures/fakeCdpBrowserEvents.ts +++ b/tests/fixtures/fakeCdpBrowserEvents.ts @@ -305,6 +305,8 @@ const emitSessionTimeline = ( sessionId: command.sessionId, targetId: "allowed-page", }); + if (options.sessionTimeline === "target_crashed") + event(socket, "Inspector.targetCrashed", command.sessionId, {}); } }; diff --git a/tests/fixtures/fakeCdpBrowserTypes.ts b/tests/fixtures/fakeCdpBrowserTypes.ts index 56b03659b6..08472f451c 100644 --- a/tests/fixtures/fakeCdpBrowserTypes.ts +++ b/tests/fixtures/fakeCdpBrowserTypes.ts @@ -92,7 +92,8 @@ export interface FakeOptions { readonly sessionTimeline?: | "same_origin" | "outside_policy" - | "target_detached"; + | "target_detached" + | "target_crashed"; readonly sessionTimelineEventCount?: number; readonly sessionRedirectResourceType?: "Document" | "Fetch" | "Script"; readonly closeAfterMethod?: string;