diff --git a/src/js/internal/inspector/cdp.ts b/src/js/internal/inspector/cdp.ts index 259dcca85a8a..02a4e9c63a8c 100644 --- a/src/js/internal/inspector/cdp.ts +++ b/src/js/internal/inspector/cdp.ts @@ -7,13 +7,44 @@ // JSC-protocol JSON from the backend connection. Command ids from the client // are preserved by giving backend commands their own id space and correlating // the responses. + +// Type-only, so the builtin bundler erases it. +import type { JSC } from "../../../../packages/bun-inspector-protocol/src/protocol/jsc/index.d.ts"; + const { pathToFileURL, fileURLToPath } = require("node:url"); const { isAbsolute } = require("node:path"); const EXECUTION_CONTEXT_ID = 1; +// CDP (client-facing) shapes stay untyped. type AnyObject = Record; +type BackendResult = JSC.ResponseMap[keyof JSC.ResponseMap]; + +// BackendDispatcher::sendPendingErrors in InspectorBackendDispatcher.cpp. +type BackendError = { code: number; message: string }; + +// A response to one of this adapter's commands, or an event. +type BackendMessage = { + id?: number | null; + result?: BackendResult; + error?: BackendError; + method?: string; + params?: unknown; +}; + +// Discriminated on `method`, which JSC.Event's default instantiation is not. +type BackendEvent = { [M in keyof JSC.EventMap]: { method: M; params: JSC.EventMap[M] } }[keyof JSC.EventMap]; + +// The JSC response answering each CDP command that #translateResult reshapes. +type TranslatedResponses = { + "Runtime.evaluate": JSC.Runtime.EvaluateResponse | JSC.Runtime.AwaitPromiseResponse; + "Runtime.callFunctionOn": JSC.Runtime.CallFunctionOnResponse; + "Debugger.evaluateOnCallFrame": JSC.Debugger.EvaluateOnCallFrameResponse; + "Runtime.getProperties": JSC.Runtime.GetPropertiesResponse; + "Debugger.getPossibleBreakpoints": JSC.Debugger.GetBreakpointLocationsResponse; +}; + function toCdpUrl(url: string): string { // V8 reports filesystem-backed scripts with file:// URLs; JSC script URLs // are usually plain absolute paths. @@ -54,7 +85,7 @@ function breakpointUrlRegex(url: string): string { return Array.from(candidates, candidate => `^${escapeRegex(candidate)}$`).join("|"); } -const SCOPE_TYPE_MAP: Record = { +const SCOPE_TYPE_MAP: Record = { global: "global", with: "with", closure: "closure", @@ -68,7 +99,7 @@ const SCOPE_TYPE_MAP: Record = { // { type: "log", level: "warning"/"error"/... }, so a type-level match on "log" // would mask the level. #translateConsoleMessage falls through to // CONSOLE_LEVEL_MAP for those and for console.log itself. -const CONSOLE_TYPE_MAP: Record = { +const CONSOLE_TYPE_MAP: Partial, string>> = { dir: "dir", dirxml: "dirxml", table: "table", @@ -83,7 +114,7 @@ const CONSOLE_TYPE_MAP: Record = { profileEnd: "profileEnd", }; -const CONSOLE_LEVEL_MAP: Record = { +const CONSOLE_LEVEL_MAP: Record = { log: "log", info: "info", warning: "warning", @@ -98,7 +129,12 @@ class InspectorCDPAdapter { #nextExceptionId = 1; #pending = new Map< number, - { clientId: number | string | null; method: string; onResult?: (result: AnyObject, error?: AnyObject) => void } + { + clientId: number | string | null; + method: string; + // Typed per command at #sendToBackend. + onResult?: (result: any, error?: BackendError) => void; + } >(); #scripts = new Map(); @@ -125,7 +161,7 @@ class InspectorCDPAdapter { } handleBackendMessage(message: string): void { - let parsed: AnyObject; + let parsed: BackendMessage; try { parsed = JSON.parse(message); } catch { @@ -150,7 +186,7 @@ class InspectorCDPAdapter { return; } if (typeof method === "string") { - this.#translateBackendEvent(method, parsed.params || {}); + this.#translateBackendEvent({ method, params: parsed.params || {} } as BackendEvent); } } @@ -168,13 +204,13 @@ class InspectorCDPAdapter { // `clientId` undefined/null marks an adapter-internal command whose response // is dropped instead of being forwarded to the client. `onResult` intercepts - // the response for adapter-side chaining (e.g. Runtime.evaluate awaitPromise). - #sendToBackend( - method: string, - params?: AnyObject, + // the response for adapter-side chaining; on a backend error it gets `{}`. + #sendToBackend( + method: M, + params?: JSC.RequestMap[M], clientId: number | string | null = null, - clientMethod = method, - onResult?: (result: AnyObject, error?: AnyObject) => void, + clientMethod: string = method, + onResult?: (result: JSC.ResponseMap[M], error?: BackendError) => void, ): void { const id = this.#nextBackendId++; this.#pending.$set(id, { clientId, method: clientMethod, onResult }); @@ -220,7 +256,7 @@ class InspectorCDPAdapter { case "Runtime.evaluate": { // JSC's JSGlobalObjectRuntimeAgent rejects any contextId ("only one // execution context"), so drop it even though CDP clients echo it. - const jscParams = { + const jscParams: JSC.Runtime.EvaluateRequest = { expression: params.expression, objectGroup: params.objectGroup, includeCommandLineAPI: params.includeCommandLineAPI, @@ -292,7 +328,7 @@ class InspectorCDPAdapter { case "Runtime.callFunctionOn": { const { objectId, executionContextId } = params; - const forward = (targetObjectId: unknown) => + const forward = (targetObjectId: JSC.Runtime.RemoteObjectId) => this.#sendToBackend( "Runtime.callFunctionOn", { @@ -339,7 +375,7 @@ class InspectorCDPAdapter { case "Runtime.releaseObject": case "Runtime.releaseObjectGroup": - this.#sendToBackend(method, params, id, method); + this.#sendToBackend(method, params as JSC.RequestMap[typeof method], id, method); return; case "Runtime.getIsolateId": @@ -382,7 +418,7 @@ class InspectorCDPAdapter { case "Debugger.removeBreakpoint": case "Debugger.continueToLocation": case "Debugger.getScriptSource": - this.#sendToBackend(method, params, id, method); + this.#sendToBackend(method, params as JSC.RequestMap[typeof method], id, method); return; case "Debugger.setPauseOnExceptions": @@ -400,9 +436,9 @@ class InspectorCDPAdapter { case "Debugger.setBreakpointByUrl": { const { condition, urlRegex, url } = params; - const options: AnyObject = {}; + const options: JSC.Debugger.BreakpointOptions = {}; if (condition) options.condition = condition; - const jscParams: AnyObject = { + const jscParams: JSC.Debugger.SetBreakpointByUrlRequest = { lineNumber: params.lineNumber, columnNumber: params.columnNumber, options, @@ -520,14 +556,16 @@ class InspectorCDPAdapter { } } - #translateResult(method: string, result: AnyObject): AnyObject { + // `method` is the CDP command being answered; see TranslatedResponses. + #translateResult(method: string, response: BackendResult): AnyObject { switch (method) { case "Debugger.enable": - return { debuggerId: "(bun)", ...result }; + return { debuggerId: "(bun)", ...response }; case "Runtime.evaluate": case "Runtime.callFunctionOn": case "Debugger.evaluateOnCallFrame": { + const result = response as TranslatedResponses[typeof method]; const out: AnyObject = { result: result.result ?? { type: "undefined" } }; if (result.wasThrown) { out.exceptionDetails = { @@ -542,7 +580,8 @@ class InspectorCDPAdapter { } case "Runtime.getProperties": { - const properties = (result.properties ?? []).map((property: AnyObject) => ({ + const result = response as TranslatedResponses[typeof method]; + const properties = (result.properties ?? []).map(property => ({ configurable: false, enumerable: false, ...property, @@ -553,15 +592,17 @@ class InspectorCDPAdapter { return out; } - case "Debugger.getPossibleBreakpoints": + case "Debugger.getPossibleBreakpoints": { + const result = response as TranslatedResponses[typeof method]; return { locations: result.locations ?? [] }; + } default: - return result; + return response; } } - #translateBackendEvent(method: string, params: AnyObject): void { + #translateBackendEvent({ method, params }: BackendEvent): void { switch (method) { case "Debugger.scriptParsed": { const url = params.sourceURL || params.url || ""; @@ -590,12 +631,12 @@ class InspectorCDPAdapter { } case "Debugger.paused": { - const callFrames = (params.callFrames ?? []).map((frame: AnyObject) => ({ + const callFrames = (params.callFrames ?? []).map(frame => ({ callFrameId: frame.callFrameId, functionName: frame.functionName ?? "", location: frame.location, url: this.#scripts.$get(frame.location?.scriptId)?.cdpUrl ?? "", - scopeChain: (frame.scopeChain ?? []).map((scope: AnyObject) => ({ + scopeChain: (frame.scopeChain ?? []).map(scope => ({ type: SCOPE_TYPE_MAP[scope.type] ?? "closure", object: scope.object, name: scope.name, @@ -612,9 +653,11 @@ class InspectorCDPAdapter { case "assert": cdpParams.reason = "assert"; break; - case "Breakpoint": - if (data?.breakpointId) cdpParams.hitBreakpoints = [data.breakpointId]; + case "Breakpoint": { + const hit = data as JSC.Debugger.BreakpointPauseReason | undefined; + if (hit?.breakpointId) cdpParams.hitBreakpoints = [hit.breakpointId]; break; + } } if (asyncStackTrace) cdpParams.asyncStackTrace = this.#translateStackTrace(asyncStackTrace); this.#emitToClient("Debugger.paused", cdpParams); @@ -637,7 +680,7 @@ class InspectorCDPAdapter { return; case "Console.messageAdded": - this.#translateConsoleMessage(params.message || {}); + this.#translateConsoleMessage(params.message); return; default: @@ -646,10 +689,10 @@ class InspectorCDPAdapter { } } - #translateStackTrace(stackTrace: AnyObject | undefined): AnyObject | undefined { + #translateStackTrace(stackTrace: JSC.Console.StackTrace | undefined): AnyObject | undefined { if (!stackTrace) return undefined; const translated: AnyObject = { - callFrames: (stackTrace.callFrames ?? []).map((frame: AnyObject) => ({ + callFrames: (stackTrace.callFrames ?? []).map(frame => ({ functionName: frame.functionName ?? "", scriptId: frame.scriptId ?? "", url: toCdpUrl(frame.url ?? ""), @@ -664,7 +707,7 @@ class InspectorCDPAdapter { return translated; } - #translateConsoleMessage(message: AnyObject): void { + #translateConsoleMessage(message: JSC.Console.ConsoleMessage): void { const level = message.level ?? "log"; const args = message.parameters?.length ? message.parameters : [{ type: "string", value: message.text ?? "" }]; diff --git a/test/cli/inspect/bun-inspector-protocol.test.ts b/test/cli/inspect/bun-inspector-protocol.test.ts index 64d58cc7936b..acb5d524aa91 100644 --- a/test/cli/inspect/bun-inspector-protocol.test.ts +++ b/test/cli/inspect/bun-inspector-protocol.test.ts @@ -1,14 +1,19 @@ // packages/bun-inspector-protocol ships a snapshot of the inspector protocol of the WebKit // build bun links against (src/protocol/jsc/protocol.json, from which index.d.ts is -// generated). Nothing regenerates it when WebKit is bumped, so this test runs a short +// generated). Nothing regenerates it when WebKit is bumped, so the first test runs a short // debugging session against this build of bun and validates every message it sends // against the snapshot. If it fails after a WebKit upgrade, regenerate the snapshot: // // bun packages/bun-inspector-protocol/scripts/generate-protocol.ts +// +// The second test typechecks src/js/internal/inspector/cdp.ts, the node:inspector CDP adapter, +// which reads JSC's messages through index.d.ts. Regenerating the snapshot therefore also +// reports every field the adapter still reads under a name WebKit no longer sends. import { spawn } from "bun"; import { expect, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; -import { basename } from "node:path"; +import { bunEnv, bunExe, nodeExe, tempDir } from "harness"; +import { readFileSync } from "node:fs"; +import { basename, join } from "node:path"; import protocolJson from "../../../packages/bun-inspector-protocol/src/protocol/jsc/protocol.json"; import type { Property, Protocol } from "../../../packages/bun-inspector-protocol/src/protocol/schema"; @@ -262,3 +267,56 @@ test("the protocol snapshot in packages/bun-inspector-protocol matches what bun ]), ); }); + +const snapshotPath = join(import.meta.dir, "../../../packages/bun-inspector-protocol/src/protocol/jsc/index.d.ts"); + +/** + * Typechecks cdp.ts (see cdp-protocol-types-fixture.mts) and returns the diagnostics as + * `file:line: message` strings. `snapshotReplacement` is a file to use in place of index.d.ts. + * + * Runs under node rather than in this process: the debug build of bun spends tens of seconds + * transpiling typescript.js alone, and the check has nothing to do with the bun under test. + */ +async function typecheckCdpAdapter(snapshotReplacement?: string): Promise { + await using proc = spawn({ + cmd: [ + nodeExe()!, + join(import.meta.dir, "cdp-protocol-types-fixture.mts"), + ...(snapshotReplacement === undefined ? [] : [snapshotReplacement]), + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + return JSON.parse(stdout); +} + +test.skipIf(!nodeExe())( + "src/js/internal/inspector/cdp.ts reads JSC messages through the snapshot's types", + async () => { + // Renaming things in the snapshot has to surface at the adapter's uses of them, otherwise the + // typecheck below proves nothing. One event parameter, one response field, one request parameter. + const renamed = ["scriptType", "wasThrown", "doNotPauseOnExceptionsAndMuteConsole"]; + let snapshot = readFileSync(snapshotPath, "utf8"); + for (const name of renamed) { + const withRename = snapshot.replaceAll(new RegExp(`\\b${name}\\b`, "g"), `${name}Renamed`); + if (withRename === snapshot) throw new Error(`${name} is no longer in the snapshot; rename something else here`); + snapshot = withRename; + } + using dir = tempDir("cdp-protocol-types", { "index.d.ts": snapshot }); + + const [diagnostics, diagnosticsAfterRenames] = await Promise.all([ + typecheckCdpAdapter(), + typecheckCdpAdapter(join(String(dir), "index.d.ts")), + ]); + // Failures here after regenerating the snapshot are the fields WebKit renamed or dropped that + // cdp.ts still reads or sends, one diagnostic per use site. + expect(diagnostics).toEqual([]); + expect( + renamed.filter(name => diagnosticsAfterRenames.some(diagnostic => diagnostic.includes(`'${name}'`))), + ).toEqual(renamed); + }, +); diff --git a/test/cli/inspect/cdp-protocol-types-fixture.mts b/test/cli/inspect/cdp-protocol-types-fixture.mts new file mode 100644 index 000000000000..aaa4205dd23a --- /dev/null +++ b/test/cli/inspect/cdp-protocol-types-fixture.mts @@ -0,0 +1,53 @@ +// Run under node by bun-inspector-protocol.test.ts: typechecks src/js/internal/inspector/cdp.ts +// with the options of `tsc -p src/js` and prints the diagnostics as a JSON array of +// "file:line: message" strings. An optional argument is a file whose contents stand in for the +// protocol snapshot (packages/bun-inspector-protocol/src/protocol/jsc/index.d.ts) cdp.ts imports. +// +// builtins.d.ts declares the `$` intrinsics cdp.ts uses. What the real project layers on top of it, +// the build's codegen output that builtins.d.ts references and the @types auto-include (between +// them, the per-module typing of `require()`), is left out: the codegen output does not exist in a +// test checkout and none of it bears on the JSC side, so `require()` is declared loosely instead. +import { readFileSync } from "node:fs"; +import { basename, join } from "node:path"; +import ts from "typescript"; + +const repoRoot = join(import.meta.dirname, "../../.."); +const jsDir = join(repoRoot, "src/js"); +const configPath = join(jsDir, "tsconfig.json"); +const builtinsPath = join(jsDir, "builtins.d.ts"); +const snapshotPath = join(repoRoot, "packages/bun-inspector-protocol/src/protocol/jsc/index.d.ts"); +// Only exists inside the compiler host below. +const requireStubPath = join(jsDir, "require.d.ts"); +const snapshotReplacement = process.argv[2] === undefined ? undefined : readFileSync(process.argv[2], "utf8"); + +// TypeScript reports paths with forward slashes on every platform. +function samePath(a: string, b: string): boolean { + return a.replaceAll("\\", "/") === b.replaceAll("\\", "/"); +} + +const { config, error } = ts.readConfigFile(configPath, ts.sys.readFile); +if (error) throw new Error(ts.flattenDiagnosticMessageText(error.messageText, "\n")); +const { options, fileNames } = ts.parseJsonConfigFileContent( + { ...config, include: undefined, files: ["internal/inspector/cdp.ts", "builtins.d.ts"] }, + ts.sys, + jsDir, + { composite: false, types: [] }, + configPath, +); + +const host = ts.createCompilerHost(options); +const { fileExists, readFile } = host; +host.fileExists = file => samePath(file, requireStubPath) || fileExists(file); +host.readFile = file => { + if (samePath(file, requireStubPath)) return "declare function require(id: string): any;\n"; + if (samePath(file, snapshotPath) && snapshotReplacement !== undefined) return snapshotReplacement; + const text = readFile(file); + return samePath(file, builtinsPath) ? text?.replaceAll(/^\/\/\/ { + const where = file ? `${basename(file.fileName)}:${file.getLineAndCharacterOfPosition(start!).line + 1}: ` : ""; + return where + ts.flattenDiagnosticMessageText(messageText, "\n"); +}); +console.log(JSON.stringify(diagnostics));