diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index 08b444b..3ac38af 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -5,11 +5,11 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten ## The copies are intentional - `stacks/lint` and `stacks/test` are deliberate near-verbatim copies of the upstream extensions, kept close to upstream so changes can be synced by diffing. Do NOT deduplicate or refactor across the two stacks — the duplication is the point; consolidation is a later, explicit phase. -- The copies diverge from upstream in exactly ten ways (the "adaptations" below). When syncing upstream, preserve them. An eleventh divergence is either a bug or must be added to this list. +- The copies diverge from upstream in exactly eleven ways (the "adaptations" below). When syncing upstream, preserve them. A twelfth divergence is either a bug or must be added to this list. - **Tracked upstream state.** `stacks/lint` tracks web-infra-dev/rslint `packages/vscode-extension` at **e5d76242** (0.9.3); `stacks/test` tracks web-infra-dev/rstest `packages/vscode` at **41daaa1b** (0.12.2). Additional ports: Rslint **84f9c9b5** trace wording and **72cd2b1f** fixAll waits; Rstest **988f8e1d** per-bundle license notices, with explicit native-binding inclusion and no network license-text replenishment. We replace upstream's plugin-host failure toast with status (adaptations 4/7). Diff `CoreResolver.ts`, `RuntimeManager.ts`, `WorkspaceDocumentRouter.ts`, and `Rslint.ts` on future syncs. The rstest#1888 IPC port uses default JSON instead of advanced serialization: values must survive a JSON round-trip, and both `createBirpc` calls spread `rpcErrorCodec` (`stacks/test/shared/rpc.ts`) to preserve thrown errors. Failed-project retry and worker cleanup remain ahead of upstream. - **Ahead of upstream — offer these back when syncing** (bug fixes, not adaptations): (1) `RuntimeManager.reconcile` resolves the document's core **before** sweeping pending uses (`planDocumentCore`), so a reconcile landing on the key a pending start is already producing adopts that start instead of tearing it down mid-`initialize` — the teardown made vscode-languageclient force-notify ("couldn't create connection to server") whenever the register-time pass, a detection change and `didOpen` landed inside one worker startup window (`tests/stacks/lint/runtimeManager.test.ts`). (2) `Rslint.close()` gives a still-Starting language client a bounded chance to settle before tearing down its transport, so a legitimate mid-start close (document closed during start, core key changed) stops cleanly instead of triggering the same force-notified toasts. (3) The registry-harness E2E gives its never-settling startup operation 500ms to begin and accepts only the in-flight timeout message, so a stalled runner cannot satisfy the assertion through the already-expired path (`e2e/lint/suite/registry-harness.test.ts`). (4) `Project.retryFailedConfig()` keeps a failed Rstest project and retries its config evaluation in place with one single-flight promise, so repeated dependency-change passes neither overlap workers nor repeat an unchanged not-installed warning. (5) `RuntimeManager` retires a stopped client even when its resolved key is unchanged. The existing closing barrier and pending-use adoption share one replacement across documents; running and starting clients remain untouched (`tests/stacks/lint/runtimeManager.test.ts`). -## The ten adaptations +## The eleven adaptations 1. **Shell activation** — stacks never self-activate; `register()` returns fast and never blocks on starting a server/worker. 2. **Namespace** — everything user-visible is `rstack.*`. Legacy `rslint.*` / `rstest.*` settings and command ids are not read, aliased or migrated (breaking old settings and keybindings was an accepted cost). @@ -23,6 +23,8 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten 10. **Test file ownership** (test) — publication mirrors the CLI per project. When a request carries the same file or the same case from several projects (merged gutter, run-at-cursor, run-current-file; all three profiles reach `startTestRun`), only items of the deepest project root run and the others are reported skipped to clear stale merged gutter state — implemented in `runRouting.ts`, `index.ts` (`startTestRun`), and `master.ts` (`RstestApi.project` made public). Explicit single-project selections, project/folder/Run All, and `rstack.rstest.runInTerminal` (VS Code supplies one explicitly picked item) keep CLI scope. +11. **Debugger-owned test processes** (test) — debug runs use js-debug `launch` with child-process auto-attach instead of upstream's `--inspect-wait` plus `attach`. js-debug owns inspector endpoints and process teardown; `debuggerPort` / `debuggerAddress` are removed without migration. `debugWorker.ts` owns the session and a debug-only local socket carrying JSON birpc messages (`shared/socketRpc.ts`) after a first-line per-launch secret authenticates the worker; normal runs retain Node IPC. Preserve the 9229-occupied breakpoint and continue/cancel/stop cleanup regression in `e2e/rstest/suite/debug.test.ts` when syncing. + ## Rules - **Pre-1.0.0 the extension breaks freely.** No compatibility is owed with earlier unpublished states of this extension — settings, command ids and behavior may change without deprecation paths, and dead compat code for them is removed, not kept. No settings migration exists either — not for earlier states of this extension, and not for the two retired standalone extensions (removed in #15; users re-enter their settings under `rstack.*`). Testing and fixtures track only the latest published releases, pinned exactly and bumped by Renovate; a green E2E run speaks only for those releases. `SUPPORT_MATRIX` floors are the minimum versions the extension accepts: each entry is the lowest release evidence shows works with the current code, and its comment records that evidence. Move a floor only when a change makes older releases stop working, never because a devDependency or fixture moved. Raising a floor needs no transition story; the status names the required version. diff --git a/packages/vscode/README.md b/packages/vscode/README.md index 4c46b9c..181c3e7 100644 --- a/packages/vscode/README.md +++ b/packages/vscode/README.md @@ -90,8 +90,6 @@ All settings live under the unified `rstack.*` namespace. There are no `rslint.* | `rstack.rstest.debugNodeEnv` | `null` | Extra env when debugging tests. | | `rstack.rstest.debugExclude` | `["/**"]` | Debug `skipFiles`. | | `rstack.rstest.debugOutFiles` | `[]` | Debug `outFiles`. | -| `rstack.rstest.debuggerPort` | — | Debugger port. | -| `rstack.rstest.debuggerAddress` | — | Debugger address. | | `rstack.rstest.terminalShellPath` | — | Shell used by **Run in Terminal**. | | `rstack.rstest.terminalShellArgs` | `[]` | Shell args for **Run in Terminal**. | | `rstack.fmt.enable` | `true` | Enable/disable the formatter integration. | diff --git a/packages/vscode/e2e/rstest/suite/debug.test.ts b/packages/vscode/e2e/rstest/suite/debug.test.ts new file mode 100644 index 0000000..514d45a --- /dev/null +++ b/packages/vscode/e2e/rstest/suite/debug.test.ts @@ -0,0 +1,166 @@ +import assert from 'node:assert'; +import net from 'node:net'; +import path from 'node:path'; +import vscode from 'vscode'; +import { + FIXTURES_ROOT, + getRstestExports, + getTestItemByLabels, + waitFor, +} from './helpers'; + +suite('Rstest debug launch', () => { + for (const finish of ['continue', 'cancel', 'stop'] as const) { + test(`hits a breakpoint with port 9229 occupied and cleans up after ${finish}`, async () => { + const occupied = net.createServer(); + await new Promise((resolve, reject) => { + occupied.once('error', reject); + occupied.listen(9229, '127.0.0.1', resolve); + }); + const cancellation = new vscode.CancellationTokenSource(); + const sessions = new Map(); + const processIds = new Set(); + const disposables: vscode.Disposable[] = []; + const file = path.join(FIXTURES_ROOT, 'workspace-1/test/index.test.ts'); + const breakpoint = new vscode.SourceBreakpoint( + new vscode.Location(vscode.Uri.file(file), new vscode.Position(5, 0)), + ); + let stopped: + { session: vscode.DebugSession; threadId: number } | undefined; + const passed: vscode.TestItem[] = []; + let ended = false; + let running: Promise | undefined; + try { + disposables.push( + vscode.debug.onDidStartDebugSession((session) => { + sessions.set(session.id, session); + }), + vscode.debug.onDidTerminateDebugSession((session) => + sessions.delete(session.id), + ), + vscode.debug.registerDebugAdapterTrackerFactory('*', { + // js-debug resolves the public 'node' type to 'pwa-node'. + createDebugAdapterTracker: (session) => ({ + onDidSendMessage: (message) => { + if ( + message.type === 'event' && + message.event === 'stopped' && + message.body.reason === 'breakpoint' + ) { + stopped = { session, threadId: message.body.threadId }; + } + }, + }), + }), + ); + vscode.debug.addBreakpoints([breakpoint]); + const api = await getRstestExports(); + const item = await waitFor(() => + getTestItemByLabels(api.testController.items, [ + 'test', + 'index.test.ts', + ]), + ); + const request = new vscode.TestRunRequest( + [item], + undefined, + api.debugProfile, + ); + // A real TestRun is required for startDebugging's testRun association. + running = api.startTestRun( + request, + cancellation.token, + false, + (runRequest) => { + const run = api.testController.createTestRun(runRequest); + const originalPassed = run.passed.bind(run); + const originalEnd = run.end.bind(run); + run.passed = (test, duration) => { + passed.push(test); + originalPassed(test, duration); + }; + run.end = () => { + ended = true; + originalEnd(); + }; + return run; + }, + ); + await waitFor( + () => assert.ok(stopped, 'debuggee should stop at the breakpoint'), + { timeoutMs: 60_000 }, + ); + assert.ok(stopped); + const stack = await stopped.session.customRequest('stackTrace', { + threadId: stopped.threadId, + }); + assert.equal( + path.normalize(stack.stackFrames[0].source.path), + path.normalize(file), + ); + assert.equal(stack.stackFrames[0].line, 6); + const root = [...sessions.values()].find( + (session) => session.name === 'Rstest Debug', + ); + assert.ok(root); + for (const session of sessions.values()) { + if (session.id === root.id) continue; + const evaluation = await session.customRequest('evaluate', { + expression: 'process.pid', + context: 'repl', + }); + const pid = Number(evaluation.result); + assert.ok(Number.isInteger(pid) && pid > 0, evaluation.result); + processIds.add(pid); + } + assert.ok( + processIds.size >= 2, + 'worker and pool child must be debugged', + ); + if (finish === 'continue') { + await stopped.session.customRequest('continue', { + threadId: stopped.threadId, + }); + } else if (finish === 'cancel') { + cancellation.cancel(); + } else { + await vscode.debug.stopDebugging(root); + } + await waitFor( + () => { + assert.equal(ended, true, 'TestRun must end'); + if (finish === 'continue') { + assert.deepEqual(passed.map((test) => test.label).sort(), [ + 'Index', + 'index.test.ts', + 'should add two numbers correctly', + 'should test source code correctly', + ]); + } + assert.equal(sessions.size, 0, 'all debug sessions must end'); + for (const pid of processIds) { + assert.throws( + () => process.kill(pid, 0), + { code: 'ESRCH' }, + `process ${pid} must exit`, + ); + } + }, + { timeoutMs: 60_000 }, + ); + await running; + } finally { + cancellation.cancel(); + vscode.debug.removeBreakpoints([breakpoint]); + await Promise.all( + [...sessions.values()].map((session) => + vscode.debug.stopDebugging(session), + ), + ); + for (const disposable of disposables) disposable.dispose(); + cancellation.dispose(); + await new Promise((resolve) => occupied.close(() => resolve())); + } + }); + } +}); diff --git a/packages/vscode/e2e/rstest/suite/helpers.ts b/packages/vscode/e2e/rstest/suite/helpers.ts index 6c41c09..899f05f 100644 --- a/packages/vscode/e2e/rstest/suite/helpers.ts +++ b/packages/vscode/e2e/rstest/suite/helpers.ts @@ -14,6 +14,7 @@ import type { RstackExtensionExports } from '../../../src/types'; export interface RstestExports { testController: vscode.TestController; runProfile: vscode.TestRunProfile; + debugProfile: vscode.TestRunProfile; getResolvedRstestPath: (sourceUri: string) => string | undefined; startTestRun: ( request: vscode.TestRunRequest, diff --git a/packages/vscode/package.json b/packages/vscode/package.json index 9d55dfc..bd04f6e 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -274,18 +274,6 @@ "scope": "resource", "markdownDescription": "When source maps are enabled, glob patterns locating the generated JavaScript files (maps to the debug session's `outFiles`)." }, - "rstack.rstest.debuggerPort": { - "order": 11, - "type": "number", - "scope": "resource", - "description": "Port the debugger attaches to. Defaults to Node's inspector behavior (9229, or a free port if taken)." - }, - "rstack.rstest.debuggerAddress": { - "order": 12, - "type": "string", - "scope": "resource", - "description": "TCP/IP address the debugger attaches to. Defaults to localhost." - }, "rstack.rstest.terminalShellPath": { "order": 13, "type": "string", diff --git a/packages/vscode/src/stacks/test/config.ts b/packages/vscode/src/stacks/test/config.ts index 1d2d0bf..d1b0428 100644 --- a/packages/vscode/src/stacks/test/config.ts +++ b/packages/vscode/src/stacks/test/config.ts @@ -4,7 +4,6 @@ import { fallback, type InferOutput, literal, - number, object, optional, parse, @@ -30,8 +29,6 @@ const configSchema = object({ nodeExecArgs: fallback(array(string()), []), nodeEnv: fallback(optional(record(string(), string())), undefined), debugNodeEnv: fallback(optional(record(string(), string())), undefined), - debuggerPort: fallback(optional(number()), undefined), - debuggerAddress: fallback(optional(string()), undefined), debugExclude: fallback(array(string()), ['/**']), debugOutFiles: fallback(array(string()), []), configFileGlobPattern: fallback(array(string()), [ diff --git a/packages/vscode/src/stacks/test/debugWorker.ts b/packages/vscode/src/stacks/test/debugWorker.ts new file mode 100644 index 0000000..c37a66b --- /dev/null +++ b/packages/vscode/src/stacks/test/debugWorker.ts @@ -0,0 +1,162 @@ +import { randomUUID } from 'node:crypto'; +import { mkdtempSync, rmSync } from 'node:fs'; +import net from 'node:net'; +import os from 'node:os'; +import path from 'node:path'; +import { createBirpc } from 'birpc'; +import vscode from 'vscode'; +import { rpcErrorCodec } from './shared/rpc'; +import { + DEBUG_PIPE_ENV, + DEBUG_PIPE_TOKEN_ENV, + socketRpc, +} from './shared/socketRpc'; +import type { TestRunReporter } from './testRunReporter'; +import type { Worker } from './worker'; +import { logger } from './logger'; + +/** Own the launch session and its local RPC socket as one lifetime. */ +export function createDebugWorker( + reporter: TestRunReporter, + onClose: (reason?: Error) => void, +) { + const id = randomUUID(); + const secret = randomUUID(); + const prefix = path.join(os.tmpdir(), 'rstest-'); + const directory = + process.platform === 'win32' + ? undefined + : mkdtempSync( + Buffer.byteLength(`${prefix}XXXXXX/rpc`) >= + (process.platform === 'darwin' ? 104 : 108) + ? '/tmp/rstest-' + : prefix, + ); + const endpoint = directory + ? path.join(directory, 'rpc') + : `\\\\.\\pipe\\rstest-${id}`; + const server = net.createServer(); + const pending = new Set(); + let socket: net.Socket | undefined; + let rpc: ReturnType | undefined; + let receive: ((data: unknown) => void) | undefined; + let session: vscode.DebugSession | undefined; + const ready = Promise.withResolvers(); + // Closing before start() must not produce an unhandled rejection. + void ready.promise.catch(() => {}); + const subscriptions: vscode.Disposable[] = []; + let reason: Error | undefined; + const stop = () => { + if (session) { + void vscode.debug.stopDebugging(session).then(undefined, (error) => { + logger.debug('Failed to stop Rstest debug session', error); + }); + } + }; + const worker = createBirpc(reporter, { + post: (data) => rpc?.post(data), + on: (fn) => { + receive = fn; + }, + bind: 'functions', + ...rpcErrorCodec, + timeout: 600_000, + off: () => { + ready.reject(reason ?? new Error('Rstest debug worker stopped')); + socket?.destroy(); + for (const connection of pending) connection.destroy(); + server.close(); + if (directory) rmSync(directory, { recursive: true, force: true }); + stop(); + for (const disposable of subscriptions) disposable.dispose(); + onClose(reason); + }, + }); + const fail = (error: Error) => { + reason ??= error; + if (!worker.$closed) worker.$close(error); + }; + server.on('error', fail); + server.on('connection', (connection) => { + if (socket) { + connection.destroy(); + return; + } + pending.add(connection); + const transport = socketRpc(connection, (token) => { + if (token !== secret || socket) return false; + pending.delete(connection); + socket = connection; + rpc = transport; + ready.resolve(); + return true; + }); + transport.on((data) => receive?.(data)); + connection.on('error', (error) => { + if (connection === socket) fail(error); + else connection.destroy(); + }); + connection.on('close', () => { + pending.delete(connection); + if (connection === socket) + fail(new Error('Rstest debug worker disconnected')); + }); + }); + + return { + worker, + async start( + workspace: vscode.WorkspaceFolder, + configuration: vscode.DebugConfiguration, + testRun?: vscode.TestRun, + token?: vscode.CancellationToken, + ) { + const matches = (candidate: vscode.DebugSession) => + candidate.configuration.rstestDebugId === id; + subscriptions.push( + vscode.debug.onDidStartDebugSession((candidate) => { + if (!matches(candidate)) return; + session = candidate; + }), + vscode.debug.onDidTerminateDebugSession((candidate) => { + if (matches(candidate)) fail(new Error('Rstest debug session ended')); + }), + ); + if (token) + subscriptions.push( + token.onCancellationRequested(() => + fail(new Error('Rstest debug run cancelled')), + ), + ); + try { + if (token?.isCancellationRequested) { + throw new Error('Rstest debug run cancelled'); + } + server.listen(endpoint); + const launch = Promise.resolve( + vscode.debug.startDebugging( + workspace, + { + ...configuration, + rstestDebugId: id, + env: { + ...configuration.env, + [DEBUG_PIPE_ENV]: endpoint, + [DEBUG_PIPE_TOKEN_ENV]: secret, + }, + }, + { testRun }, + ), + ).then((started) => { + if (!started) throw new Error('Failed to launch Rstest debug worker'); + }); + await Promise.all([launch, ready.promise]); + if (worker.$closed) + throw new Error('Rstest debug worker stopped during launch'); + } catch (error) { + fail(error as Error); + throw error; + } + }, + }; +} diff --git a/packages/vscode/src/stacks/test/index.ts b/packages/vscode/src/stacks/test/index.ts index e07d5ac..f185926 100644 --- a/packages/vscode/src/stacks/test/index.ts +++ b/packages/vscode/src/stacks/test/index.ts @@ -68,6 +68,7 @@ class Rstest implements vscode.Disposable { private errorStore = new TestErrorStore(); private detection: DetectionSnapshot; private runProfile!: vscode.TestRunProfile; + private debugProfile!: vscode.TestRunProfile; private coverageProfile!: vscode.TestRunProfile; private disposed = false; @@ -100,6 +101,7 @@ class Rstest implements vscode.Disposable { hasNotInstalledState: () => status.hasNotInstalled(), testController: this.ctrl, runProfile: this.runProfile, + debugProfile: this.debugProfile, startTestRun: this.startTestRun, getResolvedRstestPath: (sourceUri: string) => { for (const workspace of this.workspaces.values()) { @@ -159,7 +161,7 @@ class Rstest implements vscode.Disposable { this.registerCommands(); - this.ctrl.createRunProfile( + this.debugProfile = this.ctrl.createRunProfile( 'Debug Tests', vscode.TestRunProfileKind.Debug, this.startTestRun, diff --git a/packages/vscode/src/stacks/test/master.ts b/packages/vscode/src/stacks/test/master.ts index c5310dd..388253e 100644 --- a/packages/vscode/src/stacks/test/master.ts +++ b/packages/vscode/src/stacks/test/master.ts @@ -1,7 +1,6 @@ import { spawn } from 'node:child_process'; import { statSync } from 'node:fs'; import { createRequire } from 'node:module'; -import net from 'node:net'; import path, { dirname } from 'node:path'; import { type BirpcReturn, createBirpc } from 'birpc'; import regexpEscape from 'core-js-pure/actual/regexp/escape'; @@ -19,6 +18,7 @@ import { getConfiguredNodeExecutable, } from '../../shared/nodeExecutableSetting'; import { CONFIG_SECTION, getConfigValue } from './config'; +import { createDebugWorker } from './debugWorker'; import { MessageLatch } from '../../shared/messageLatch'; import { formatNotInstalledLog, @@ -65,9 +65,16 @@ export const runningWorkers = new Set(); export const WATCHER_CLOSE_TIMEOUT_MS = 30_000; const forceKilledWorkers = new WeakSet(); const workerClosePromises = new WeakMap>(); +const debugWorkers = new WeakSet(); export const closeWorkerGracefully = (worker: WorkerRpc): Promise => { if (worker.$closed) return Promise.resolve(); + // A paused debuggee cannot answer closeWatcher. js-debug owns and terminates + // the entire process tree, including paused pool children. + if (debugWorkers.has(worker)) { + worker.$close(); + return Promise.resolve(); + } const pendingClose = workerClosePromises.get(worker); if (pendingClose) return pendingClose; const closePromise = (async () => { @@ -148,28 +155,9 @@ export const warmWorkerNodePreflight = ( } }; -// Default host for a fixed debug port. The spawn (`--inspect-wait`), the port -// preflight, and the attach config must all use the same host: on a dual-stack -// machine `localhost` can resolve to `::1` while the worker listens on IPv4, so -// the debugger would attach to the wrong endpoint. Prefer an explicit IPv4 -// literal over `localhost` so both ends agree. -const DEFAULT_DEBUG_HOST = '127.0.0.1'; - // The specifier used when `rstestPackagePath` is unset. const CORE_PACKAGE_JSON = '@rstest/core/package.json'; -// Probe whether a fixed inspector port can be bound. `--inspect-wait=host:port` -// does not fall back when the port is taken: Node reports address-in-use and -// runs the worker without the inspector, and attaching by that port could hit an -// unrelated process. Preflight so we fail with a clear message instead. -const isPortAvailable = (port: number, host?: string): Promise => - new Promise((resolve) => { - const server = net.createServer(); - server.once('error', () => resolve(false)); - server.once('listening', () => server.close(() => resolve(true))); - server.listen(port, host ?? DEFAULT_DEBUG_HOST); - }); - export class RstestApi { private workers = new Set(); private disposePromise?: Promise; @@ -618,8 +606,9 @@ export class RstestApi { testRunReporter, kind === vscode.TestRunProfileKind.Debug, run, + token, ); - token.onCancellationRequested(() => { + const cancellation = token.onCancellationRequested(() => { void closeWorkerGracefully(worker).finally(onFinish); }); @@ -651,7 +640,10 @@ export class RstestApi { onFinish(); }) .finally(() => { - if (!continuous) worker.$close(); + if (!continuous) { + cancellation.dispose(); + worker.$close(); + } }); await promise; @@ -742,6 +734,7 @@ export class RstestApi { testRunReporter = new TestRunReporter(), startDebugging?: boolean, testRun?: vscode.TestRun, + token?: vscode.CancellationToken, ) { // Cheap fast-fail; the load-bearing check is the one after the Node // preflight below, which covers a dispose landing mid-await. This one @@ -770,26 +763,6 @@ export class RstestApi { if (!paths) { throw new ReportedRstestResolutionError(); } - const debuggerPort = getConfigValue('debuggerPort', this.workspace); - const debuggerAddress = getConfigValue('debuggerAddress', this.workspace); - if ( - startDebugging && - debuggerPort && - !(await isPortAvailable(debuggerPort, debuggerAddress)) - ) { - const at = `${debuggerAddress ?? DEFAULT_DEBUG_HOST}:${debuggerPort}`; - const message = `Rstest debug port ${at} is already in use. Set a free "${CONFIG_SECTION}.debuggerPort" or free the port.`; - vscode.window.showErrorMessage(message); - throw new Error(message); - } - const execArgv: string[] = []; - if (startDebugging) { - execArgv.push( - debuggerPort - ? `--inspect-wait=${debuggerAddress ?? DEFAULT_DEBUG_HOST}:${debuggerPort}` - : '--inspect-wait', - ); - } const workerPath = path.resolve(__dirname, 'worker.js'); const { nodeExecutable, nodeExecArgs } = await this.resolveWorkerNodeCommand(); @@ -824,18 +797,67 @@ export class RstestApi { // the worker retracts it when the config disables color (adaptation #9, // shared/colorEnv.ts). injectForceColor(workerEnv); - const rstestProcess = spawn( - nodeExecutable, - [...nodeExecArgs, ...execArgv, workerPath], - { - cwd: this.cwd, - stdio: ['pipe', 'pipe', 'pipe', 'ipc'], - // Default JSON serialization: `advanced` uses the V8 serializer, whose - // format follows the V8 version, and Electron's V8 can be newer than - // the user's Node can read. - env: workerEnv, - }, - ); + if (startDebugging) { + const debugOutFiles = getConfigValue('debugOutFiles', this.workspace); + let started = false; + const debug = createDebugWorker(testRunReporter, (reason) => { + this.workers.delete(debug.worker); + runningWorkers.delete(debug.worker); + if ( + started && + reason && + !token?.isCancellationRequested && + !this.disposed + ) { + status.crashed( + `worker exited unexpectedly: ${reason.message}`, + this.statusSource, + ); + } + }); + this.workers.add(debug.worker); + runningWorkers.add(debug.worker); + debugWorkers.add(debug.worker); + try { + await debug.start( + this.workspace, + { + type: 'node', + name: 'Rstest Debug', + request: 'launch', + runtimeExecutable: nodeExecutable, + runtimeArgs: nodeExecArgs, + program: workerPath, + cwd: this.cwd, + env: workerEnv, + autoAttachChildProcesses: true, + skipFiles: getConfigValue('debugExclude', this.workspace), + ...(debugOutFiles.length ? { outFiles: debugOutFiles } : {}), + }, + testRun, + token, + ); + started = true; + status.workerSpawned(this.statusSource); + return { worker: debug.worker, ...paths }; + } catch (error) { + if (!token?.isCancellationRequested && !this.disposed) { + status.crashed( + `worker launch failed: ${toErrorMessage(error)}`, + this.statusSource, + ); + } + throw error; + } + } + const rstestProcess = spawn(nodeExecutable, [...nodeExecArgs, workerPath], { + cwd: this.cwd, + stdio: ['pipe', 'pipe', 'pipe', 'ipc'], + // Default JSON serialization: `advanced` uses the V8 serializer, whose + // format follows the V8 version, and Electron's V8 can be newer than + // the user's Node can read. + env: workerEnv, + }); rstestProcess.stdout?.on('data', (d) => { const content = d.toString(); @@ -941,45 +963,6 @@ export class RstestApi { worker.$close(); }); - // Attach the debugger only after the error/exit handlers are wired, so a - // spawn failure (e.g. a misconfigured `nodeExecutable`) during this await is - // handled instead of throwing uncaught in the extension host. - if (startDebugging) { - const debugOutFiles = getConfigValue('debugOutFiles', this.workspace); - try { - const startedDebugging = await vscode.debug.startDebugging( - this.workspace, - { - type: 'node', - name: 'Rstest Debug', - request: 'attach', - skipFiles: getConfigValue('debugExclude', this.workspace), - ...(debugOutFiles.length ? { outFiles: debugOutFiles } : {}), - ...(debuggerPort - ? { - port: debuggerPort, - address: debuggerAddress ?? DEFAULT_DEBUG_HOST, - } - : { processId: rstestProcess.pid }), - }, - { testRun }, - ); - if (this.disposed) { - throw new Error( - 'worker spawn aborted: this master was disposed while the debugger was attaching', - ); - } - if (!startedDebugging) { - throw new Error( - `Failed to attach debugger to test worker process (PID: ${rstestProcess.pid})`, - ); - } - } catch (error) { - if (!worker.$closed) worker.$close(); - throw error; - } - } - return { worker, ...paths }; } diff --git a/packages/vscode/src/stacks/test/shared/socketRpc.ts b/packages/vscode/src/stacks/test/shared/socketRpc.ts new file mode 100644 index 0000000..9bf1530 --- /dev/null +++ b/packages/vscode/src/stacks/test/shared/socketRpc.ts @@ -0,0 +1,37 @@ +import type { Socket } from 'node:net'; +import { createInterface } from 'node:readline'; + +export const DEBUG_PIPE_ENV = 'RSTACK_RSTEST_DEBUG_PIPE'; +export const DEBUG_PIPE_TOKEN_ENV = 'RSTACK_RSTEST_DEBUG_PIPE_TOKEN'; + +// Debug launches have no Node IPC channel. Keep the same JSON wire values as +// the normal spawn transport, framing each message with a newline. +export const socketRpc = ( + socket: Socket, + authenticate?: (token: string) => boolean, +) => ({ + post: (data: unknown) => { + if (!socket.destroyed) socket.write(`${JSON.stringify(data)}\n`); + }, + on: (fn: (data: unknown) => void) => { + const lines = createInterface({ input: socket }); + // readline re-emits input errors; the socket owner handles the failure. + lines.on('error', () => lines.close()); + lines.on('line', (line) => { + if (socket.destroyed) return; + if (authenticate) { + if (!authenticate(line)) socket.destroy(); + else authenticate = undefined; + return; + } + let data: unknown; + try { + data = JSON.parse(line); + } catch (error) { + socket.destroy(error as Error); + return; + } + fn(data); + }); + }, +}); diff --git a/packages/vscode/src/stacks/test/worker/index.ts b/packages/vscode/src/stacks/test/worker/index.ts index 976f734..79b4dc4 100644 --- a/packages/vscode/src/stacks/test/worker/index.ts +++ b/packages/vscode/src/stacks/test/worker/index.ts @@ -1,4 +1,5 @@ import { pathToFileURL } from 'node:url'; +import { connect } from 'node:net'; import { createBirpc } from 'birpc'; import { missingDependencyCauseOf } from '../../../shared/missingDependency'; import { SUPPORT_MATRIX } from '../../../shared/versionCheck'; @@ -6,6 +7,11 @@ import type { TestRunReporter } from '../testRunReporter'; import type { NormalizedConfigResult, WorkerInitOptions } from '../types'; import { retractForceColorIfDisabled } from '../shared/colorEnv'; import { rpcErrorCodec } from '../shared/rpc'; +import { + DEBUG_PIPE_ENV, + DEBUG_PIPE_TOKEN_ENV, + socketRpc, +} from '../shared/socketRpc'; import { logger } from './logger'; import { CoverageReporter, ProgressLogger, ProgressReporter } from './reporter'; @@ -206,9 +212,21 @@ export class Worker { } const worker = new Worker(); +// Consume before loading project code so pool children cannot inherit the +// master's RPC endpoint. js-debug's own environment is left untouched. +const debugEndpoint = process.env[DEBUG_PIPE_ENV]; +const debugToken = process.env[DEBUG_PIPE_TOKEN_ENV]; +delete process.env[DEBUG_PIPE_ENV]; +delete process.env[DEBUG_PIPE_TOKEN_ENV]; +const debugSocket = debugEndpoint ? connect(debugEndpoint) : undefined; +debugSocket?.write(`${debugToken}\n`); export const masterApi = createBirpc(worker, { - post: (data) => process.send?.(data), - on: (fn) => process.on('message', fn), + ...(debugSocket + ? socketRpc(debugSocket) + : { + post: (data: unknown) => process.send?.(data), + on: (fn: (data: unknown) => void) => process.on('message', fn), + }), bind: 'functions', ...rpcErrorCodec, }); @@ -227,6 +245,8 @@ if (process.argv[1] === __filename) { .finally(() => process.exit()); }; process.once('disconnect', shutdown); + debugSocket?.once('close', shutdown); + debugSocket?.once('error', shutdown); process.once('SIGINT', shutdown); process.once('SIGTERM', shutdown); } diff --git a/packages/vscode/tests/stacks/test/master.test.ts b/packages/vscode/tests/stacks/test/master.test.ts index 5fb210b..4dd0a36 100644 --- a/packages/vscode/tests/stacks/test/master.test.ts +++ b/packages/vscode/tests/stacks/test/master.test.ts @@ -1,9 +1,11 @@ -import { EventEmitter } from 'node:events'; +import { EventEmitter, once } from 'node:events'; import fs from 'node:fs'; import { createRequire } from 'node:module'; import { spawn } from 'node:child_process'; import os from 'node:os'; +import net from 'node:net'; import path from 'node:path'; +import type vscode from 'vscode'; import { afterEach, beforeEach, describe, expect, it, rs } from '@rstest/core'; import { logger } from '../../../src/stacks/test/logger'; import { @@ -18,6 +20,10 @@ import { resetUserNodeCaches, } from '../../../src/shared/nodeResolution'; import { status } from '../../../src/stacks/test/status'; +import { + DEBUG_PIPE_ENV, + DEBUG_PIPE_TOKEN_ENV, +} from '../../../src/stacks/test/shared/socketRpc'; import type { TestRunReporter } from '../../../src/stacks/test/testRunReporter'; import type { WorkerInitOptions } from '../../../src/stacks/test/types'; import { Worker } from '../../../src/stacks/test/worker'; @@ -80,7 +86,11 @@ const loggedErrors: string[] = []; const loggedWarnings: string[] = []; const createdTerminals: string[] = []; const settings: Record = {}; -let startDebugging = async (): Promise => true; +let startDebugging = async ( + _config: vscode.DebugConfiguration, +): Promise => true; +const debugEvents = new EventEmitter(); +const stopDebugging = rs.fn(async (_session: vscode.DebugSession) => {}); const channel = { debug: () => {}, @@ -100,7 +110,31 @@ logger.bind(channel as never); rs.mock('vscode', () => { const vscode = { TestRunProfileKind: { Run: 1, Debug: 2, Coverage: 3 }, - debug: { startDebugging: () => startDebugging() }, + debug: { + startDebugging: ( + _workspace: unknown, + config: vscode.DebugConfiguration, + ) => startDebugging(config), + stopDebugging: (session: vscode.DebugSession) => stopDebugging(session), + onDidStartDebugSession: (fn: (session: vscode.DebugSession) => void) => { + debugEvents.on('start', fn); + return { + dispose: () => { + debugEvents.off('start', fn); + }, + }; + }, + onDidTerminateDebugSession: ( + fn: (session: vscode.DebugSession) => void, + ) => { + debugEvents.on('end', fn); + return { + dispose: () => { + debugEvents.off('end', fn); + }, + }; + }, + }, env: { shell: '/bin/sh' }, FileCoverage: class {}, Position: class {}, @@ -873,6 +907,7 @@ describe('Rstest public API', () => { shownMessages.length = 0; loggedWarnings.length = 0; startDebugging = async () => true; + stopDebugging.mockClear(); resetUserNodeCaches(); settings['rstack.nodeExecutable'] = process.execPath; await configuredNodeBelowFloor(process.execPath, { @@ -1250,39 +1285,279 @@ describe('Rstest public API', () => { ); }); - it('closes a spawned worker when debugger attachment rejects', async () => { - startDebugging = async () => { - throw new Error('Debugger attachment failed.'); + it('settles disposal while debug launch is still pending', async () => { + const attachment = Promise.withResolvers(); + const started = Promise.withResolvers(); + startDebugging = () => { + started.resolve(); + return attachment.promise; }; const api = createApi(root); - await expect(api.createChildProcess(undefined, true)).rejects.toThrow( - 'Debugger attachment failed.', + const starting = api.createChildProcess(undefined, true); + const rejected = expect(starting).rejects.toThrow( + 'Rstest debug worker stopped', ); - expect(spawnedProcesses[0].killSignals).toEqual(['SIGTERM']); - expect((api as any).workers.size).toBe(0); + await started.promise; + await api.dispose(); + await rejected; + attachment.resolve(true); expect(runningWorkers.size).toBe(0); }); - it('closes a worker when disposal starts during debugger attachment', async () => { - const attachment = Promise.withResolvers(); + it('preserves user settings in the debug launch configuration', async () => { + settings.nodeExecArgs = ['--enable-source-maps']; + settings.nodeEnv = { + NODE_ENV: 'custom', + SHARED: 'node', + RSTEST: 'false', + }; + settings.debugNodeEnv = { SHARED: 'debug', DEBUG_ONLY: 'yes' }; + settings.debugExclude = ['**/vendor/**']; + settings.debugOutFiles = ['**/compiled/**/*.js']; + let configuration!: vscode.DebugConfiguration; + let socket!: net.Socket; + const api = createApi(root); + startDebugging = async (config) => { + configuration = config; + debugEvents.emit('start', { configuration }); + socket = net.connect(config.env[DEBUG_PIPE_ENV]); + socket.on('error', (error) => { + expect(error).toMatchObject({ code: 'ECONNRESET' }); + }); + socket.write(`${config.env[DEBUG_PIPE_TOKEN_ENV]}\n`); + return true; + }; + try { + await api.createChildProcess(undefined, true); + expect(configuration).toMatchObject({ + runtimeArgs: ['--enable-source-maps'], + env: { + NODE_ENV: 'custom', + SHARED: 'debug', + DEBUG_ONLY: 'yes', + RSTEST: 'true', + }, + skipFiles: ['**/vendor/**'], + outFiles: ['**/compiled/**/*.js'], + }); + } finally { + socket?.destroy(); + await api.dispose(); + } + }); + + it.each(['wrong token', 'no token'])( + 'accepts the worker after a connection with %s', + async (kind) => { + const sockets: net.Socket[] = []; + const received: Buffer[] = []; + const api = createApi(root); + startDebugging = async (configuration) => { + debugEvents.emit('start', { configuration }); + const endpoint = configuration.env[DEBUG_PIPE_ENV]; + const secret = configuration.env[DEBUG_PIPE_TOKEN_ENV]; + expect(secret).toBeTruthy(); + expect(secret).not.toBe(configuration.rstestDebugId); + const bad = net.connect(endpoint); + bad.on('error', (error) => { + expect(error).toMatchObject({ code: 'ECONNRESET' }); + }); + sockets.push(bad); + bad.on('data', (data) => received.push(data)); + // events.once rejects on error, but the master may reset rejected peers. + const closed = new Promise((resolve) => + bad.once('close', () => resolve()), + ); + if (kind === 'wrong token') bad.write(`wrong\n${secret}\n`); + else bad.end(); + await closed; + const idle = net.connect(endpoint); + idle.on('error', (error) => { + expect(error).toMatchObject({ code: 'ECONNRESET' }); + }); + sockets.push(idle); + idle.on('data', (data) => received.push(data)); + await once(idle, 'connect'); + const good = net.connect(endpoint); + good.on('error', (error) => { + expect(error).toMatchObject({ code: 'ECONNRESET' }); + }); + sockets.push(good); + good.write(`${secret}\n`); + return true; + }; + try { + const { worker } = await api.createChildProcess(undefined, true); + const message = once(sockets[2], 'data'); + const response = expect(worker.closeWatcher()).rejects.toThrow(); + const [data] = await message; + expect(JSON.parse(data.toString())).toMatchObject({ + m: 'closeWatcher', + }); + const idleClosed = new Promise((resolve) => + sockets[1].once('close', () => resolve()), + ); + await api.dispose(); + await idleClosed; + await response; + expect(received).toEqual([]); + } finally { + for (const socket of sockets) socket.destroy(); + await api.dispose(); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'uses a private /tmp socket when TMPDIR is too long in bytes', + async () => { + rs.spyOn(os, 'tmpdir').mockReturnValue(`/tmp/${'é'.repeat(50)}`); + const api = createApi(root); + let socket: net.Socket | undefined; + startDebugging = async (configuration) => { + const endpoint = configuration.env[DEBUG_PIPE_ENV]; + expect(path.dirname(path.dirname(endpoint))).toBe('/tmp'); + expect(fs.statSync(path.dirname(endpoint)).mode & 0o777).toBe(0o700); + socket = net.connect(endpoint); + socket.on('error', (error) => { + expect(error).toMatchObject({ code: 'ECONNRESET' }); + }); + socket.write(`${configuration.env[DEBUG_PIPE_TOKEN_ENV]}\n`); + return true; + }; + try { + await api.createChildProcess(undefined, true); + } finally { + socket?.destroy(); + await api.dispose(); + } + }, + ); + + it('cancels before the debug worker connects', async () => { + const { token, cancel } = createRunContext(); const started = Promise.withResolvers(); - startDebugging = () => { + startDebugging = async (config) => { + debugEvents.emit('start', { configuration: config }); started.resolve(); - return attachment.promise; + return true; }; const api = createApi(root); - const starting = api.createChildProcess(undefined, true); + const starting = api.createChildProcess( + undefined, + true, + undefined, + token, + ); + const rejected = expect(starting).rejects.toThrow( + 'Rstest debug run cancelled', + ); await started.promise; - // dispose() sets this flag synchronously before closing its current worker - // snapshot; isolate the post-attach guard from the close RPC exercised by - // the disposal tests above. - (api as any).disposed = true; - attachment.resolve(true); - await expect(starting).rejects.toThrow( - 'worker spawn aborted: this master was disposed while the debugger was attaching', + cancel(); + await rejected; + expect(stopDebugging).toHaveBeenCalledTimes(1); + expect(runningWorkers.size).toBe(0); + }); + + it('reports a rejected launch without waiting for a connection', async () => { + const { reporter, reported } = createStatusRecorder(); + status.bind(reporter); + startDebugging = async () => false; + await expect( + createApi(root).createChildProcess(undefined, true), + ).rejects.toThrow('Failed to launch Rstest debug worker'); + expect(reported.filter((state) => state.kind === 'crashed')).toEqual([ + { + kind: 'crashed', + detail: 'worker launch failed: Failed to launch Rstest debug worker', + }, + ]); + expect(runningWorkers.size).toBe(0); + expect(debugEvents.listenerCount('start')).toBe(0); + expect(debugEvents.listenerCount('end')).toBe(0); + }); + + it.each(['unexpected', 'cancelled', 'disposed', 'closed'])( + 'reports a crash only for an unexpected debug session end (%s)', + async (ending) => { + const { reporter, reported } = createStatusRecorder(); + status.bind(reporter); + const { token, cancel } = createRunContext(); + const api = createApi(root); + let socket: net.Socket | undefined; + let configuration!: vscode.DebugConfiguration; + startDebugging = async (config) => { + configuration = config; + debugEvents.emit('start', { configuration }); + socket = net.connect(config.env[DEBUG_PIPE_ENV]); + socket.on('error', (error) => { + expect(error).toMatchObject({ code: 'ECONNRESET' }); + }); + socket.write(`${config.env[DEBUG_PIPE_TOKEN_ENV]}\n`); + return true; + }; + try { + const { worker } = await api.createChildProcess( + undefined, + true, + undefined, + token, + ); + expect(status.hasFailed(`test://${root}`)).toBe(false); + if (ending === 'cancelled') cancel(); + if (ending === 'disposed') await api.dispose(); + if (ending === 'closed') worker.$close(); + debugEvents.emit('end', { configuration }); + expect(status.hasFailed(`test://${root}`)).toBe( + ending === 'unexpected', + ); + expect(reported.filter((state) => state.kind === 'crashed')).toEqual( + ending === 'unexpected' + ? [ + { + kind: 'crashed', + detail: + 'worker exited unexpectedly: Rstest debug session ended', + }, + ] + : [], + ); + } finally { + socket?.destroy(); + await api.dispose(); + } + }, + ); + + it('stops the session on worker exit and makes repeated RPC close harmless', async () => { + let socket!: net.Socket; + let endpoint!: string; + startDebugging = async (configuration) => { + debugEvents.emit('start', { configuration }); + endpoint = configuration.env[DEBUG_PIPE_ENV]; + socket = net.connect(endpoint); + socket.on('error', (error) => { + expect(error).toMatchObject({ code: 'ECONNRESET' }); + }); + socket.write(`${configuration.env[DEBUG_PIPE_TOKEN_ENV]}\n`); + return true; + }; + const { worker } = await createApi(root).createChildProcess( + undefined, + true, ); - expect(spawnedProcesses[0].killSignals).toEqual(['SIGTERM']); + const pending = expect(worker.closeWatcher()).rejects.toThrow(); + socket.destroy(); + await pending; + worker.$close(); + worker.$close(); + expect(stopDebugging).toHaveBeenCalled(); expect(runningWorkers.size).toBe(0); + if (process.platform !== 'win32') { + await expect + .poll(() => fs.existsSync(path.dirname(endpoint))) + .toBe(false); + } }); }); }); diff --git a/packages/vscode/tests/stacks/test/rpc.test.ts b/packages/vscode/tests/stacks/test/rpc.test.ts index 530631a..4b23f66 100644 --- a/packages/vscode/tests/stacks/test/rpc.test.ts +++ b/packages/vscode/tests/stacks/test/rpc.test.ts @@ -1,5 +1,7 @@ +import { Socket } from 'node:net'; import { describe, expect, it } from '@rstest/core'; import { rpcErrorCodec } from '../../../src/stacks/test/shared/rpc'; +import { socketRpc } from '../../../src/stacks/test/shared/socketRpc'; const jsonRoundTrip = (message: object) => rpcErrorCodec.deserialize( @@ -22,3 +24,19 @@ describe('rpcErrorCodec', () => { expect(jsonRoundTrip(message)).toEqual(message); }); }); + +it('leaves socket error handling to the owner without an unhandled readline error', () => { + const socket = new Socket(); + const errors: Error[] = []; + socketRpc(socket).on(() => {}); + socket.on('error', (error) => errors.push(error)); + const error = Object.assign(new Error('read ECONNRESET'), { + code: 'ECONNRESET', + }); + try { + expect(() => socket.emit('error', error)).not.toThrow(); + expect(errors).toEqual([error]); + } finally { + socket.destroy(); + } +});