Skip to content

Commit 7019da8

Browse files
vincentkocvincentkoc
committed
fix(scripts): wait for extension boundary process groups
1 parent 5a15ea1 commit 7019da8

2 files changed

Lines changed: 153 additions & 20 deletions

File tree

‎scripts/check-extension-package-tsc-boundary.mjs‎

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ const prepareBoundaryArtifactsBin = resolve(
3535
const extensionPackageBoundaryBaseConfig = "../tsconfig.package-boundary.base.json";
3636
const FAILURE_OUTPUT_TAIL_LINES = 40;
3737
const STEP_OUTPUT_MAX_CHARS = 256 * 1024;
38+
const STEP_PROCESS_GROUP_EXIT_POLL_MS = 25;
39+
const STEP_POST_FORCE_KILL_WAIT_MS = 1_000;
3840
const SLOW_COMPILE_SUMMARY_LIMIT = 10;
3941
const COMPILE_INPUT_EXTENSIONS = new Set([".ts", ".tsx", ".mts", ".cts", ".js", ".mjs", ".json"]);
4042
const ROOTDIR_BOUNDARY_CANARY_IMPORT_PATH =
@@ -420,6 +422,34 @@ export function runNodeStepAsync(label, args, timeoutMs, params = {}) {
420422
child.kill(signal);
421423
}
422424
};
425+
const processGroupAlive = () => {
426+
if (platform === "win32" || typeof child.pid !== "number") {
427+
return false;
428+
}
429+
try {
430+
killProcess(-child.pid, 0);
431+
return true;
432+
} catch (error) {
433+
return error?.code === "EPERM";
434+
}
435+
};
436+
const waitForProcessGroupExit = async (ms) => {
437+
const deadlineAt = Date.now() + ms;
438+
while (Date.now() < deadlineAt) {
439+
if (!processGroupAlive()) {
440+
return true;
441+
}
442+
await new Promise((resolvePoll) => {
443+
setTimeout(resolvePoll, STEP_PROCESS_GROUP_EXIT_POLL_MS);
444+
});
445+
}
446+
return !processGroupAlive();
447+
};
448+
const waitAfterForceKill = async () => {
449+
if (processGroupAlive()) {
450+
await waitForProcessGroupExit(STEP_POST_FORCE_KILL_WAIT_MS);
451+
}
452+
};
423453
const abortSignal = abortController?.signal;
424454
const abortListener = () => {
425455
signalChild("SIGTERM");
@@ -443,30 +473,33 @@ export function runNodeStepAsync(label, args, timeoutMs, params = {}) {
443473
settled = true;
444474
cleanup();
445475
signalChild("SIGKILL");
446-
const stdoutText = formatCapturedStepOutput(stdout);
447-
const stderrText = formatCapturedStepOutput(stderr);
448-
const error = attachStepFailureMetadata(
449-
new Error(
450-
formatStepFailure(label, {
476+
void (async () => {
477+
await waitAfterForceKill();
478+
const stdoutText = formatCapturedStepOutput(stdout);
479+
const stderrText = formatCapturedStepOutput(stderr);
480+
const error = attachStepFailureMetadata(
481+
new Error(
482+
formatStepFailure(label, {
483+
stdout: stdoutText,
484+
stderr: stderrText,
485+
kind: "timeout",
486+
elapsedMs: Date.now() - startedAt,
487+
note: `${label} timed out after ${timeoutMs}ms`,
488+
}),
489+
),
490+
label,
491+
{
451492
stdout: stdoutText,
452493
stderr: stderrText,
453494
kind: "timeout",
454495
elapsedMs: Date.now() - startedAt,
455496
note: `${label} timed out after ${timeoutMs}ms`,
456-
}),
457-
),
458-
label,
459-
{
460-
stdout: stdoutText,
461-
stderr: stderrText,
462-
kind: "timeout",
463-
elapsedMs: Date.now() - startedAt,
464-
note: `${label} timed out after ${timeoutMs}ms`,
465-
},
466-
);
467-
onFailure?.(error);
468-
abortSiblingSteps(abortController);
469-
rejectPromise(toLintErrorObject(error, "Step timed out"));
497+
},
498+
);
499+
onFailure?.(error);
500+
abortSiblingSteps(abortController);
501+
rejectPromise(toLintErrorObject(error, "Step timed out"));
502+
})();
470503
}, timeoutMs);
471504

472505
child.stdout.setEncoding("utf8");

‎test/scripts/check-extension-package-tsc-boundary.test.ts‎

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Check Extension Package Tsc Boundary tests cover check extension package tsc boundary script behavior.
2+
import { spawn } from "node:child_process";
23
import { EventEmitter } from "node:events";
34
import fs from "node:fs";
45
import os from "node:os";
@@ -46,6 +47,43 @@ function createMockPipe() {
4647
return pipe;
4748
}
4849

50+
function isProcessAlive(pid: number): boolean {
51+
try {
52+
process.kill(pid, 0);
53+
return true;
54+
} catch {
55+
return false;
56+
}
57+
}
58+
59+
async function sleep(ms: number): Promise<void> {
60+
await new Promise((resolve) => {
61+
setTimeout(resolve, ms);
62+
});
63+
}
64+
65+
async function waitForFile(filePath: string, timeoutMs: number): Promise<void> {
66+
const deadlineAt = Date.now() + timeoutMs;
67+
while (Date.now() < deadlineAt) {
68+
if (fs.existsSync(filePath)) {
69+
return;
70+
}
71+
await sleep(25);
72+
}
73+
throw new Error(`timeout waiting for ${filePath}`);
74+
}
75+
76+
async function waitForDead(pid: number, timeoutMs: number): Promise<void> {
77+
const deadlineAt = Date.now() + timeoutMs;
78+
while (Date.now() < deadlineAt) {
79+
if (!isProcessAlive(pid)) {
80+
return;
81+
}
82+
await sleep(25);
83+
}
84+
throw new Error(`process still alive: ${pid}`);
85+
}
86+
4987
afterEach(() => {
5088
for (const rootDir of tempRoots) {
5189
fs.rmSync(rootDir, { force: true, recursive: true });
@@ -423,6 +461,7 @@ describe("check-extension-package-tsc-boundary", () => {
423461

424462
it("hard-kills timed out async node steps", async () => {
425463
const processSignals: Array<[number, NodeJS.Signals | number | undefined]> = [];
464+
let processGroupAlive = true;
426465
const child = new EventEmitter() as EventEmitter & {
427466
kill: (signal?: NodeJS.Signals | number) => boolean;
428467
pid: number;
@@ -445,6 +484,13 @@ describe("check-extension-package-tsc-boundary", () => {
445484
return child;
446485
},
447486
killProcess(pid: number, signal?: NodeJS.Signals | number) {
487+
if (signal === "SIGKILL") {
488+
processGroupAlive = false;
489+
}
490+
if (signal === 0 && !processGroupAlive) {
491+
processSignals.push([pid, signal]);
492+
throw Object.assign(new Error("gone"), { code: "ESRCH" });
493+
}
448494
processSignals.push([pid, signal]);
449495
return true;
450496
},
@@ -457,7 +503,10 @@ describe("check-extension-package-tsc-boundary", () => {
457503
(error: unknown) => error,
458504
);
459505

460-
expect(processSignals).toEqual([[-1234, "SIGKILL"]]);
506+
expect(processSignals).toEqual([
507+
[-1234, "SIGKILL"],
508+
[-1234, 0],
509+
]);
461510
expect(failure).toBeInstanceOf(Error);
462511
if (!(failure instanceof Error)) {
463512
throw new Error("expected timeout failure to reject with an Error");
@@ -466,6 +515,57 @@ describe("check-extension-package-tsc-boundary", () => {
466515
expect((failure as { kind?: unknown }).kind).toBe("timeout");
467516
});
468517

518+
it.skipIf(process.platform === "win32")(
519+
"waits for timed-out async node step process groups",
520+
async () => {
521+
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-extension-tsc-timeout-"));
522+
tempRoots.add(root);
523+
const childPidPath = path.join(root, "child.pid");
524+
let childPid = 0;
525+
const childScript = [
526+
"process.on('SIGTERM', () => {});",
527+
"setInterval(() => {}, 1000);",
528+
].join("");
529+
const parentScript = [
530+
"const { spawn } = require('node:child_process');",
531+
"const fs = require('node:fs');",
532+
`const child = spawn(process.execPath, ['-e', ${JSON.stringify(childScript)}], { stdio: 'ignore' });`,
533+
`fs.writeFileSync(${JSON.stringify(childPidPath)}, String(child.pid));`,
534+
"setInterval(() => {}, 1000);",
535+
].join("");
536+
537+
try {
538+
const failurePromise = runNodeStepAsync(
539+
"hung-step-group",
540+
["--eval", parentScript],
541+
100,
542+
{
543+
spawnImpl(command: string, args: string[], options: unknown) {
544+
return spawn(command, args, options as Parameters<typeof spawn>[2]);
545+
},
546+
},
547+
).then(
548+
() => {
549+
throw new Error("expected hung-step-group to time out");
550+
},
551+
(error: unknown) => error,
552+
);
553+
554+
await waitForFile(childPidPath, 2_000);
555+
childPid = Number.parseInt(fs.readFileSync(childPidPath, "utf8"), 10);
556+
expect(isProcessAlive(childPid)).toBe(true);
557+
558+
const failure = await failurePromise;
559+
expect(failure).toBeInstanceOf(Error);
560+
await waitForDead(childPid, 2_000);
561+
} finally {
562+
if (childPid && isProcessAlive(childPid)) {
563+
process.kill(childPid, "SIGKILL");
564+
}
565+
}
566+
},
567+
);
568+
469569
it("aborts concurrent sibling steps after the first failure", async () => {
470570
const startedAt = Date.now();
471571
const slowStepTimeoutMs = 60_000;

0 commit comments

Comments
 (0)