fix(doctor): add timeouts to subprocess spawns to prevent exit code 137
Doctor checks spawned subprocesses (gh, opencode, sg) without timeouts, causing the process to hang indefinitely if any binary was stuck. The OS would then SIGKILL the process (exit code 137). - Add spawnWithTimeout utility with 10s per-spawn timeout - Add 30s overall doctor command timeout with graceful error message - Update all doctor check subprocess calls to use timeouts
This commit is contained in:
@@ -3,7 +3,7 @@ import { createRequire } from "node:module"
|
|||||||
import { dirname, join } from "node:path"
|
import { dirname, join } from "node:path"
|
||||||
|
|
||||||
import type { DependencyInfo } from "../types"
|
import type { DependencyInfo } from "../types"
|
||||||
import { spawnWithWindowsHide } from "../../../shared/spawn-with-windows-hide"
|
import { spawnWithTimeout } from "../spawn-with-timeout"
|
||||||
|
|
||||||
async function checkBinaryExists(binary: string): Promise<{ exists: boolean; path: string | null }> {
|
async function checkBinaryExists(binary: string): Promise<{ exists: boolean; path: string | null }> {
|
||||||
try {
|
try {
|
||||||
@@ -19,16 +19,12 @@ async function checkBinaryExists(binary: string): Promise<{ exists: boolean; pat
|
|||||||
|
|
||||||
async function getBinaryVersion(binary: string): Promise<string | null> {
|
async function getBinaryVersion(binary: string): Promise<string | null> {
|
||||||
try {
|
try {
|
||||||
const proc = spawnWithWindowsHide([binary, "--version"], { stdout: "pipe", stderr: "pipe" })
|
const result = await spawnWithTimeout([binary, "--version"], { stdout: "pipe", stderr: "pipe" })
|
||||||
const output = await new Response(proc.stdout).text()
|
if (result.timedOut || result.exitCode !== 0) return null
|
||||||
await proc.exited
|
return result.stdout.trim().split("\n")[0] ?? null
|
||||||
if (proc.exitCode === 0) {
|
|
||||||
return output.trim().split("\n")[0]
|
|
||||||
}
|
|
||||||
} catch {
|
} catch {
|
||||||
// intentionally empty - version unavailable
|
return null
|
||||||
}
|
}
|
||||||
return null
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function checkAstGrepCli(): Promise<DependencyInfo> {
|
export async function checkAstGrepCli(): Promise<DependencyInfo> {
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { existsSync } from "node:fs"
|
import { existsSync } from "node:fs"
|
||||||
import { homedir } from "node:os"
|
import { homedir } from "node:os"
|
||||||
import { join } from "node:path"
|
import { join } from "node:path"
|
||||||
import { spawnWithWindowsHide } from "../../../shared/spawn-with-windows-hide"
|
import { spawnWithTimeout } from "../spawn-with-timeout"
|
||||||
|
|
||||||
import { OPENCODE_BINARIES } from "../constants"
|
import { OPENCODE_BINARIES } from "../constants"
|
||||||
|
|
||||||
@@ -111,12 +111,9 @@ export async function getOpenCodeVersion(
|
|||||||
): Promise<string | null> {
|
): Promise<string | null> {
|
||||||
try {
|
try {
|
||||||
const command = buildVersionCommand(binaryPath, platform)
|
const command = buildVersionCommand(binaryPath, platform)
|
||||||
const processResult = spawnWithWindowsHide(command, { stdout: "pipe", stderr: "pipe" })
|
const result = await spawnWithTimeout(command, { stdout: "pipe", stderr: "pipe" })
|
||||||
const output = await new Response(processResult.stdout).text()
|
if (result.timedOut || result.exitCode !== 0) return null
|
||||||
await processResult.exited
|
return result.stdout.trim() || null
|
||||||
|
|
||||||
if (processResult.exitCode !== 0) return null
|
|
||||||
return output.trim() || null
|
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { spawnWithWindowsHide } from "../../../shared/spawn-with-windows-hide"
|
import { spawnWithTimeout } from "../spawn-with-timeout"
|
||||||
|
|
||||||
export interface GhCliInfo {
|
export interface GhCliInfo {
|
||||||
installed: boolean
|
installed: boolean
|
||||||
@@ -21,13 +21,11 @@ async function checkBinaryExists(binary: string): Promise<{ exists: boolean; pat
|
|||||||
|
|
||||||
async function getGhVersion(): Promise<string | null> {
|
async function getGhVersion(): Promise<string | null> {
|
||||||
try {
|
try {
|
||||||
const processResult = spawnWithWindowsHide(["gh", "--version"], { stdout: "pipe", stderr: "pipe" })
|
const result = await spawnWithTimeout(["gh", "--version"], { stdout: "pipe", stderr: "pipe" })
|
||||||
const output = await new Response(processResult.stdout).text()
|
if (result.timedOut || result.exitCode !== 0) return null
|
||||||
await processResult.exited
|
|
||||||
if (processResult.exitCode !== 0) return null
|
|
||||||
|
|
||||||
const matchedVersion = output.match(/gh version (\S+)/)
|
const matchedVersion = result.stdout.match(/gh version (\S+)/)
|
||||||
return matchedVersion?.[1] ?? output.trim().split("\n")[0] ?? null
|
return matchedVersion?.[1] ?? result.stdout.trim().split("\n")[0] ?? null
|
||||||
} catch {
|
} catch {
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
@@ -40,18 +38,17 @@ async function getGhAuthStatus(): Promise<{
|
|||||||
error: string | null
|
error: string | null
|
||||||
}> {
|
}> {
|
||||||
try {
|
try {
|
||||||
const processResult = spawnWithWindowsHide(["gh", "auth", "status"], {
|
const result = await spawnWithTimeout(
|
||||||
stdout: "pipe",
|
["gh", "auth", "status"],
|
||||||
stderr: "pipe",
|
{ stdout: "pipe", stderr: "pipe", env: { ...process.env, GH_NO_UPDATE_NOTIFIER: "1" } }
|
||||||
env: { ...process.env, GH_NO_UPDATE_NOTIFIER: "1" },
|
)
|
||||||
})
|
|
||||||
|
|
||||||
const stdout = await new Response(processResult.stdout).text()
|
if (result.timedOut) {
|
||||||
const stderr = await new Response(processResult.stderr).text()
|
return { authenticated: false, username: null, scopes: [], error: "gh auth status timed out" }
|
||||||
await processResult.exited
|
}
|
||||||
|
|
||||||
const output = stderr || stdout
|
const output = result.stdout
|
||||||
if (processResult.exitCode === 0) {
|
if (result.exitCode === 0) {
|
||||||
const usernameMatch = output.match(/Logged in to github\.com account (\S+)/)
|
const usernameMatch = output.match(/Logged in to github\.com account (\S+)/)
|
||||||
const scopesMatch = output.match(/Token scopes?:\s*(.+)/i)
|
const scopesMatch = output.match(/Token scopes?:\s*(.+)/i)
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,8 @@ import { getAllCheckDefinitions, gatherSystemInfo, gatherToolsSummary } from "./
|
|||||||
import { EXIT_CODES } from "./constants"
|
import { EXIT_CODES } from "./constants"
|
||||||
import { formatDoctorOutput, formatJsonOutput } from "./formatter"
|
import { formatDoctorOutput, formatJsonOutput } from "./formatter"
|
||||||
|
|
||||||
|
const DOCTOR_TIMEOUT_MS = 30_000
|
||||||
|
|
||||||
export async function runCheck(check: CheckDefinition): Promise<CheckResult> {
|
export async function runCheck(check: CheckDefinition): Promise<CheckResult> {
|
||||||
const start = performance.now()
|
const start = performance.now()
|
||||||
try {
|
try {
|
||||||
@@ -39,12 +41,35 @@ export async function runDoctor(options: DoctorOptions): Promise<DoctorResult> {
|
|||||||
const start = performance.now()
|
const start = performance.now()
|
||||||
|
|
||||||
const allChecks = getAllCheckDefinitions()
|
const allChecks = getAllCheckDefinitions()
|
||||||
const [results, systemInfo, tools] = await Promise.all([
|
|
||||||
|
const checksPromise = Promise.all([
|
||||||
Promise.all(allChecks.map(runCheck)),
|
Promise.all(allChecks.map(runCheck)),
|
||||||
gatherSystemInfo(),
|
gatherSystemInfo(),
|
||||||
gatherToolsSummary(),
|
gatherToolsSummary(),
|
||||||
])
|
])
|
||||||
|
|
||||||
|
const timeoutPromise = new Promise<never>((_, reject) => {
|
||||||
|
setTimeout(() => reject(new Error("Doctor timed out")), DOCTOR_TIMEOUT_MS)
|
||||||
|
})
|
||||||
|
|
||||||
|
let results: CheckResult[]
|
||||||
|
let systemInfo: Awaited<ReturnType<typeof gatherSystemInfo>>
|
||||||
|
let tools: Awaited<ReturnType<typeof gatherToolsSummary>>
|
||||||
|
|
||||||
|
try {
|
||||||
|
;[results, systemInfo, tools] = await Promise.race([checksPromise, timeoutPromise])
|
||||||
|
} catch {
|
||||||
|
console.error("\nDoctor timed out after 30s. A subprocess may be hanging.")
|
||||||
|
console.error("Try running with --verbose to identify the stuck check.\n")
|
||||||
|
return {
|
||||||
|
results: [],
|
||||||
|
systemInfo: { opencodeVersion: null, opencodePath: null, pluginVersion: null, loadedVersion: null, bunVersion: null, configPath: null, configValid: false, isLocalDev: false },
|
||||||
|
tools: { lspServers: [], astGrepCli: false, astGrepNapi: false, commentChecker: false, ghCli: { installed: false, authenticated: false, username: null }, mcpBuiltin: [], mcpUser: [] },
|
||||||
|
summary: { total: 0, passed: 0, failed: 0, warnings: 0, skipped: 0, duration: Math.round(performance.now() - start) },
|
||||||
|
exitCode: EXIT_CODES.FAILURE,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const duration = performance.now() - start
|
const duration = performance.now() - start
|
||||||
const summary = calculateSummary(results, duration)
|
const summary = calculateSummary(results, duration)
|
||||||
const exitCode = determineExitCode(results)
|
const exitCode = determineExitCode(results)
|
||||||
|
|||||||
@@ -0,0 +1,56 @@
|
|||||||
|
import { describe, it, expect } from "bun:test"
|
||||||
|
import { spawnWithTimeout } from "./spawn-with-timeout"
|
||||||
|
|
||||||
|
describe("spawnWithTimeout", () => {
|
||||||
|
describe("#given a command that completes quickly", () => {
|
||||||
|
it("returns stdout and exit code", async () => {
|
||||||
|
// when
|
||||||
|
const result = await spawnWithTimeout(["echo", "hello"], { stdout: "pipe", stderr: "pipe" })
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result.timedOut).toBe(false)
|
||||||
|
expect(result.exitCode).toBe(0)
|
||||||
|
expect(result.stdout.trim()).toBe("hello")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("#given a command that fails", () => {
|
||||||
|
it("returns non-zero exit code without timing out", async () => {
|
||||||
|
// when
|
||||||
|
const result = await spawnWithTimeout(["false"], { stdout: "pipe", stderr: "pipe" })
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result.timedOut).toBe(false)
|
||||||
|
expect(result.exitCode).not.toBe(0)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("#given a command that exceeds timeout", () => {
|
||||||
|
it("returns timedOut true and kills the process", async () => {
|
||||||
|
// when
|
||||||
|
const result = await spawnWithTimeout(
|
||||||
|
["bash", "-c", "while true; do :; done"],
|
||||||
|
{ stdout: "pipe", stderr: "pipe" },
|
||||||
|
200
|
||||||
|
)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result.timedOut).toBe(true)
|
||||||
|
expect(result.stdout).toBe("")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("#given a nonexistent command", () => {
|
||||||
|
it("handles gracefully without hanging", async () => {
|
||||||
|
// when
|
||||||
|
const result = await spawnWithTimeout(
|
||||||
|
["nonexistent-binary-that-does-not-exist-12345"],
|
||||||
|
{ stdout: "pipe", stderr: "pipe" },
|
||||||
|
2000
|
||||||
|
)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result.timedOut).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -0,0 +1,36 @@
|
|||||||
|
import type { SpawnOptions } from "../../shared/spawn-with-windows-hide"
|
||||||
|
import { spawnWithWindowsHide } from "../../shared/spawn-with-windows-hide"
|
||||||
|
|
||||||
|
const DEFAULT_SPAWN_TIMEOUT_MS = 10_000
|
||||||
|
|
||||||
|
export async function spawnWithTimeout(
|
||||||
|
command: string[],
|
||||||
|
options: SpawnOptions,
|
||||||
|
timeoutMs: number = DEFAULT_SPAWN_TIMEOUT_MS
|
||||||
|
): Promise<{ stdout: string; exitCode: number; timedOut: boolean }> {
|
||||||
|
let proc: ReturnType<typeof spawnWithWindowsHide>
|
||||||
|
try {
|
||||||
|
proc = spawnWithWindowsHide(command, options)
|
||||||
|
} catch {
|
||||||
|
return { stdout: "", exitCode: 1, timedOut: false }
|
||||||
|
}
|
||||||
|
|
||||||
|
const timeoutPromise = new Promise<"timeout">((resolve) => {
|
||||||
|
setTimeout(() => resolve("timeout"), timeoutMs)
|
||||||
|
})
|
||||||
|
|
||||||
|
const processPromise = (async (): Promise<"done"> => {
|
||||||
|
await proc.exited
|
||||||
|
return "done"
|
||||||
|
})()
|
||||||
|
|
||||||
|
const race = await Promise.race([processPromise, timeoutPromise])
|
||||||
|
|
||||||
|
if (race === "timeout") {
|
||||||
|
proc.kill("SIGTERM")
|
||||||
|
return { stdout: "", exitCode: 1, timedOut: true }
|
||||||
|
}
|
||||||
|
|
||||||
|
const stdout = proc.stdout ? await new Response(proc.stdout).text() : ""
|
||||||
|
return { stdout, exitCode: proc.exitCode ?? 1, timedOut: false }
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user