fix: bridge respawned child signals (#933) (thanks @roshanasingh4)
Co-authored-by: Roshan Singh <roshanasingh4@users.noreply.github.com>
This commit is contained in:
@@ -24,6 +24,7 @@
|
|||||||
- WhatsApp: add `channels.whatsapp.sendReadReceipts` to disable auto read receipts. (#882) — thanks @chrisrodz.
|
- WhatsApp: add `channels.whatsapp.sendReadReceipts` to disable auto read receipts. (#882) — thanks @chrisrodz.
|
||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
- Gateway: forward termination signals to respawned CLI child processes to avoid orphaned systemd runs. (#933) — thanks @roshanasingh4.
|
||||||
- Browser: add tests for snapshot labels/efficient query params and labeled image responses.
|
- Browser: add tests for snapshot labels/efficient query params and labeled image responses.
|
||||||
- macOS: ensure launchd log directory exists with a test-only override. (#909) — thanks @roshanasingh4.
|
- macOS: ensure launchd log directory exists with a test-only override. (#909) — thanks @roshanasingh4.
|
||||||
- Packaging: run `pnpm build` on `prepack` so npm publishes include fresh `dist/` output.
|
- Packaging: run `pnpm build` on `prepack` so npm publishes include fresh `dist/` output.
|
||||||
|
|||||||
17
src/entry.ts
17
src/entry.ts
@@ -1,8 +1,9 @@
|
|||||||
#!/usr/bin/env node
|
#!/usr/bin/env node
|
||||||
|
import { spawn } from "node:child_process";
|
||||||
import process from "node:process";
|
import process from "node:process";
|
||||||
|
|
||||||
import { applyCliProfileEnv, parseCliProfileArgs } from "./cli/profile.js";
|
import { applyCliProfileEnv, parseCliProfileArgs } from "./cli/profile.js";
|
||||||
import { spawnWithSignalForwarding } from "./process/spawn-with-signal-forwarding.js";
|
import { attachChildProcessBridge } from "./process/child-process-bridge.js";
|
||||||
|
|
||||||
if (process.argv.includes("--no-color")) {
|
if (process.argv.includes("--no-color")) {
|
||||||
process.env.NO_COLOR = "1";
|
process.env.NO_COLOR = "1";
|
||||||
@@ -24,7 +25,7 @@ function ensureExperimentalWarningSuppressed(): boolean {
|
|||||||
process.env.CLAWDBOT_NODE_OPTIONS_READY = "1";
|
process.env.CLAWDBOT_NODE_OPTIONS_READY = "1";
|
||||||
process.env.NODE_OPTIONS = `${nodeOptions} ${EXPERIMENTAL_WARNING_FLAG}`.trim();
|
process.env.NODE_OPTIONS = `${nodeOptions} ${EXPERIMENTAL_WARNING_FLAG}`.trim();
|
||||||
|
|
||||||
const { child } = spawnWithSignalForwarding(
|
const child = spawn(
|
||||||
process.execPath,
|
process.execPath,
|
||||||
[...process.execArgv, ...process.argv.slice(1)],
|
[...process.execArgv, ...process.argv.slice(1)],
|
||||||
{
|
{
|
||||||
@@ -33,7 +34,9 @@ function ensureExperimentalWarningSuppressed(): boolean {
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
child.on("exit", (code, signal) => {
|
attachChildProcessBridge(child);
|
||||||
|
|
||||||
|
child.once("exit", (code, signal) => {
|
||||||
if (signal) {
|
if (signal) {
|
||||||
process.exitCode = 1;
|
process.exitCode = 1;
|
||||||
return;
|
return;
|
||||||
@@ -41,6 +44,14 @@ function ensureExperimentalWarningSuppressed(): boolean {
|
|||||||
process.exit(code ?? 1);
|
process.exit(code ?? 1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
child.once("error", (error) => {
|
||||||
|
console.error(
|
||||||
|
"[clawdbot] Failed to respawn CLI:",
|
||||||
|
error instanceof Error ? (error.stack ?? error.message) : error,
|
||||||
|
);
|
||||||
|
process.exit(1);
|
||||||
|
});
|
||||||
|
|
||||||
// Parent must not continue running the CLI.
|
// Parent must not continue running the CLI.
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,10 +1,11 @@
|
|||||||
|
import { spawn } from "node:child_process";
|
||||||
import net from "node:net";
|
import net from "node:net";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import process from "node:process";
|
import process from "node:process";
|
||||||
|
|
||||||
import { afterEach, describe, expect, it } from "vitest";
|
import { afterEach, describe, expect, it } from "vitest";
|
||||||
|
|
||||||
import { spawnWithSignalForwarding } from "./spawn-with-signal-forwarding.js";
|
import { attachChildProcessBridge } from "./child-process-bridge.js";
|
||||||
|
|
||||||
function waitForLine(stream: NodeJS.ReadableStream, timeoutMs = 10_000): Promise<string> {
|
function waitForLine(stream: NodeJS.ReadableStream, timeoutMs = 10_000): Promise<string> {
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
@@ -52,10 +53,19 @@ function canConnect(port: number): Promise<boolean> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
describe("spawnWithSignalForwarding", () => {
|
describe("attachChildProcessBridge", () => {
|
||||||
const children: Array<{ kill: (signal?: NodeJS.Signals) => boolean }> = [];
|
const children: Array<{ kill: (signal?: NodeJS.Signals) => boolean }> = [];
|
||||||
|
const detachments: Array<() => void> = [];
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
for (const detach of detachments) {
|
||||||
|
try {
|
||||||
|
detach();
|
||||||
|
} catch {
|
||||||
|
// ignore
|
||||||
|
}
|
||||||
|
}
|
||||||
|
detachments.length = 0;
|
||||||
for (const child of children) {
|
for (const child of children) {
|
||||||
try {
|
try {
|
||||||
child.kill("SIGKILL");
|
child.kill("SIGKILL");
|
||||||
@@ -67,15 +77,16 @@ describe("spawnWithSignalForwarding", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it(
|
it(
|
||||||
"forwards SIGTERM to spawned child",
|
"forwards SIGTERM to the wrapped child",
|
||||||
async () => {
|
async () => {
|
||||||
const tsxPath = path.resolve(process.cwd(), "node_modules/.bin/tsx");
|
const childPath = path.resolve(process.cwd(), "test/fixtures/child-process-bridge/child.js");
|
||||||
const childPath = path.resolve(process.cwd(), "test/fixtures/signal-forwarding/child.ts");
|
|
||||||
|
|
||||||
const { child } = spawnWithSignalForwarding(tsxPath, [childPath], {
|
const child = spawn(process.execPath, [childPath], {
|
||||||
stdio: ["ignore", "pipe", "inherit"],
|
stdio: ["ignore", "pipe", "inherit"],
|
||||||
env: process.env,
|
env: process.env,
|
||||||
});
|
});
|
||||||
|
const { detach } = attachChildProcessBridge(child);
|
||||||
|
detachments.push(detach);
|
||||||
children.push(child);
|
children.push(child);
|
||||||
|
|
||||||
if (!child.stdout) throw new Error("expected stdout");
|
if (!child.stdout) throw new Error("expected stdout");
|
||||||
47
src/process/child-process-bridge.ts
Normal file
47
src/process/child-process-bridge.ts
Normal file
@@ -0,0 +1,47 @@
|
|||||||
|
import type { ChildProcess } from "node:child_process";
|
||||||
|
import process from "node:process";
|
||||||
|
|
||||||
|
export type ChildProcessBridgeOptions = {
|
||||||
|
signals?: NodeJS.Signals[];
|
||||||
|
onSignal?: (signal: NodeJS.Signals) => void;
|
||||||
|
};
|
||||||
|
|
||||||
|
const defaultSignals: NodeJS.Signals[] =
|
||||||
|
process.platform === "win32"
|
||||||
|
? ["SIGTERM", "SIGINT", "SIGBREAK"]
|
||||||
|
: ["SIGTERM", "SIGINT", "SIGHUP", "SIGQUIT"];
|
||||||
|
|
||||||
|
export function attachChildProcessBridge(
|
||||||
|
child: ChildProcess,
|
||||||
|
{ signals = defaultSignals, onSignal }: ChildProcessBridgeOptions = {},
|
||||||
|
): { detach: () => void } {
|
||||||
|
const listeners = new Map<NodeJS.Signals, () => void>();
|
||||||
|
for (const signal of signals) {
|
||||||
|
const listener = (): void => {
|
||||||
|
onSignal?.(signal);
|
||||||
|
try {
|
||||||
|
child.kill(signal);
|
||||||
|
} catch {
|
||||||
|
// ignore
|
||||||
|
}
|
||||||
|
};
|
||||||
|
try {
|
||||||
|
process.on(signal, listener);
|
||||||
|
listeners.set(signal, listener);
|
||||||
|
} catch {
|
||||||
|
// Unsupported signal on this platform.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const detach = (): void => {
|
||||||
|
for (const [signal, listener] of listeners) {
|
||||||
|
process.off(signal, listener);
|
||||||
|
}
|
||||||
|
listeners.clear();
|
||||||
|
};
|
||||||
|
|
||||||
|
child.once("exit", detach);
|
||||||
|
child.once("error", detach);
|
||||||
|
|
||||||
|
return { detach };
|
||||||
|
}
|
||||||
@@ -1,40 +0,0 @@
|
|||||||
import type { ChildProcess, SpawnOptions } from "node:child_process";
|
|
||||||
import { spawn } from "node:child_process";
|
|
||||||
import process from "node:process";
|
|
||||||
|
|
||||||
export type SpawnWithSignalForwardingOptions = {
|
|
||||||
signals?: NodeJS.Signals[];
|
|
||||||
};
|
|
||||||
|
|
||||||
export function spawnWithSignalForwarding(
|
|
||||||
command: string,
|
|
||||||
args: string[],
|
|
||||||
options: SpawnOptions,
|
|
||||||
{ signals = ["SIGTERM", "SIGINT", "SIGHUP", "SIGQUIT"] }: SpawnWithSignalForwardingOptions = {},
|
|
||||||
): { child: ChildProcess; detach: () => void } {
|
|
||||||
const child = spawn(command, args, options);
|
|
||||||
|
|
||||||
const listeners = new Map<NodeJS.Signals, () => void>();
|
|
||||||
for (const signal of signals) {
|
|
||||||
const listener = (): void => {
|
|
||||||
try {
|
|
||||||
child.kill(signal);
|
|
||||||
} catch {
|
|
||||||
// ignore
|
|
||||||
}
|
|
||||||
};
|
|
||||||
listeners.set(signal, listener);
|
|
||||||
process.on(signal, listener);
|
|
||||||
}
|
|
||||||
|
|
||||||
const detach = (): void => {
|
|
||||||
for (const [signal, listener] of listeners) {
|
|
||||||
process.off(signal, listener);
|
|
||||||
}
|
|
||||||
listeners.clear();
|
|
||||||
};
|
|
||||||
|
|
||||||
child.once("exit", detach);
|
|
||||||
|
|
||||||
return { child, detach };
|
|
||||||
}
|
|
||||||
@@ -11,7 +11,7 @@ server.listen(0, "127.0.0.1", () => {
|
|||||||
process.stdout.write(`${addr.port}\n`);
|
process.stdout.write(`${addr.port}\n`);
|
||||||
});
|
});
|
||||||
|
|
||||||
const shutdown = (): void => {
|
const shutdown = () => {
|
||||||
server.close(() => process.exit(0));
|
server.close(() => process.exit(0));
|
||||||
};
|
};
|
||||||
|
|
||||||
Reference in New Issue
Block a user