From 00dbd14368d55b5acc9de3c76c934de8edfac0ec Mon Sep 17 00:00:00 2001 From: Jason Jean Date: Fri, 26 Jan 2024 19:05:04 -0500 Subject: [PATCH] =?UTF-8?q?fix(core):=20fix=20sending=20sigint=20to=20chil?= =?UTF-8?q?d=20tasks=20with=20the=20new=20psuedo=20tty=20=E2=80=A6=20(#213?= =?UTF-8?q?69)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jonathan Cammisuli --- .prettierignore | 4 ++- .../run-commands/run-commands.impl.ts | 15 +++++++++-- .../executors/run-script/run-script.impl.ts | 26 ++++++++++++------- packages/nx/src/native/command.rs | 13 +++++----- packages/nx/src/native/index.d.ts | 2 +- packages/nx/src/native/tests/command.spec.ts | 6 ++--- .../forked-process-task-runner.ts | 21 +++++++-------- packages/nx/src/utils/child-process.ts | 24 ++++++++++++++++- packages/nx/src/utils/exit-codes.ts | 16 ++++++++++++ 9 files changed, 90 insertions(+), 37 deletions(-) create mode 100644 packages/nx/src/utils/exit-codes.ts diff --git a/.prettierignore b/.prettierignore index f8d347e3e6db5..f104106c262db 100644 --- a/.prettierignore +++ b/.prettierignore @@ -17,6 +17,8 @@ packages/nx/src/plugins/js/lock-file/__fixtures__/**/*.* packages/**/schematics/**/files/**/*.html packages/**/generators/**/files/**/*.html 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/public/documentation graph/client/src/assets/environment.js @@ -37,4 +39,4 @@ CODEOWNERS /.nx/cache -.pnpm-store \ No newline at end of file +.pnpm-store diff --git a/packages/nx/src/executors/run-commands/run-commands.impl.ts b/packages/nx/src/executors/run-commands/run-commands.impl.ts index 19a55ab61cfa9..a9a2900935d25 100644 --- a/packages/nx/src/executors/run-commands/run-commands.impl.ts +++ b/packages/nx/src/executors/run-commands/run-commands.impl.ts @@ -5,6 +5,7 @@ import { env as appendLocalEnv } from 'npm-run-path'; import { ExecutorContext } from '../../config/misc-interfaces'; import * as chalk from 'chalk'; import { runCommand } from '../../native'; +import { PseudoTtyProcess } from '../../utils/child-process'; export const LARGE_BUFFER = 1024 * 1000000; @@ -230,7 +231,9 @@ async function createProcess( !commandConfig.prefix && !isParallel ) { - const cp = runCommand(commandConfig.command, cwd, env); + const cp = new PseudoTtyProcess( + runCommand(commandConfig.command, cwd, env) + ); return new Promise((res) => { 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); + } + }); }); } diff --git a/packages/nx/src/executors/run-script/run-script.impl.ts b/packages/nx/src/executors/run-script/run-script.impl.ts index 089ecbb5147f0..053388f8200d0 100644 --- a/packages/nx/src/executors/run-script/run-script.impl.ts +++ b/packages/nx/src/executors/run-script/run-script.impl.ts @@ -3,6 +3,8 @@ import type { ExecutorContext } from '../../config/misc-interfaces'; import * as path from 'path'; import { env as appendLocalEnv } from 'npm-run-path'; import { runCommand } from '../../native'; +import { PseudoTtyProcess } from '../../utils/child-process'; +import { signalToCode } from '../../utils/exit-codes'; export interface RunScriptOptions { script: string; @@ -16,20 +18,24 @@ export default async function ( const pm = getPackageManagerCommand(); try { await new Promise((res, rej) => { - const cp = runCommand( - pm.run(options.script, options.__unparsed__.join(' ')), - path.join( - context.root, - context.projectsConfigurations.projects[context.projectName].root - ), - { - ...process.env, - ...appendLocalEnv(), - } + const cp = new PseudoTtyProcess( + runCommand( + pm.run(options.script, options.__unparsed__.join(' ')), + path.join( + context.root, + context.projectsConfigurations.projects[context.projectName].root + ), + { + ...process.env, + ...appendLocalEnv(), + } + ) ); cp.onExit((code) => { if (code === 0) { res(); + } else if (code >= 128) { + process.exit(code); } else { rej(); } diff --git a/packages/nx/src/native/command.rs b/packages/nx/src/native/command.rs index 31a4675d4cbf6..82c275e36f1e7 100644 --- a/packages/nx/src/native/command.rs +++ b/packages/nx/src/native/command.rs @@ -39,14 +39,14 @@ pub enum ChildProcessMessage { pub struct ChildProcess { process_killer: Box, message_receiver: Receiver, - wait_receiver: Receiver, + wait_receiver: Receiver, } #[napi] impl ChildProcess { pub fn new( process_killer: Box, message_receiver: Receiver, - exit_receiver: Receiver, + exit_receiver: Receiver, ) -> Self { Self { process_killer, @@ -57,17 +57,16 @@ impl ChildProcess { #[napi] pub fn kill(&mut self) -> anyhow::Result<()> { - self.process_killer.kill().map_err(anyhow::Error::from)?; - Ok(()) + self.process_killer.kill().map_err(anyhow::Error::from) } #[napi] pub fn on_exit( &mut self, - #[napi(ts_arg_type = "(code: number) => void")] callback: JsFunction, + #[napi(ts_arg_type = "(message: string) => void")] callback: JsFunction, ) -> napi::Result<()> { let wait = self.wait_receiver.clone(); - let callback_tsfn: ThreadsafeFunction = + let callback_tsfn: ThreadsafeFunction = callback.create_threadsafe_function(0, |ctx| Ok(vec![ctx.value]))?; 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 drop(pair.master); 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)) diff --git a/packages/nx/src/native/index.d.ts b/packages/nx/src/native/index.d.ts index d9abbcf57d874..3a3fc09cacd97 100644 --- a/packages/nx/src/native/index.d.ts +++ b/packages/nx/src/native/index.d.ts @@ -148,7 +148,7 @@ export interface FileMap { export function testOnlyTransferFileMap(projectFiles: Record>, nonProjectFiles: Array): NxWorkspaceFilesExternals export class ChildProcess { kill(): void - onExit(callback: (code: number) => void): void + onExit(callback: (message: string) => void): void onOutput(callback: (message: string) => void): void } export class ImportResult { diff --git a/packages/nx/src/native/tests/command.spec.ts b/packages/nx/src/native/tests/command.spec.ts index 1d81b79b13add..5daa80d49c424 100644 --- a/packages/nx/src/native/tests/command.spec.ts +++ b/packages/nx/src/native/tests/command.spec.ts @@ -28,7 +28,7 @@ describe('runCommand', () => { }); childProcess.onExit(() => { - expect(output.trim()).toEqual('hello world'); + expect(output.trim()).toContain('hello world'); done(); }); }); @@ -41,9 +41,7 @@ describe('runCommand', () => { // check to make sure that we have ansi sequence characters only available in tty terminals }); childProcess.onExit((_) => { - expect(JSON.stringify(output)).toMatchInlineSnapshot( - `""\\u001b[33mtrue\\u001b[39m""` - ); + expect(JSON.stringify(output)).toContain('\\u001b[33mtrue\\u001b[39m'); done(); }); }); diff --git a/packages/nx/src/tasks-runner/forked-process-task-runner.ts b/packages/nx/src/tasks-runner/forked-process-task-runner.ts index ffd9c741ab419..fe661918e6a43 100644 --- a/packages/nx/src/tasks-runner/forked-process-task-runner.ts +++ b/packages/nx/src/tasks-runner/forked-process-task-runner.ts @@ -19,6 +19,7 @@ import { ChildProcess as NativeChildProcess, nxFork } from '../native'; import { PsuedoIPCServer } from './psuedo-ipc'; import { FORKED_PROCESS_OS_SOCKET_PATH } from '../daemon/socket-utils'; import { PseudoTtyProcess } from '../utils/child-process'; +import { signalToCode } from '../utils/exit-codes'; const forkScript = join(__dirname, './fork.js'); @@ -71,7 +72,7 @@ export class ForkedProcessTaskRunner { p.once('exit', (code, signal) => { this.processes.delete(p); - if (code === null) code = this.signalToCode(signal); + if (code === null) code = signalToCode(signal); if (code !== 0) { const results: BatchResults = {}; for (const rootTaskId of batchTaskGraph.roots) { @@ -236,6 +237,11 @@ export class ForkedProcessTaskRunner { return new Promise((res) => { 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({ code, terminalOutput, @@ -339,7 +345,7 @@ export class ForkedProcessTaskRunner { p.on('exit', (code, signal) => { 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 // print all the collected output| const terminalOutput = outWithErr.join(''); @@ -403,7 +409,7 @@ export class ForkedProcessTaskRunner { }); 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 // print all the collected output let terminalOutput = ''; @@ -445,13 +451,6 @@ export class ForkedProcessTaskRunner { 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() { this.psuedoIPC.onMessageFromChildren((message: Serializable) => { process.send(message); @@ -484,7 +483,7 @@ export class ForkedProcessTaskRunner { } }); // 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', () => { this.processes.forEach((p) => { diff --git a/packages/nx/src/utils/child-process.ts b/packages/nx/src/utils/child-process.ts index b9574dbfef3fd..2f9ae1d4dbbe7 100644 --- a/packages/nx/src/utils/child-process.ts +++ b/packages/nx/src/utils/child-process.ts @@ -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 { isAlive = true; exitCallbacks = []; constructor(private childProcess: ChildProcess) { - childProcess.onExit((exitCode) => { + childProcess.onExit((message) => { this.isAlive = false; + + const exitCode = messageToCode(message); + this.exitCallbacks.forEach((cb) => cb(exitCode)); }); } diff --git a/packages/nx/src/utils/exit-codes.ts b/packages/nx/src/utils/exit-codes.ts new file mode 100644 index 0000000000000..6ef982167f740 --- /dev/null +++ b/packages/nx/src/utils/exit-codes.ts @@ -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; + } +}