From 897caa814d53e95f8bbe9d797a72654c43df1dd2 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 9 Nov 2026 19:34:22 +0800 Subject: [PATCH] fix(inspector): drain discovery requests after sibling failure (#9598) fix(inspector): drain failed discovery requests --- src/inspector/V8InspectorEndpoint.test.ts | 95 +++++++++++++++++++++++ src/inspector/V8InspectorEndpoint.ts | 22 +++++- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/inspector/V8InspectorEndpoint.test.ts b/src/inspector/V8InspectorEndpoint.test.ts index f92b969267..38f8977be5 100644 --- a/src/inspector/V8InspectorEndpoint.test.ts +++ b/src/inspector/V8InspectorEndpoint.test.ts @@ -3,6 +3,7 @@ import { createServer } from "node:http"; import { expect, it } from "vitest"; import { V8InspectorProvider } from "./V8InspectorProvider.js"; +import { awaitAcknowledgement } from "../../tests/fixtures/lifecycleAcknowledgement.js"; import { startFakeV8Inspector } from "../../tests/fixtures/inspector/fakeV8Inspector.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; @@ -149,3 +150,97 @@ it("keeps connectable siblings when a target omits its direct socket", async () await new Promise((resolve) => server.close(() => resolve())); } }); + +it.each(["/json/version", "/json/list"])( + "closes the other discovery request when %s fails", + async (failedPath) => { + let failedResponse: import("node:http").ServerResponse | undefined; + let held = false; + let heldClosed!: () => void; + const closed = new Promise((resolve) => { + heldClosed = resolve; + }); + const server = createServer((request, response) => { + if (request.url === failedPath) { + failedResponse = response; + } else { + held = true; + request.on("close", heldClosed); + } + if (held) failedResponse?.writeHead(500).end(); + }); + await new Promise((resolve) => + server.listen(0, "127.0.0.1", resolve), + ); + const address = server.address(); + if (address === null || typeof address === "string") + throw new Error("no address"); + const provider = new V8InspectorProvider(); + try { + const result = await provider.listTargets({ + inspector_endpoint: `http://127.0.0.1:${address.port}`, + }); + if (result.ok) throw new Error("expected discovery failure"); + expect(projectAnalysisError(result.error).code).toBe("unreadable_output"); + await awaitAcknowledgement( + closed, + new Promise(() => undefined), + "pending discovery socket close", + ); + } finally { + server.closeAllConnections(); + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())), + ); + await provider.close(); + } + }, +); + +it("closes both discovery requests and retains caller cancellation", async () => { + let arrived = 0; + let released = 0; + let acknowledgeStarted!: () => void; + let acknowledgeClosed!: () => void; + const started = new Promise((resolve) => { + acknowledgeStarted = resolve; + }); + const closed = new Promise((resolve) => { + acknowledgeClosed = resolve; + }); + const server = createServer((request) => { + request.on("close", () => { + if (++released === 2) acknowledgeClosed(); + }); + if (++arrived === 2) acknowledgeStarted(); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (address === null || typeof address === "string") + throw new Error("no address"); + const provider = new V8InspectorProvider(); + const controller = new AbortController(); + const reason = new Error("caller stopped observation"); + const operation = provider.listTargets( + { inspector_endpoint: `http://127.0.0.1:${address.port}` }, + { signal: controller.signal }, + ); + try { + await awaitAcknowledgement(started, operation, "both discovery requests"); + controller.abort(reason); + const result = await operation; + if (result.ok) throw new Error("expected cancellation"); + expect(result.error.cause).toBe(reason); + await awaitAcknowledgement( + closed, + new Promise(() => undefined), + "cancelled discovery sockets", + ); + } finally { + server.closeAllConnections(); + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())), + ); + await provider.close(); + } +}); diff --git a/src/inspector/V8InspectorEndpoint.ts b/src/inspector/V8InspectorEndpoint.ts index 05a3c198ef..6a63745667 100644 --- a/src/inspector/V8InspectorEndpoint.ts +++ b/src/inspector/V8InspectorEndpoint.ts @@ -39,10 +39,24 @@ export const discoverV8Inspector = async ( operation: BrowserObservationOperation, signal?: AbortSignal, ): Promise => { - const [versionInput, targetsInput] = await Promise.all([ - readCdpJson(new URL("/json/version", endpoint), operation, signal), - readCdpJson(new URL("/json/list", endpoint), operation, signal), - ]); + const controller = new AbortController(); + const discoverySignal = + signal === undefined + ? controller.signal + : AbortSignal.any([signal, controller.signal]); + const reads = [ + readCdpJson(new URL("/json/version", endpoint), operation, discoverySignal), + readCdpJson(new URL("/json/list", endpoint), operation, discoverySignal), + ] as const; + let versionInput: unknown; + let targetsInput: unknown; + try { + [versionInput, targetsInput] = await Promise.all(reads); + } catch (cause: unknown) { + controller.abort(cause); + await Promise.allSettled(reads); + throw cause; + } const version = parseCdpEndpointValue(versionSchema, versionInput, operation); const { targets, limitations, ambiguousTargetIds } = parseCdpDiscoveryTargets( targetsInput,