mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-07 18:42:25 +00:00
fix(e2e): preserve host command timeout grace
This commit is contained in:
@@ -14,6 +14,10 @@ export const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)
|
||||
const HOST_COMMAND_MAX_BUFFER_BYTES = 50 * 1024 * 1024;
|
||||
const HOST_COMMAND_WRAPPER_EXTRA_BUFFER_BYTES = 1024 * 1024;
|
||||
const HOST_COMMAND_WRAPPER_BACKSTOP_MS = 5_000;
|
||||
const HOST_COMMAND_TIMEOUT_KILL_GRACE_MS = 100;
|
||||
const HOST_COMMAND_STREAMING_TIMEOUT_KILL_GRACE_MS = 2_000;
|
||||
const HOST_COMMAND_PROCESS_GROUP_EXIT_POLL_MS = 25;
|
||||
const HOST_COMMAND_POST_FORCE_KILL_WAIT_MS = 100;
|
||||
const HOST_COMMAND_CHILD_PID_PREFIX = "__OPENCLAW_HOST_COMMAND_CHILD_PID__";
|
||||
const HOST_COMMAND_SPAWN_ERROR_PREFIX = "__OPENCLAW_HOST_COMMAND_SPAWN_ERROR__";
|
||||
const HOST_COMMAND_TIMEOUT_PREFIX = "__OPENCLAW_HOST_COMMAND_TIMEOUT__";
|
||||
@@ -108,6 +112,7 @@ writeSync(
|
||||
|
||||
let timedOut = false;
|
||||
let killTimer;
|
||||
let killDeadlineAt = 0;
|
||||
let outputExceeded = false;
|
||||
let stderrBytes = 0;
|
||||
let stdoutBytes = 0;
|
||||
@@ -132,6 +137,58 @@ function signalGroup(signal) {
|
||||
}
|
||||
}
|
||||
|
||||
function groupAlive() {
|
||||
if (!child.pid) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
process.kill(-child.pid, 0);
|
||||
return true;
|
||||
} catch (error) {
|
||||
return Boolean(error && error.code === "EPERM");
|
||||
}
|
||||
}
|
||||
|
||||
function finishTimedOut() {
|
||||
if (killTimer) {
|
||||
clearTimeout(killTimer);
|
||||
}
|
||||
writeSync(3, ${JSON.stringify(HOST_COMMAND_TIMEOUT_PREFIX)} + "{}\n");
|
||||
process.exit(124);
|
||||
}
|
||||
|
||||
function finishTimedOutAfterCleanup() {
|
||||
if (!groupAlive()) {
|
||||
finishTimedOut();
|
||||
return;
|
||||
}
|
||||
const pollMs = Math.max(1, Math.min(25, payload.timeoutKillGraceMs));
|
||||
let pollTimer;
|
||||
let forceFinishTimer;
|
||||
let postForceFinishTimer;
|
||||
const finish = () => {
|
||||
if (pollTimer) {
|
||||
clearInterval(pollTimer);
|
||||
}
|
||||
if (forceFinishTimer) {
|
||||
clearTimeout(forceFinishTimer);
|
||||
}
|
||||
if (postForceFinishTimer) {
|
||||
clearTimeout(postForceFinishTimer);
|
||||
}
|
||||
finishTimedOut();
|
||||
};
|
||||
pollTimer = setInterval(() => {
|
||||
if (!groupAlive()) {
|
||||
finish();
|
||||
}
|
||||
}, pollMs);
|
||||
forceFinishTimer = setTimeout(() => {
|
||||
signalGroup("SIGKILL");
|
||||
postForceFinishTimer = setTimeout(finish, pollMs);
|
||||
}, Math.max(0, killDeadlineAt - Date.now()));
|
||||
}
|
||||
|
||||
function forwardBounded(stream, chunk) {
|
||||
const currentBytes = stream === "stdout" ? stdoutBytes : stderrBytes;
|
||||
const nextBytes = currentBytes + chunk.byteLength;
|
||||
@@ -170,7 +227,8 @@ for (const signal of ["SIGHUP", "SIGINT", "SIGTERM"]) {
|
||||
const timeout = setTimeout(() => {
|
||||
timedOut = true;
|
||||
signalGroup("SIGTERM");
|
||||
killTimer = setTimeout(() => signalGroup("SIGKILL"), 100);
|
||||
killDeadlineAt = Date.now() + payload.timeoutKillGraceMs;
|
||||
killTimer = setTimeout(() => signalGroup("SIGKILL"), payload.timeoutKillGraceMs);
|
||||
killTimer.unref();
|
||||
}, payload.timeoutMs);
|
||||
timeout.unref();
|
||||
@@ -199,14 +257,13 @@ child.on("error", (error) => {
|
||||
});
|
||||
child.on("close", (code, signal) => {
|
||||
clearTimeout(timeout);
|
||||
if (timedOut) {
|
||||
finishTimedOutAfterCleanup();
|
||||
return;
|
||||
}
|
||||
if (killTimer) {
|
||||
clearTimeout(killTimer);
|
||||
}
|
||||
if (timedOut) {
|
||||
signalGroup("SIGKILL");
|
||||
writeSync(3, ${JSON.stringify(HOST_COMMAND_TIMEOUT_PREFIX)} + "{}\n");
|
||||
process.exit(124);
|
||||
}
|
||||
if (outputExceeded) {
|
||||
process.exit(1);
|
||||
}
|
||||
@@ -407,6 +464,7 @@ function runPosixTimedCommandSync(
|
||||
input: options.input,
|
||||
maxBufferBytes: HOST_COMMAND_MAX_BUFFER_BYTES,
|
||||
shell: invocation.shell,
|
||||
timeoutKillGraceMs: HOST_COMMAND_TIMEOUT_KILL_GRACE_MS,
|
||||
timeoutMs: options.timeoutMs,
|
||||
});
|
||||
return spawnSync(process.execPath, ["-e", POSIX_TIMEOUT_WRAPPER], {
|
||||
@@ -461,6 +519,29 @@ export async function runStreaming(
|
||||
}
|
||||
}
|
||||
};
|
||||
const streamingProcessGroupAlive = (): boolean => {
|
||||
if (!detached || !childPid) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
process.kill(-childPid, 0);
|
||||
return true;
|
||||
} catch (error) {
|
||||
return (error as NodeJS.ErrnoException).code === "EPERM";
|
||||
}
|
||||
};
|
||||
const waitForStreamingProcessGroupExit = async (timeoutMs: number): Promise<boolean> => {
|
||||
const deadlineAt = Date.now() + timeoutMs;
|
||||
while (Date.now() < deadlineAt) {
|
||||
if (!streamingProcessGroupAlive()) {
|
||||
return true;
|
||||
}
|
||||
await new Promise((resolvePoll) => {
|
||||
setTimeout(resolvePoll, HOST_COMMAND_PROCESS_GROUP_EXIT_POLL_MS);
|
||||
});
|
||||
}
|
||||
return !streamingProcessGroupAlive();
|
||||
};
|
||||
logStream?.on("error", (error) => {
|
||||
logStreamError = error;
|
||||
signalStreamingChild("SIGTERM");
|
||||
@@ -520,13 +601,32 @@ export async function runStreaming(
|
||||
|
||||
let timedOut = false;
|
||||
let killTimer: NodeJS.Timeout | undefined;
|
||||
let killDeadlineAt = 0;
|
||||
const waitForStreamingTimeoutCleanup = async (): Promise<void> => {
|
||||
if (!detached) {
|
||||
signalStreamingChild("SIGKILL");
|
||||
return;
|
||||
}
|
||||
const remainingGraceMs = Math.max(0, killDeadlineAt - Date.now());
|
||||
if (remainingGraceMs > 0) {
|
||||
await waitForStreamingProcessGroupExit(remainingGraceMs);
|
||||
}
|
||||
if (streamingProcessGroupAlive()) {
|
||||
signalStreamingChild("SIGKILL");
|
||||
await waitForStreamingProcessGroupExit(HOST_COMMAND_POST_FORCE_KILL_WAIT_MS);
|
||||
}
|
||||
};
|
||||
const timer =
|
||||
options.timeoutMs == null
|
||||
? undefined
|
||||
: setTimeout(() => {
|
||||
timedOut = true;
|
||||
signalHostCommandProcess(childPid, "SIGTERM");
|
||||
killTimer = setTimeout(() => signalHostCommandProcess(childPid, "SIGKILL"), 2_000);
|
||||
killDeadlineAt = Date.now() + HOST_COMMAND_STREAMING_TIMEOUT_KILL_GRACE_MS;
|
||||
killTimer = setTimeout(
|
||||
() => signalHostCommandProcess(childPid, "SIGKILL"),
|
||||
HOST_COMMAND_STREAMING_TIMEOUT_KILL_GRACE_MS,
|
||||
);
|
||||
killTimer.unref();
|
||||
}, options.timeoutMs);
|
||||
|
||||
@@ -546,12 +646,12 @@ export async function runStreaming(
|
||||
if (timer) {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
if (killTimer) {
|
||||
clearTimeout(killTimer);
|
||||
}
|
||||
removeParentSignalHandlers();
|
||||
if (timedOut) {
|
||||
signalStreamingChild("SIGKILL");
|
||||
await waitForStreamingTimeoutCleanup();
|
||||
}
|
||||
if (killTimer) {
|
||||
clearTimeout(killTimer);
|
||||
}
|
||||
if (logStream) {
|
||||
logStream.end();
|
||||
|
||||
@@ -15,7 +15,7 @@ import { tmpdir } from "node:os";
|
||||
import { basename, delimiter, join, win32 } from "node:path";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
extractLastOpenClawVersionFromLog,
|
||||
modelProviderConfigBatchJson,
|
||||
@@ -50,6 +50,7 @@ import {
|
||||
import { parseArgs as parseWindowsSmokeArgs } from "../../scripts/e2e/parallels/windows-smoke.ts";
|
||||
import { withEnv } from "../../src/test-utils/env.js";
|
||||
import { spawnNodeEvalSync } from "../../src/test-utils/node-process.js";
|
||||
import { cleanupTempDirs, makeTempDir } from "../helpers/temp-dir.js";
|
||||
|
||||
const WRAPPERS = {
|
||||
linux: "scripts/e2e/parallels-linux-smoke.sh",
|
||||
@@ -82,6 +83,11 @@ const TS_PATHS = {
|
||||
};
|
||||
|
||||
const OS_TS_PATHS = [TS_PATHS.linux, TS_PATHS.macos, TS_PATHS.windows];
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
cleanupTempDirs(tempDirs);
|
||||
});
|
||||
|
||||
function countNonEmptyLines(value: string): number {
|
||||
let count = 0;
|
||||
@@ -1300,28 +1306,50 @@ if (isPrlctl) {
|
||||
expect(result.stdout).toBeTypeOf("string");
|
||||
});
|
||||
|
||||
it("does not wait for host commands that trap SIGTERM after a timeout", () => {
|
||||
const startedAt = Date.now();
|
||||
const result = run(
|
||||
process.execPath,
|
||||
[
|
||||
"-e",
|
||||
[
|
||||
"process.on('SIGTERM', () => {});",
|
||||
"setTimeout(() => process.exit(77), 700);",
|
||||
"setInterval(() => {}, 1000);",
|
||||
].join(""),
|
||||
],
|
||||
{
|
||||
check: false,
|
||||
quiet: true,
|
||||
timeoutMs: 50,
|
||||
},
|
||||
);
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"lets timed host command descendants drain before force kill",
|
||||
() => {
|
||||
const tempDir = makeTempDir(tempDirs, "openclaw-parallels-host-command-drain-");
|
||||
const readyFile = join(tempDir, "ready");
|
||||
const drainFile = join(tempDir, "drained");
|
||||
const descendantScript = [
|
||||
"const { writeFileSync } = require('node:fs');",
|
||||
"writeFileSync(process.env.READY_FILE, 'ready');",
|
||||
"process.on('SIGTERM', () => {",
|
||||
" setTimeout(() => {",
|
||||
" writeFileSync(process.env.DRAIN_FILE, 'drained');",
|
||||
" process.exit(0);",
|
||||
" }, 25);",
|
||||
"});",
|
||||
"setInterval(() => {}, 1000);",
|
||||
].join("\n");
|
||||
const parentScript = [
|
||||
"const { spawn } = require('node:child_process');",
|
||||
`spawn(process.execPath, ['-e', ${JSON.stringify(descendantScript)}], { env: process.env, stdio: 'ignore' });`,
|
||||
"process.on('SIGTERM', () => process.exit(0));",
|
||||
"setInterval(() => {}, 1000);",
|
||||
].join("\n");
|
||||
|
||||
expect(result.status).toBe(124);
|
||||
expect(Date.now() - startedAt).toBeLessThan(500);
|
||||
});
|
||||
try {
|
||||
const result = run(process.execPath, ["-e", parentScript], {
|
||||
check: false,
|
||||
env: {
|
||||
...process.env,
|
||||
DRAIN_FILE: drainFile,
|
||||
READY_FILE: readyFile,
|
||||
},
|
||||
quiet: true,
|
||||
timeoutMs: 1_000,
|
||||
});
|
||||
|
||||
expect(result.status).toBe(124);
|
||||
expect(existsSync(readyFile)).toBe(true);
|
||||
expect(readFileSync(drainFile, "utf8")).toBe("drained");
|
||||
} finally {
|
||||
cleanupTempDirs(tempDirs);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")("throws checked timed host command timeouts", () => {
|
||||
expect(() =>
|
||||
@@ -1424,6 +1452,52 @@ setInterval(() => {}, 1000);
|
||||
}
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"lets timed streaming host command descendants drain before force kill",
|
||||
async () => {
|
||||
const tempDir = makeTempDir(tempDirs, "openclaw-parallels-streaming-host-command-drain-");
|
||||
const readyFile = join(tempDir, "ready");
|
||||
const drainFile = join(tempDir, "drained");
|
||||
const logPath = join(tempDir, "stream.log");
|
||||
const descendantScript = [
|
||||
"const { writeFileSync } = require('node:fs');",
|
||||
"writeFileSync(process.env.READY_FILE, 'ready');",
|
||||
"process.on('SIGTERM', () => {",
|
||||
" setTimeout(() => {",
|
||||
" writeFileSync(process.env.DRAIN_FILE, 'drained');",
|
||||
" process.exit(0);",
|
||||
" }, 50);",
|
||||
"});",
|
||||
"setInterval(() => {}, 1000);",
|
||||
].join("\n");
|
||||
const parentScript = [
|
||||
"const { spawn } = require('node:child_process');",
|
||||
`spawn(process.execPath, ['-e', ${JSON.stringify(descendantScript)}], { env: process.env, stdio: 'ignore' });`,
|
||||
"process.on('SIGTERM', () => process.exit(0));",
|
||||
"setInterval(() => {}, 1000);",
|
||||
].join("\n");
|
||||
|
||||
try {
|
||||
const statusPromise = runStreaming(process.execPath, ["-e", parentScript], {
|
||||
env: {
|
||||
...process.env,
|
||||
DRAIN_FILE: drainFile,
|
||||
READY_FILE: readyFile,
|
||||
},
|
||||
logPath,
|
||||
quiet: true,
|
||||
timeoutMs: 500,
|
||||
});
|
||||
|
||||
await waitFor(() => existsSync(readyFile), 2_000);
|
||||
await expect(statusPromise).resolves.toBe(124);
|
||||
expect(readFileSync(drainFile, "utf8")).toBe("drained");
|
||||
} finally {
|
||||
cleanupTempDirs(tempDirs);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("streams host command logs instead of retaining them in memory", async () => {
|
||||
const source = readFileSync(TS_PATHS.hostCommand, "utf8");
|
||||
const runStreamingBlock = source.slice(source.indexOf("export async function runStreaming"));
|
||||
|
||||
Reference in New Issue
Block a user