Merge pull request #3915 from code-yeongyu/fix/installer-checking-opencode-hang
fix(installer): timeout opencode --version probe to avoid Desktop binary hang
This commit is contained in:
@@ -9,17 +9,25 @@ type OpenCodeBinaryModule = typeof import("./opencode-binary")
|
|||||||
|
|
||||||
type CreateProcOptions = {
|
type CreateProcOptions = {
|
||||||
exitCode?: number | null
|
exitCode?: number | null
|
||||||
output?: { stdout?: string; stderr?: string }
|
exited?: Promise<number>
|
||||||
|
output?: {
|
||||||
|
stdout?: string
|
||||||
|
stdoutStream?: ReadableStream<Uint8Array>
|
||||||
|
stderr?: string
|
||||||
|
}
|
||||||
|
kill?: (signal?: NodeJS.Signals) => void
|
||||||
}
|
}
|
||||||
|
|
||||||
function createProc(options: CreateProcOptions = {}): ReturnType<typeof spawnHelpers.spawnWithWindowsHide> {
|
function createProc(options: CreateProcOptions = {}): ReturnType<typeof spawnHelpers.spawnWithWindowsHide> {
|
||||||
const exitCode = options.exitCode ?? 0
|
const exitCode = options.exitCode ?? 0
|
||||||
return {
|
return {
|
||||||
exited: Promise.resolve(exitCode),
|
exited: options.exited ?? Promise.resolve(exitCode),
|
||||||
exitCode,
|
exitCode,
|
||||||
stdout: options.output?.stdout !== undefined ? new Blob([options.output.stdout]).stream() : undefined,
|
stdout:
|
||||||
|
options.output?.stdoutStream ??
|
||||||
|
(options.output?.stdout !== undefined ? new Blob([options.output.stdout]).stream() : undefined),
|
||||||
stderr: options.output?.stderr !== undefined ? new Blob([options.output.stderr]).stream() : undefined,
|
stderr: options.output?.stderr !== undefined ? new Blob([options.output.stderr]).stream() : undefined,
|
||||||
kill: () => {},
|
kill: options.kill ?? (() => {}),
|
||||||
} satisfies ReturnType<typeof spawnHelpers.spawnWithWindowsHide>
|
} satisfies ReturnType<typeof spawnHelpers.spawnWithWindowsHide>
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -71,6 +79,82 @@ describe("getOpenCodeVersion (installer)", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("#given timeout path #when getOpenCodeVersion #then sends SIGTERM and SIGKILL and returns null without hanging", () => {
|
||||||
|
it("bounds process lifetime on hung --version", async () => {
|
||||||
|
const killCalls: Array<NodeJS.Signals | undefined> = []
|
||||||
|
spawnSpy.mockReturnValue(
|
||||||
|
createProc({
|
||||||
|
exited: new Promise<number>(() => {}),
|
||||||
|
output: { stdout: "" },
|
||||||
|
kill: (signal?: NodeJS.Signals) => {
|
||||||
|
killCalls.push(signal)
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
|
||||||
|
const immediateSetTimeout = ((handler: TimerHandler) => {
|
||||||
|
if (typeof handler === "function") {
|
||||||
|
handler()
|
||||||
|
}
|
||||||
|
return 1 as unknown as ReturnType<typeof setTimeout>
|
||||||
|
}) as unknown as typeof globalThis.setTimeout
|
||||||
|
const setTimeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation(immediateSetTimeout)
|
||||||
|
|
||||||
|
const result = await getOpenCodeVersion()
|
||||||
|
|
||||||
|
expect(result).toBe(null)
|
||||||
|
expect(killCalls).toEqual(["SIGTERM", "SIGKILL"])
|
||||||
|
|
||||||
|
setTimeoutSpy.mockRestore()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("#given never-closing stdout after kill #when getOpenCodeVersion #then returns within bounded time", () => {
|
||||||
|
it("bounds outputPromise wait and returns null", async () => {
|
||||||
|
const neverClosingStdout = new ReadableStream<Uint8Array>({
|
||||||
|
start() {
|
||||||
|
// Intentionally never closing to simulate a hung stdout stream.
|
||||||
|
},
|
||||||
|
})
|
||||||
|
spawnSpy.mockReturnValue(
|
||||||
|
createProc({
|
||||||
|
exited: new Promise<number>(() => {}),
|
||||||
|
output: { stdoutStream: neverClosingStdout },
|
||||||
|
kill: () => {},
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
|
||||||
|
const immediateSetTimeout = ((handler: TimerHandler) => {
|
||||||
|
if (typeof handler === "function") {
|
||||||
|
handler()
|
||||||
|
}
|
||||||
|
return 1 as unknown as ReturnType<typeof setTimeout>
|
||||||
|
}) as unknown as typeof globalThis.setTimeout
|
||||||
|
const setTimeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation(immediateSetTimeout)
|
||||||
|
|
||||||
|
const result = await getOpenCodeVersion()
|
||||||
|
|
||||||
|
expect(result).toBe(null)
|
||||||
|
|
||||||
|
setTimeoutSpy.mockRestore()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("#given quick successful exit #when getOpenCodeVersion #then clears active timers", () => {
|
||||||
|
it("avoids timer leaks after success", async () => {
|
||||||
|
spawnSpy.mockReturnValue(createProc({ output: { stdout: "1.14.33\n" } }))
|
||||||
|
|
||||||
|
const clearTimeoutSpy = spyOn(globalThis, "clearTimeout")
|
||||||
|
|
||||||
|
const result = await getOpenCodeVersion()
|
||||||
|
|
||||||
|
expect(result).toBe("1.14.33")
|
||||||
|
expect(clearTimeoutSpy).toHaveBeenCalledTimes(2)
|
||||||
|
|
||||||
|
clearTimeoutSpy.mockRestore()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("#given no opencode binary on PATH #when getOpenCodeVersion #then returns null", () => {
|
describe("#given no opencode binary on PATH #when getOpenCodeVersion #then returns null", () => {
|
||||||
it("all candidate spawns throw", async () => {
|
it("all candidate spawns throw", async () => {
|
||||||
spawnSpy.mockImplementation(() => {
|
spawnSpy.mockImplementation(() => {
|
||||||
|
|||||||
@@ -4,6 +4,9 @@ import { spawnWithWindowsHide } from "../../shared/spawn-with-windows-hide"
|
|||||||
import { initConfigContext } from "./config-context"
|
import { initConfigContext } from "./config-context"
|
||||||
|
|
||||||
const OPENCODE_BINARIES = ["opencode", "opencode-desktop"] as const
|
const OPENCODE_BINARIES = ["opencode", "opencode-desktop"] as const
|
||||||
|
const OPENCODE_VERSION_CHECK_TIMEOUT_MS = 1500
|
||||||
|
const OPENCODE_VERSION_KILL_GRACE_MS = 200
|
||||||
|
const OPENCODE_OUTPUT_WAIT_TIMEOUT_MS = 200
|
||||||
|
|
||||||
interface OpenCodeBinaryResult {
|
interface OpenCodeBinaryResult {
|
||||||
binary: OpenCodeBinaryType
|
binary: OpenCodeBinaryType
|
||||||
@@ -17,10 +20,61 @@ async function findOpenCodeBinaryWithVersion(): Promise<OpenCodeBinaryResult | n
|
|||||||
stdout: "pipe",
|
stdout: "pipe",
|
||||||
stderr: "pipe",
|
stderr: "pipe",
|
||||||
})
|
})
|
||||||
const output = await new Response(proc.stdout).text()
|
|
||||||
await proc.exited
|
const outputPromise = new Response(proc.stdout).text()
|
||||||
if (proc.exitCode === 0) {
|
let killTimer: ReturnType<typeof setTimeout> | null = null
|
||||||
|
let killGraceTimer: ReturnType<typeof setTimeout> | null = null
|
||||||
|
const timedExitResult = await Promise.race([
|
||||||
|
proc.exited.then((exitCode) => ({ type: "exit" as const, exitCode })),
|
||||||
|
new Promise<{ type: "timeout" }>((resolve) => {
|
||||||
|
killTimer = setTimeout(() => {
|
||||||
|
proc.kill("SIGTERM")
|
||||||
|
killGraceTimer = setTimeout(() => {
|
||||||
|
proc.kill("SIGKILL")
|
||||||
|
}, OPENCODE_VERSION_KILL_GRACE_MS)
|
||||||
|
resolve({ type: "timeout" })
|
||||||
|
}, OPENCODE_VERSION_CHECK_TIMEOUT_MS)
|
||||||
|
}),
|
||||||
|
])
|
||||||
|
|
||||||
|
if (killTimer) {
|
||||||
|
clearTimeout(killTimer)
|
||||||
|
}
|
||||||
|
|
||||||
|
if (timedExitResult.type === "timeout") {
|
||||||
|
void outputPromise.catch(() => {})
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if (killGraceTimer) {
|
||||||
|
clearTimeout(killGraceTimer)
|
||||||
|
}
|
||||||
|
|
||||||
|
let outputTimer: ReturnType<typeof setTimeout> | null = null
|
||||||
|
const outputResult = await Promise.race([
|
||||||
|
outputPromise.then((output) => ({ type: "output" as const, output })),
|
||||||
|
new Promise<{ type: "timeout" }>((resolve) => {
|
||||||
|
outputTimer = setTimeout(() => {
|
||||||
|
resolve({ type: "timeout" })
|
||||||
|
}, OPENCODE_OUTPUT_WAIT_TIMEOUT_MS)
|
||||||
|
}),
|
||||||
|
]).catch(() => ({ type: "timeout" as const }))
|
||||||
|
|
||||||
|
if (outputTimer) {
|
||||||
|
clearTimeout(outputTimer)
|
||||||
|
}
|
||||||
|
|
||||||
|
if (outputResult.type !== "output") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if (timedExitResult.exitCode === 0 && proc.exitCode === 0) {
|
||||||
|
const output = outputResult.output
|
||||||
const version = extractSemverFromOutput(output) ?? output.trim()
|
const version = extractSemverFromOutput(output) ?? output.trim()
|
||||||
|
if (version.length === 0) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
initConfigContext(binary, version)
|
initConfigContext(binary, version)
|
||||||
return { binary, version }
|
return { binary, version }
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user