From 744a9b913a1308e450f7c50492915c8574b45429 Mon Sep 17 00:00:00 2001 From: "Tommy D. Rossi" Date: Fri, 13 Feb 2026 15:42:43 +0100 Subject: [PATCH] fix: recover relay startup from EADDRINUSE --- playwriter/CHANGELOG.md | 11 ++ playwriter/package.json | 2 +- playwriter/src/kill-port.ts | 171 +++++++++++++++++------- playwriter/src/relay-client.ts | 37 +++-- playwriter/src/relay-navigation.test.ts | 8 +- playwriter/test/kill-port.test.ts | 112 ++++++++++++++++ 6 files changed, 284 insertions(+), 57 deletions(-) create mode 100644 playwriter/test/kill-port.test.ts diff --git a/playwriter/CHANGELOG.md b/playwriter/CHANGELOG.md index ccb71fd..870499f 100644 --- a/playwriter/CHANGELOG.md +++ b/playwriter/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 0.0.60 + +### Bug Fixes + +- **Fix relay startup EADDRINUSE timeouts**: If the relay port is already bound but `/version` is not responding, Playwriter now detects the listening PID(s), stops the existing process, and only then starts the relay (the 5s startup timeout now measures post-spawn readiness, not port cleanup time). +- **Harden port-kill implementation**: Replaced Playwriter's port killer with an implementation that mirrors `kill-port-process` (lsof/grep/awk/xargs on unix; taskkill on Windows) and includes the `xargs.stdout` pipe fix from upstream PR #199. + +### Tests + +- **Add kill-port subprocess test**: New test starts a real HTTP server subprocess on an ephemeral port, measures kill latency, and asserts the port is released. + ## 0.0.59 ### Bug Fixes diff --git a/playwriter/package.json b/playwriter/package.json index adf4dd3..4e53200 100644 --- a/playwriter/package.json +++ b/playwriter/package.json @@ -1,7 +1,7 @@ { "name": "playwriter", "description": "", - "version": "0.0.59", + "version": "0.0.60", "type": "module", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/playwriter/src/kill-port.ts b/playwriter/src/kill-port.ts index e4488e7..527474a 100644 --- a/playwriter/src/kill-port.ts +++ b/playwriter/src/kill-port.ts @@ -1,14 +1,22 @@ /** * Cross-platform process termination for a TCP port. - * Replaces external kill-port-process dependency for bunx/runtime stability. + * + * This mirrors the approach used by the `kill-port-process` npm package: + * https://github.com/hilleer/kill-port-process + * + * Important fix (ported from https://github.com/hilleer/kill-port-process/pull/199): + * do NOT pipe `xargs.stdout` into `process.stdin` (stdin is not writable and can + * throw `dest.end is not a function`). We simply don't pipe `xargs` stdout. */ -import { execFile } from 'node:child_process' +import { execFile, spawn } from 'node:child_process' import os from 'node:os' import { promisify } from 'node:util' const execFileAsync = promisify(execFile) +export type KillPortSignal = 'SIGTERM' | 'SIGKILL' + function isValidPort(port: number): boolean { return Number.isInteger(port) && port > 0 && port <= 65535 } @@ -32,6 +40,28 @@ function parsePids(output: string): number[] { return [...new Set(pids)] } +function parseUnixLsofPids(stdout: string): number[] { + const pids = stdout + .split(/\r?\n/g) + .map((line) => { + return line.trim() + }) + .filter((line) => { + // lsof output starts with a header row; lines we care about contain LISTEN. + return Boolean(line) && !line.startsWith('COMMAND') && line.includes('LISTEN') + }) + .map((line) => { + const columns = line.split(/\s+/g) + // `awk '{print $2}'` in kill-port-process extracts PID from 2nd column. + return Number(columns[1] || '') + }) + .filter((pid) => { + return Number.isInteger(pid) && pid > 0 + }) + + return [...new Set(pids)] +} + function parseWindowsNetstatPids(output: string, port: number): number[] { const rows = output .split(/\r?\n/g) @@ -85,13 +115,14 @@ async function getPidsForPortWindows(port: number): Promise { async function getPidsForPortUnix(port: number): Promise { try { - const { stdout } = await execFileAsync('lsof', ['-nP', '-iTCP:' + String(port), '-sTCP:LISTEN', '-t']) - const pids = parsePids(stdout) + const { stdout } = await execFileAsync('lsof', ['-i', `tcp:${port}`]) + const pids = parseUnixLsofPids(stdout) if (pids.length > 0) { return pids } } catch {} + // Fallback for Linux environments that may not have `lsof`. try { const { stdout } = await execFileAsync('fuser', [`${port}/tcp`]) return parsePids(stdout) @@ -100,19 +131,27 @@ async function getPidsForPortUnix(port: number): Promise { } } -function sleep(ms: number): Promise { - return new Promise((resolve) => { - setTimeout(resolve, ms) - }) +/** + * Return PIDs (if any) that are currently LISTENing on the given TCP port. + * + * This is intentionally separate from killPortProcess(): callers sometimes need + * to detect/diagnose EADDRINUSE before attempting to start a server. + */ +export async function getListeningPidsForPort({ port }: { port: number }): Promise { + if (!isValidPort(port)) { + throw new Error(`Invalid port: ${port}`) + } + + return os.platform() === 'win32' + ? await getPidsForPortWindows(port) + : await getPidsForPortUnix(port) } -function isProcessAlive(pid: number): boolean { - try { - process.kill(pid, 0) - return true - } catch { - return false +function toError(value: unknown): Error { + if (value instanceof Error) { + return value } + return new Error(String(value)) } async function terminatePidWindows(pid: number): Promise { @@ -121,47 +160,89 @@ async function terminatePidWindows(pid: number): Promise { } catch {} } -async function terminatePidUnix(pid: number): Promise { - try { - process.kill(pid, 'SIGTERM') - } catch { - return - } +async function unixKillPortUsingPipes({ port, signal }: { port: number; signal: KillPortSignal }): Promise { + const killCommand = signal === 'SIGTERM' ? '-15' : '-9' - await sleep(200) - if (!isProcessAlive(pid)) { - return - } + await new Promise((resolve, reject) => { + const lsof = spawn('lsof', ['-i', `tcp:${port}`], { stdio: ['ignore', 'pipe', 'pipe'] }) + const grep = spawn('grep', ['LISTEN'], { stdio: ['pipe', 'pipe', 'pipe'] }) + const awk = spawn('awk', ['{print $2}'], { stdio: ['pipe', 'pipe', 'pipe'] }) + const xargs = spawn('xargs', ['kill', killCommand], { stdio: ['pipe', 'ignore', 'pipe'] }) - try { - process.kill(pid, 'SIGKILL') - } catch {} + if (!lsof.stdout || !grep.stdin || !grep.stdout || !awk.stdin || !awk.stdout || !xargs.stdin) { + reject(new Error('Failed to create stdio pipes for kill-port process chain')) + return + } + + lsof.stdout.pipe(grep.stdin) + grep.stdout.pipe(awk.stdin) + awk.stdout.pipe(xargs.stdin) + + // IMPORTANT: do not pipe xargs stdout anywhere. + + const stderrChunks: string[] = [] + const collectStderr = (name: string, stream: NodeJS.ReadableStream | null) => { + stream?.on('data', (data: unknown) => { + const text = data instanceof Buffer ? data.toString() : String(data) + stderrChunks.push(`${name} - ${text}`) + }) + } + + collectStderr('lsof', lsof.stderr) + collectStderr('grep', grep.stderr) + collectStderr('awk', awk.stderr) + collectStderr('xargs', xargs.stderr) + + const onError = (name: string) => { + return (error: unknown) => { + reject(new Error(`kill-port process error in ${name}`, { cause: toError(error) })) + } + } + + lsof.on('error', onError('lsof')) + grep.on('error', onError('grep')) + awk.on('error', onError('awk')) + xargs.on('error', onError('xargs')) + + xargs.on('close', (code) => { + if (code === 0) { + resolve() + return + } + + const extra = stderrChunks.length > 0 ? `\n${stderrChunks.join('\n')}` : '' + reject(new Error(`Failed to kill process on port ${port} (exit code ${code}).${extra}`)) + }) + }) } /** * Kill any listening process bound to the provided TCP port. */ -export async function killPortProcess({ port }: { port: number }): Promise { +export async function killPortProcess({ + port, + signal = 'SIGKILL', +}: { + port: number + signal?: KillPortSignal +}): Promise { if (!isValidPort(port)) { throw new Error(`Invalid port: ${port}`) } - const pids = os.platform() === 'win32' - ? await getPidsForPortWindows(port) - : await getPidsForPortUnix(port) - - const currentPid = process.pid - const targetPids = pids.filter((pid) => { - return pid !== currentPid - }) - - await Promise.all( - targetPids.map(async (pid) => { - if (os.platform() === 'win32') { + if (os.platform() === 'win32') { + const pids = await getListeningPidsForPort({ port }) + const currentPid = process.pid + const targetPids = pids.filter((pid) => { + return pid !== currentPid + }) + await Promise.all( + targetPids.map(async (pid) => { await terminatePidWindows(pid) - return - } - await terminatePidUnix(pid) - }), - ) + }), + ) + return + } + + await unixKillPortUsingPipes({ port, signal }) } diff --git a/playwriter/src/relay-client.ts b/playwriter/src/relay-client.ts index 6d25edf..5facf43 100644 --- a/playwriter/src/relay-client.ts +++ b/playwriter/src/relay-client.ts @@ -7,7 +7,7 @@ import { spawn } from 'node:child_process' import path from 'node:path' import { fileURLToPath } from 'node:url' import pc from 'picocolors' -import { killPortProcess } from './kill-port.js' +import { getListeningPidsForPort, killPortProcess } from './kill-port.js' import { VERSION, sleep, LOG_FILE_PATH } from './utils.js' const __filename = fileURLToPath(import.meta.url) @@ -71,11 +71,23 @@ export async function waitForExtension(options: { return false } -async function killRelayServer(port: number): Promise { +async function killRelayServer(options: { port: number; waitForFreeMs?: number }): Promise { + const { port, waitForFreeMs = 3000 } = options + try { await killPortProcess({ port }) - await sleep(500) - } catch {} + } catch { + return + } + + const startTime = Date.now() + while (Date.now() - startTime < waitForFreeMs) { + const pids = await getListeningPidsForPort({ port }).catch(() => []) + if (pids.length === 0) { + return + } + await sleep(100) + } } /** @@ -128,12 +140,18 @@ export async function ensureRelayServer(options: EnsureRelayServerOptions = {}): if (serverVersion !== null) { if (restartOnVersionMismatch) { logger?.log(pc.yellow(`CDP relay server version mismatch (server: ${serverVersion}, client: ${VERSION}), restarting...`)) - await killRelayServer(RELAY_PORT) + await killRelayServer({ port: RELAY_PORT }) } else { // Server is running but different version, just use it return } } else { + const listeningPids = await getListeningPidsForPort({ port: RELAY_PORT }).catch(() => []) + if (listeningPids.length > 0) { + logger?.log(pc.yellow(`Port ${RELAY_PORT} is already in use (pid(s): ${listeningPids.join(', ')}). Attempting to stop the existing process...`)) + await killRelayServer({ port: RELAY_PORT }) + } + logger?.log(pc.dim('CDP relay server not running, starting it...')) } @@ -152,9 +170,11 @@ export async function ensureRelayServer(options: EnsureRelayServerOptions = {}): serverProcess.unref() + const startTimeoutMs = 5000 + const startTime = Date.now() - for (let i = 0; i < 10; i++) { - await sleep(500) + while (Date.now() - startTime < startTimeoutMs) { + await sleep(200) const newVersion = await getRelayServerVersion(RELAY_PORT) if (newVersion) { logger?.log(pc.green('CDP relay server started successfully')) @@ -163,5 +183,6 @@ export async function ensureRelayServer(options: EnsureRelayServerOptions = {}): } } - throw new Error(`Failed to start CDP relay server after 5 seconds. Check logs at: ${LOG_FILE_PATH}`) + const waitedMs = Date.now() - startTime + throw new Error(`Failed to start CDP relay server within ${waitedMs}ms. Check logs at: ${LOG_FILE_PATH}`) } diff --git a/playwriter/src/relay-navigation.test.ts b/playwriter/src/relay-navigation.test.ts index d6124fd..6f0be04 100644 --- a/playwriter/src/relay-navigation.test.ts +++ b/playwriter/src/relay-navigation.test.ts @@ -261,11 +261,13 @@ describe('Relay Navigation Tests', () => { errorMessage: 'Timed out waiting for plugin frame URL in empty-src iframe test', }) - const buttonCount = await withTimeout({ - promise: pluginFrame.locator('button').count(), + await withTimeout({ + promise: pluginFrame.locator('button').first().waitFor({ state: 'attached' }), timeoutMs: 5000, - errorMessage: 'Timed out counting button locator in empty-src iframe test', + errorMessage: 'Timed out waiting for button locator in empty-src iframe test', }) + + const buttonCount = await pluginFrame.locator('button').count() expect(buttonCount).toBe(1) } finally { await withTimeout({ diff --git a/playwriter/test/kill-port.test.ts b/playwriter/test/kill-port.test.ts new file mode 100644 index 0000000..50dc5cb --- /dev/null +++ b/playwriter/test/kill-port.test.ts @@ -0,0 +1,112 @@ +// Test that killPortProcess can terminate a real subprocess bound to a TCP port. + +import { describe, expect, it } from 'vitest' +import { spawn } from 'node:child_process' +import net from 'node:net' +import { getListeningPidsForPort, killPortProcess } from '../src/kill-port.js' + +async function getFreeTcpPort(): Promise { + return await new Promise((resolve, reject) => { + const server = net.createServer() + server.on('error', (error) => { + reject(error) + }) + + server.listen(0, '127.0.0.1', () => { + const address = server.address() + if (!address || typeof address === 'string') { + server.close(() => { + reject(new Error('Failed to get ephemeral port')) + }) + return + } + + const { port } = address + server.close((error) => { + if (error) { + reject(error) + return + } + resolve(port) + }) + }) + }) +} + +async function waitForListeningPidCount({ + port, + predicate, + timeoutMs, +}: { + port: number + predicate: (pids: number[]) => boolean + timeoutMs: number +}): Promise { + const startTime = Date.now() + while (Date.now() - startTime < timeoutMs) { + const pids = await getListeningPidsForPort({ port }).catch(() => []) + if (predicate(pids)) { + return pids + } + await new Promise((resolve) => { + setTimeout(resolve, 50) + }) + } + + const pids = await getListeningPidsForPort({ port }).catch(() => []) + throw new Error(`Timed out waiting for port ${port} pid condition (pids: ${pids.join(', ') || 'none'})`) +} + +describe('killPortProcess', () => { + it('kills a real http server subprocess and reports timing', async () => { + const port = await getFreeTcpPort() + + const child = spawn( + process.execPath, + [ + '-e', + [ + "const http = require('http');", + 'const port = Number(process.env.TEST_PORT);', + "const host = '127.0.0.1';", + "const server = http.createServer((req, res) => { res.statusCode = 200; res.end('ok'); });", + 'server.listen(port, host, () => { console.log(`listening:${port}`); });', + // Keep the process alive. + 'setInterval(() => {}, 1000);', + ].join(' '), + ], + { + env: { ...process.env, TEST_PORT: String(port) }, + stdio: 'ignore', + }, + ) + + try { + await waitForListeningPidCount({ + port, + predicate: (pids) => pids.length > 0, + timeoutMs: 5000, + }) + + const start = Date.now() + await killPortProcess({ port }) + + await waitForListeningPidCount({ + port, + predicate: (pids) => pids.length === 0, + timeoutMs: 5000, + }) + + const elapsedMs = Date.now() - start + console.log(`[kill-port] port ${port} killed in ${elapsedMs}ms`) // for perf visibility in CI logs + + const maxMs = process.platform === 'win32' ? 15000 : 5000 + expect(elapsedMs).toBeLessThan(maxMs) + } finally { + // Best-effort cleanup if the kill failed for any reason. + if (!child.killed) { + child.kill('SIGKILL') + } + } + }) +})