fix: recover relay startup from EADDRINUSE

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

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",

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<number[]> {
 
 async function getPidsForPortUnix(port: number): Promise<number[]> {
   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<number[]> {
   }
 }
 
-function sleep(ms: number): Promise<void> {
-  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<number[]> {
+  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<void> {
@@ -121,47 +160,89 @@ async function terminatePidWindows(pid: number): Promise<void> {
   } catch {}
 }
 
-async function terminatePidUnix(pid: number): Promise<void> {
-  try {
-    process.kill(pid, 'SIGTERM')
-  } catch {
-    return
-  }
+async function unixKillPortUsingPipes({ port, signal }: { port: number; signal: KillPortSignal }): Promise<void> {
+  const killCommand = signal === 'SIGTERM' ? '-15' : '-9'
 
-  await sleep(200)
-  if (!isProcessAlive(pid)) {
-    return
-  }
+  await new Promise<void>((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<void> {
+export async function killPortProcess({
+  port,
+  signal = 'SIGKILL',
+}: {
+  port: number
+  signal?: KillPortSignal
+}): Promise<void> {
   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 })
 }

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<void> {
+async function killRelayServer(options: { port: number; waitForFreeMs?: number }): Promise<void> {
+  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}`)
 }

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({

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<number> {
+  return await new Promise<number>((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<number[]> {
+  const startTime = Date.now()
+  while (Date.now() - startTime < timeoutMs) {
+    const pids = await getListeningPidsForPort({ port }).catch(() => [])
+    if (predicate(pids)) {
+      return pids
+    }
+    await new Promise<void>((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')
+      }
+    }
+  })
+})