Skip to content

Commit 61ec4a9

Browse files
committed
add skiller process timeout
1 parent f5cdf39 commit 61ec4a9

2 files changed

Lines changed: 95 additions & 8 deletions

File tree

packages/api/src/services/skiller.ts

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,22 +188,99 @@ const sleep = (durationMs: number): Promise<void> =>
188188
setTimeout(resolve, durationMs)
189189
})
190190

191-
const runProcess = (
191+
const defaultRunProcessTimeoutMs = 300_000
192+
193+
export class SkillerProcessTimeoutError extends Error {
194+
readonly args: ReadonlyArray<string>
195+
readonly command: string
196+
override readonly name = "SkillerProcessTimeoutError"
197+
readonly timeoutMs: number
198+
199+
constructor(
200+
command: string,
201+
args: ReadonlyArray<string>,
202+
timeoutMs: number,
203+
cause?: Error
204+
) {
205+
super(
206+
`${command} ${args.join(" ")} timed out after ${timeoutMs}ms.`,
207+
cause === undefined ? undefined : { cause }
208+
)
209+
this.args = args
210+
this.command = command
211+
this.timeoutMs = timeoutMs
212+
}
213+
}
214+
215+
export const runProcess = (
192216
command: string,
193217
args: ReadonlyArray<string>,
194-
options: SpawnOptions = {}
218+
options: SpawnOptions = {},
219+
timeoutMs: number = defaultRunProcessTimeoutMs
195220
): Promise<void> =>
196221
new Promise((resolve, reject) => {
197-
const child = spawn(command, [...args], options)
198-
child.once("error", reject)
199-
child.once("exit", (exitCode, signal) => {
222+
const timeoutController = new AbortController()
223+
let settled = false
224+
let timer: ReturnType<typeof setTimeout> | null = null
225+
let timeoutError: SkillerProcessTimeoutError | null = null
226+
227+
const child = spawn(command, [...args], { ...options, signal: timeoutController.signal })
228+
229+
const cleanup = (): void => {
230+
if (timer !== null) {
231+
clearTimeout(timer)
232+
timer = null
233+
}
234+
child.off("error", onError)
235+
child.off("exit", onExit)
236+
}
237+
238+
const resolveOnce = (): void => {
239+
if (settled) {
240+
return
241+
}
242+
settled = true
243+
cleanup()
244+
resolve()
245+
}
246+
247+
const rejectOnce = (error: Error): void => {
248+
if (settled) {
249+
return
250+
}
251+
settled = true
252+
cleanup()
253+
reject(error)
254+
}
255+
256+
const onError = (error: Error): void => {
257+
rejectOnce(
258+
timeoutError === null
259+
? error
260+
: new SkillerProcessTimeoutError(command, args, timeoutMs, error)
261+
)
262+
}
263+
264+
const onExit = (exitCode: number | null, signal: string | null): void => {
265+
if (timeoutError !== null) {
266+
rejectOnce(timeoutError)
267+
return
268+
}
200269
if (exitCode === 0) {
201-
resolve()
270+
resolveOnce()
202271
return
203272
}
204273
const reason = exitCode === null ? `signal ${signal}` : `exit code ${exitCode}`
205-
reject(new Error(`${command} ${args.join(" ")} failed with ${reason}.`))
206-
})
274+
rejectOnce(new Error(`${command} ${args.join(" ")} failed with ${reason}.`))
275+
}
276+
277+
child.once("error", onError)
278+
child.once("exit", onExit)
279+
timer = setTimeout(() => {
280+
timeoutError = new SkillerProcessTimeoutError(command, args, timeoutMs)
281+
timeoutController.abort(timeoutError)
282+
child.kill("SIGTERM")
283+
}, timeoutMs)
207284
})
208285

209286
const containerHomePath = (sshUser: string): string => `/home/${sshUser}`

packages/api/tests/skiller-routes.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import {
44
parseSkillerRoute,
55
resolveSkillerBrowserScopeSelection,
66
resolveSkillerRouteScopeSelection,
7+
runProcess,
8+
SkillerProcessTimeoutError,
79
skillerLaunchCommand,
810
type SkillerRoute
911
} from "../src/services/skiller.js"
@@ -74,6 +76,14 @@ describe("skiller routes", () => {
7476
expect(launch.userName).toBe("dg-skiller-u2147483001")
7577
})
7678

79+
it("fails stalled child processes with a distinct timeout error", () =>
80+
expect(runProcess(
81+
process.execPath,
82+
["-e", "setTimeout(() => undefined, 1_000)"],
83+
{},
84+
25
85+
)).rejects.toBeInstanceOf(SkillerProcessTimeoutError))
86+
7787
it("keeps the terminal session id on session-scoped app routes", () => {
7888
expect(parseSkillerRoute("/api/ssh/session/terminal-proof/skiller/app/")).toEqual({
7989
_tag: "App",

0 commit comments

Comments
 (0)