fix(core): fix sending sigint to child tasks with the new psuedo tty … (#21369)
Co-authored-by: Jonathan Cammisuli <jon@cammisuli.ca>
This commit is contained in:
parent
e1bb8bccf1
commit
00dbd14368
@ -17,6 +17,8 @@ packages/nx/src/plugins/js/lock-file/__fixtures__/**/*.*
|
|||||||
packages/**/schematics/**/files/**/*.html
|
packages/**/schematics/**/files/**/*.html
|
||||||
packages/**/generators/**/files/**/*.html
|
packages/**/generators/**/files/**/*.html
|
||||||
packages/nx/src/native/**/*.rs
|
packages/nx/src/native/**/*.rs
|
||||||
|
packages/nx/src/native/index.js
|
||||||
|
packages/nx/src/native/index.d.ts
|
||||||
nx-dev/nx-dev/.next/
|
nx-dev/nx-dev/.next/
|
||||||
nx-dev/nx-dev/public/documentation
|
nx-dev/nx-dev/public/documentation
|
||||||
graph/client/src/assets/environment.js
|
graph/client/src/assets/environment.js
|
||||||
@ -37,4 +39,4 @@ CODEOWNERS
|
|||||||
|
|
||||||
/.nx/cache
|
/.nx/cache
|
||||||
|
|
||||||
.pnpm-store
|
.pnpm-store
|
||||||
|
|||||||
@ -5,6 +5,7 @@ import { env as appendLocalEnv } from 'npm-run-path';
|
|||||||
import { ExecutorContext } from '../../config/misc-interfaces';
|
import { ExecutorContext } from '../../config/misc-interfaces';
|
||||||
import * as chalk from 'chalk';
|
import * as chalk from 'chalk';
|
||||||
import { runCommand } from '../../native';
|
import { runCommand } from '../../native';
|
||||||
|
import { PseudoTtyProcess } from '../../utils/child-process';
|
||||||
|
|
||||||
export const LARGE_BUFFER = 1024 * 1000000;
|
export const LARGE_BUFFER = 1024 * 1000000;
|
||||||
|
|
||||||
@ -230,7 +231,9 @@ async function createProcess(
|
|||||||
!commandConfig.prefix &&
|
!commandConfig.prefix &&
|
||||||
!isParallel
|
!isParallel
|
||||||
) {
|
) {
|
||||||
const cp = runCommand(commandConfig.command, cwd, env);
|
const cp = new PseudoTtyProcess(
|
||||||
|
runCommand(commandConfig.command, cwd, env)
|
||||||
|
);
|
||||||
|
|
||||||
return new Promise((res) => {
|
return new Promise((res) => {
|
||||||
cp.onOutput((output) => {
|
cp.onOutput((output) => {
|
||||||
@ -239,7 +242,15 @@ async function createProcess(
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
cp.onExit((code) => res(code === 0));
|
cp.onExit((code) => {
|
||||||
|
if (code === 0) {
|
||||||
|
res(true);
|
||||||
|
} else if (code >= 128) {
|
||||||
|
process.exit(code);
|
||||||
|
} else {
|
||||||
|
res(false);
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -3,6 +3,8 @@ import type { ExecutorContext } from '../../config/misc-interfaces';
|
|||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import { env as appendLocalEnv } from 'npm-run-path';
|
import { env as appendLocalEnv } from 'npm-run-path';
|
||||||
import { runCommand } from '../../native';
|
import { runCommand } from '../../native';
|
||||||
|
import { PseudoTtyProcess } from '../../utils/child-process';
|
||||||
|
import { signalToCode } from '../../utils/exit-codes';
|
||||||
|
|
||||||
export interface RunScriptOptions {
|
export interface RunScriptOptions {
|
||||||
script: string;
|
script: string;
|
||||||
@ -16,20 +18,24 @@ export default async function (
|
|||||||
const pm = getPackageManagerCommand();
|
const pm = getPackageManagerCommand();
|
||||||
try {
|
try {
|
||||||
await new Promise<void>((res, rej) => {
|
await new Promise<void>((res, rej) => {
|
||||||
const cp = runCommand(
|
const cp = new PseudoTtyProcess(
|
||||||
pm.run(options.script, options.__unparsed__.join(' ')),
|
runCommand(
|
||||||
path.join(
|
pm.run(options.script, options.__unparsed__.join(' ')),
|
||||||
context.root,
|
path.join(
|
||||||
context.projectsConfigurations.projects[context.projectName].root
|
context.root,
|
||||||
),
|
context.projectsConfigurations.projects[context.projectName].root
|
||||||
{
|
),
|
||||||
...process.env,
|
{
|
||||||
...appendLocalEnv(),
|
...process.env,
|
||||||
}
|
...appendLocalEnv(),
|
||||||
|
}
|
||||||
|
)
|
||||||
);
|
);
|
||||||
cp.onExit((code) => {
|
cp.onExit((code) => {
|
||||||
if (code === 0) {
|
if (code === 0) {
|
||||||
res();
|
res();
|
||||||
|
} else if (code >= 128) {
|
||||||
|
process.exit(code);
|
||||||
} else {
|
} else {
|
||||||
rej();
|
rej();
|
||||||
}
|
}
|
||||||
|
|||||||
@ -39,14 +39,14 @@ pub enum ChildProcessMessage {
|
|||||||
pub struct ChildProcess {
|
pub struct ChildProcess {
|
||||||
process_killer: Box<dyn ChildKiller + Sync + Send>,
|
process_killer: Box<dyn ChildKiller + Sync + Send>,
|
||||||
message_receiver: Receiver<String>,
|
message_receiver: Receiver<String>,
|
||||||
wait_receiver: Receiver<u32>,
|
wait_receiver: Receiver<String>,
|
||||||
}
|
}
|
||||||
#[napi]
|
#[napi]
|
||||||
impl ChildProcess {
|
impl ChildProcess {
|
||||||
pub fn new(
|
pub fn new(
|
||||||
process_killer: Box<dyn ChildKiller + Sync + Send>,
|
process_killer: Box<dyn ChildKiller + Sync + Send>,
|
||||||
message_receiver: Receiver<String>,
|
message_receiver: Receiver<String>,
|
||||||
exit_receiver: Receiver<u32>,
|
exit_receiver: Receiver<String>,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
Self {
|
Self {
|
||||||
process_killer,
|
process_killer,
|
||||||
@ -57,17 +57,16 @@ impl ChildProcess {
|
|||||||
|
|
||||||
#[napi]
|
#[napi]
|
||||||
pub fn kill(&mut self) -> anyhow::Result<()> {
|
pub fn kill(&mut self) -> anyhow::Result<()> {
|
||||||
self.process_killer.kill().map_err(anyhow::Error::from)?;
|
self.process_killer.kill().map_err(anyhow::Error::from)
|
||||||
Ok(())
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[napi]
|
#[napi]
|
||||||
pub fn on_exit(
|
pub fn on_exit(
|
||||||
&mut self,
|
&mut self,
|
||||||
#[napi(ts_arg_type = "(code: number) => void")] callback: JsFunction,
|
#[napi(ts_arg_type = "(message: string) => void")] callback: JsFunction,
|
||||||
) -> napi::Result<()> {
|
) -> napi::Result<()> {
|
||||||
let wait = self.wait_receiver.clone();
|
let wait = self.wait_receiver.clone();
|
||||||
let callback_tsfn: ThreadsafeFunction<u32, Fatal> =
|
let callback_tsfn: ThreadsafeFunction<String, Fatal> =
|
||||||
callback.create_threadsafe_function(0, |ctx| Ok(vec![ctx.value]))?;
|
callback.create_threadsafe_function(0, |ctx| Ok(vec![ctx.value]))?;
|
||||||
|
|
||||||
std::thread::spawn(move || {
|
std::thread::spawn(move || {
|
||||||
@ -205,7 +204,7 @@ pub fn run_command(
|
|||||||
// make sure that master is only dropped after we wait on the child. Otherwise windows does not like it
|
// make sure that master is only dropped after we wait on the child. Otherwise windows does not like it
|
||||||
drop(pair.master);
|
drop(pair.master);
|
||||||
disable_raw_mode().expect("Failed to restore non-raw terminal");
|
disable_raw_mode().expect("Failed to restore non-raw terminal");
|
||||||
exit_tx.send(exit.exit_code()).ok();
|
exit_tx.send(exit.to_string()).ok();
|
||||||
});
|
});
|
||||||
|
|
||||||
Ok(ChildProcess::new(process_killer, message_rx, exit_rx))
|
Ok(ChildProcess::new(process_killer, message_rx, exit_rx))
|
||||||
|
|||||||
2
packages/nx/src/native/index.d.ts
vendored
2
packages/nx/src/native/index.d.ts
vendored
@ -148,7 +148,7 @@ export interface FileMap {
|
|||||||
export function testOnlyTransferFileMap(projectFiles: Record<string, Array<FileData>>, nonProjectFiles: Array<FileData>): NxWorkspaceFilesExternals
|
export function testOnlyTransferFileMap(projectFiles: Record<string, Array<FileData>>, nonProjectFiles: Array<FileData>): NxWorkspaceFilesExternals
|
||||||
export class ChildProcess {
|
export class ChildProcess {
|
||||||
kill(): void
|
kill(): void
|
||||||
onExit(callback: (code: number) => void): void
|
onExit(callback: (message: string) => void): void
|
||||||
onOutput(callback: (message: string) => void): void
|
onOutput(callback: (message: string) => void): void
|
||||||
}
|
}
|
||||||
export class ImportResult {
|
export class ImportResult {
|
||||||
|
|||||||
@ -28,7 +28,7 @@ describe('runCommand', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
childProcess.onExit(() => {
|
childProcess.onExit(() => {
|
||||||
expect(output.trim()).toEqual('hello world');
|
expect(output.trim()).toContain('hello world');
|
||||||
done();
|
done();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@ -41,9 +41,7 @@ describe('runCommand', () => {
|
|||||||
// check to make sure that we have ansi sequence characters only available in tty terminals
|
// check to make sure that we have ansi sequence characters only available in tty terminals
|
||||||
});
|
});
|
||||||
childProcess.onExit((_) => {
|
childProcess.onExit((_) => {
|
||||||
expect(JSON.stringify(output)).toMatchInlineSnapshot(
|
expect(JSON.stringify(output)).toContain('\\u001b[33mtrue\\u001b[39m');
|
||||||
`""\\u001b[33mtrue\\u001b[39m""`
|
|
||||||
);
|
|
||||||
done();
|
done();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -19,6 +19,7 @@ import { ChildProcess as NativeChildProcess, nxFork } from '../native';
|
|||||||
import { PsuedoIPCServer } from './psuedo-ipc';
|
import { PsuedoIPCServer } from './psuedo-ipc';
|
||||||
import { FORKED_PROCESS_OS_SOCKET_PATH } from '../daemon/socket-utils';
|
import { FORKED_PROCESS_OS_SOCKET_PATH } from '../daemon/socket-utils';
|
||||||
import { PseudoTtyProcess } from '../utils/child-process';
|
import { PseudoTtyProcess } from '../utils/child-process';
|
||||||
|
import { signalToCode } from '../utils/exit-codes';
|
||||||
|
|
||||||
const forkScript = join(__dirname, './fork.js');
|
const forkScript = join(__dirname, './fork.js');
|
||||||
|
|
||||||
@ -71,7 +72,7 @@ export class ForkedProcessTaskRunner {
|
|||||||
|
|
||||||
p.once('exit', (code, signal) => {
|
p.once('exit', (code, signal) => {
|
||||||
this.processes.delete(p);
|
this.processes.delete(p);
|
||||||
if (code === null) code = this.signalToCode(signal);
|
if (code === null) code = signalToCode(signal);
|
||||||
if (code !== 0) {
|
if (code !== 0) {
|
||||||
const results: BatchResults = {};
|
const results: BatchResults = {};
|
||||||
for (const rootTaskId of batchTaskGraph.roots) {
|
for (const rootTaskId of batchTaskGraph.roots) {
|
||||||
@ -236,6 +237,11 @@ export class ForkedProcessTaskRunner {
|
|||||||
|
|
||||||
return new Promise((res) => {
|
return new Promise((res) => {
|
||||||
p.onExit((code) => {
|
p.onExit((code) => {
|
||||||
|
// If the exit code is greater than 128, it's a special exit code for a signal
|
||||||
|
if (code >= 128) {
|
||||||
|
process.exit(code);
|
||||||
|
}
|
||||||
|
|
||||||
res({
|
res({
|
||||||
code,
|
code,
|
||||||
terminalOutput,
|
terminalOutput,
|
||||||
@ -339,7 +345,7 @@ export class ForkedProcessTaskRunner {
|
|||||||
|
|
||||||
p.on('exit', (code, signal) => {
|
p.on('exit', (code, signal) => {
|
||||||
this.processes.delete(p);
|
this.processes.delete(p);
|
||||||
if (code === null) code = this.signalToCode(signal);
|
if (code === null) code = signalToCode(signal);
|
||||||
// we didn't print any output as we were running the command
|
// we didn't print any output as we were running the command
|
||||||
// print all the collected output|
|
// print all the collected output|
|
||||||
const terminalOutput = outWithErr.join('');
|
const terminalOutput = outWithErr.join('');
|
||||||
@ -403,7 +409,7 @@ export class ForkedProcessTaskRunner {
|
|||||||
});
|
});
|
||||||
|
|
||||||
p.on('exit', (code, signal) => {
|
p.on('exit', (code, signal) => {
|
||||||
if (code === null) code = this.signalToCode(signal);
|
if (code === null) code = signalToCode(signal);
|
||||||
// we didn't print any output as we were running the command
|
// we didn't print any output as we were running the command
|
||||||
// print all the collected output
|
// print all the collected output
|
||||||
let terminalOutput = '';
|
let terminalOutput = '';
|
||||||
@ -445,13 +451,6 @@ export class ForkedProcessTaskRunner {
|
|||||||
writeFileSync(outputPath, content);
|
writeFileSync(outputPath, content);
|
||||||
}
|
}
|
||||||
|
|
||||||
private signalToCode(signal: string) {
|
|
||||||
if (signal === 'SIGHUP') return 128 + 1;
|
|
||||||
if (signal === 'SIGINT') return 128 + 2;
|
|
||||||
if (signal === 'SIGTERM') return 128 + 15;
|
|
||||||
return 128;
|
|
||||||
}
|
|
||||||
|
|
||||||
private setupProcessEventListeners() {
|
private setupProcessEventListeners() {
|
||||||
this.psuedoIPC.onMessageFromChildren((message: Serializable) => {
|
this.psuedoIPC.onMessageFromChildren((message: Serializable) => {
|
||||||
process.send(message);
|
process.send(message);
|
||||||
@ -484,7 +483,7 @@ export class ForkedProcessTaskRunner {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
// we exit here because we don't need to write anything to cache.
|
// we exit here because we don't need to write anything to cache.
|
||||||
process.exit(this.signalToCode('SIGINT'));
|
process.exit(signalToCode('SIGINT'));
|
||||||
});
|
});
|
||||||
process.on('SIGTERM', () => {
|
process.on('SIGTERM', () => {
|
||||||
this.processes.forEach((p) => {
|
this.processes.forEach((p) => {
|
||||||
|
|||||||
@ -76,14 +76,36 @@ export async function runNxAsync(
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function messageToCode(message: string): number {
|
||||||
|
if (message.startsWith('Terminated by ')) {
|
||||||
|
switch (message.replace('Terminated by ', '').trim()) {
|
||||||
|
case 'Termination':
|
||||||
|
return 143;
|
||||||
|
case 'Interrupt':
|
||||||
|
return 130;
|
||||||
|
default:
|
||||||
|
return 128;
|
||||||
|
}
|
||||||
|
} else if (message.startsWith('Exited with code ')) {
|
||||||
|
return parseInt(message.replace('Exited with code ', '').trim());
|
||||||
|
} else if (message === 'Success') {
|
||||||
|
return 0;
|
||||||
|
} else {
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export class PseudoTtyProcess {
|
export class PseudoTtyProcess {
|
||||||
isAlive = true;
|
isAlive = true;
|
||||||
|
|
||||||
exitCallbacks = [];
|
exitCallbacks = [];
|
||||||
|
|
||||||
constructor(private childProcess: ChildProcess) {
|
constructor(private childProcess: ChildProcess) {
|
||||||
childProcess.onExit((exitCode) => {
|
childProcess.onExit((message) => {
|
||||||
this.isAlive = false;
|
this.isAlive = false;
|
||||||
|
|
||||||
|
const exitCode = messageToCode(message);
|
||||||
|
|
||||||
this.exitCallbacks.forEach((cb) => cb(exitCode));
|
this.exitCallbacks.forEach((cb) => cb(exitCode));
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
16
packages/nx/src/utils/exit-codes.ts
Normal file
16
packages/nx/src/utils/exit-codes.ts
Normal file
@ -0,0 +1,16 @@
|
|||||||
|
/**
|
||||||
|
* Translates NodeJS signals to numeric exit code
|
||||||
|
* @param signal
|
||||||
|
*/
|
||||||
|
export function signalToCode(signal: NodeJS.Signals): number {
|
||||||
|
switch (signal) {
|
||||||
|
case 'SIGHUP':
|
||||||
|
return 128 + 1;
|
||||||
|
case 'SIGINT':
|
||||||
|
return 128 + 2;
|
||||||
|
case 'SIGTERM':
|
||||||
|
return 128 + 15;
|
||||||
|
default:
|
||||||
|
return 128;
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
x
Reference in New Issue
Block a user