From 7c14102324bc3811672140719d96a56143421f10 Mon Sep 17 00:00:00 2001 From: Philipp Kunz Date: Thu, 20 Feb 2025 12:37:10 +0100 Subject: [PATCH] fix(core): Refactor Smartshell class for improved code clarity and performance --- changelog.md | 9 +++ ts/00_commitinfo_data.ts | 2 +- ts/classes.smartshell.ts | 169 +++++++++++++++++++-------------------- 3 files changed, 93 insertions(+), 87 deletions(-) diff --git a/changelog.md b/changelog.md index 3ef5462..57ba147 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,14 @@ # Changelog +## 2025-02-20 - 3.2.3 - fix(core) +Refactor Smartshell class for improved code clarity and performance + +- Refactored `_exec` method to improve code clarity. +- Introduced `IExecOptions` interface for better type handling. +- Replaced promise defer with native promises in command execution methods. +- Improved logging and error handling in child process execution. +- Ensured robust process management with signals handling. + ## 2024-12-13 - 3.2.2 - fix(core) Fix minor code style and formatting issues diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index 33fa0e8..2850ab5 100644 --- a/ts/00_commitinfo_data.ts +++ b/ts/00_commitinfo_data.ts @@ -3,6 +3,6 @@ */ export const commitinfo = { name: '@push.rocks/smartshell', - version: '3.2.2', + version: '3.2.3', description: 'A library for executing shell commands using promises.' } diff --git a/ts/classes.smartshell.ts b/ts/classes.smartshell.ts index 3f6dfbd..ce8ebcd 100644 --- a/ts/classes.smartshell.ts +++ b/ts/classes.smartshell.ts @@ -2,7 +2,6 @@ import * as plugins from './plugins.js'; import { ShellEnv } from './classes.shellenv.js'; import type { IShellEnvContructorOptions, TExecutor } from './classes.shellenv.js'; import { ShellLog } from './classes.shelllog.js'; - import * as cp from 'child_process'; // -- interfaces -- @@ -17,7 +16,15 @@ export interface IExecResultStreaming { kill: () => Promise; terminate: () => Promise; keyboardInterrupt: () => Promise; - customSignal: (signalArg: plugins.smartexit.TProcessSignal) => Promise; + customSignal: (signal: plugins.smartexit.TProcessSignal) => Promise; +} + +interface IExecOptions { + commandString: string; + silent?: boolean; + strict?: boolean; + streaming?: boolean; + interactive?: boolean; } export class Smartshell { @@ -29,61 +36,48 @@ export class Smartshell { } /** - * executes a given command async + * Executes a given command asynchronously. */ - private async _exec(options: { - commandString: string; - silent?: boolean; - strict?: boolean; - streaming?: boolean; - interactive?: boolean; - }): Promise { + private async _exec(options: IExecOptions): Promise { if (options.interactive) { - return await this._execInteractive(options); + return await this._execInteractive({ commandString: options.commandString }); } - return await this._execCommand(options); } - private async _execInteractive(options: { - commandString: string; - interactive?: boolean; - }): Promise { + /** + * Executes an interactive command. + */ + private async _execInteractive(options: Pick): Promise { + // Skip interactive execution in CI environments. if (process.env.CI) { return; } - const done = plugins.smartpromise.defer(); + return new Promise((resolve) => { + const shell = cp.spawn(options.commandString, { + stdio: 'inherit', + shell: true, + detached: true, + }); - const shell = cp.spawn(options.commandString, { - stdio: 'inherit', - shell: true, - detached: true + this.smartexit.addProcess(shell); + + shell.on('close', (code) => { + console.log(`Interactive shell terminated with code ${code}`); + this.smartexit.removeProcess(shell); + resolve(); + }); }); - - this.smartexit.addProcess(shell); - - shell.on('close', (code) => { - console.log(`interactive shell terminated with code ${code}`); - this.smartexit.removeProcess(shell); - done.resolve(); - }); - - await done.promise; } - private async _execCommand(options: { - commandString: string; - silent?: boolean; - strict?: boolean; - streaming?: boolean; - }): Promise { - const done = plugins.smartpromise.defer(); - const childProcessEnded = plugins.smartpromise.defer(); - + /** + * Executes a command and returns either a non-streaming result or a streaming interface. + */ + private async _execCommand(options: IExecOptions): Promise { const commandToExecute = this.shellEnv.createEnvExecString(options.commandString); - const shellLogInstance = new ShellLog(); + const execChildProcess = cp.spawn(commandToExecute, [], { shell: true, cwd: process.cwd(), @@ -93,6 +87,7 @@ export class Smartshell { this.smartexit.addProcess(execChildProcess); + // Capture stdout and stderr output. execChildProcess.stdout.on('data', (data) => { if (!options.silent) { shellLogInstance.writeToConsole(data); @@ -107,47 +102,55 @@ export class Smartshell { shellLogInstance.addToBuffer(data); }); - execChildProcess.on('exit', (code, signal) => { - this.smartexit.removeProcess(execChildProcess); - if (options.strict && code === 1) { - done.reject(); - } + // Wrap child process termination into a Promise. + const childProcessEnded: Promise = new Promise((resolve, reject) => { + execChildProcess.on('exit', (code, signal) => { + this.smartexit.removeProcess(execChildProcess); - const execResult = { - exitCode: code, - stdout: shellLogInstance.logStore.toString(), - }; + const execResult: IExecResult = { + exitCode: typeof code === 'number' ? code : (signal ? 1 : 0), + stdout: shellLogInstance.logStore.toString(), + }; - if (!options.streaming) { - done.resolve(execResult); - } - childProcessEnded.resolve(execResult); + if (options.strict && code !== 0) { + reject(new Error(`Command "${options.commandString}" exited with code ${code}`)); + } else { + resolve(execResult); + } + }); + + execChildProcess.on('error', (error) => { + this.smartexit.removeProcess(execChildProcess); + reject(error); + }); }); + // If streaming mode is enabled, return a streaming interface immediately. if (options.streaming) { - done.resolve({ + return { childProcess: execChildProcess, - finalPromise: childProcessEnded.promise, + finalPromise: childProcessEnded, kill: async () => { - console.log(`running tree kill with SIGKILL on process ${execChildProcess.pid}`); + console.log(`Running tree kill with SIGKILL on process ${execChildProcess.pid}`); await plugins.smartexit.SmartExit.killTreeByPid(execChildProcess.pid, 'SIGKILL'); }, terminate: async () => { - console.log(`running tree kill with SIGTERM on process ${execChildProcess.pid}`); + console.log(`Running tree kill with SIGTERM on process ${execChildProcess.pid}`); await plugins.smartexit.SmartExit.killTreeByPid(execChildProcess.pid, 'SIGTERM'); }, keyboardInterrupt: async () => { - console.log(`running tree kill with SIGINT on process ${execChildProcess.pid}`); + console.log(`Running tree kill with SIGINT on process ${execChildProcess.pid}`); await plugins.smartexit.SmartExit.killTreeByPid(execChildProcess.pid, 'SIGINT'); }, - customSignal: async (signalArg: plugins.smartexit.TProcessSignal) => { - console.log(`running tree kill with custom signal ${signalArg} on process ${execChildProcess.pid}`); - await plugins.smartexit.SmartExit.killTreeByPid(execChildProcess.pid, signalArg); + customSignal: async (signal: plugins.smartexit.TProcessSignal) => { + console.log(`Running tree kill with custom signal ${signal} on process ${execChildProcess.pid}`); + await plugins.smartexit.SmartExit.killTreeByPid(execChildProcess.pid, signal); }, - }); + } as IExecResultStreaming; } - return await done.promise; + // For non-streaming mode, wait for the process to complete. + return await childProcessEnded; } public async exec(commandString: string): Promise { @@ -166,41 +169,35 @@ export class Smartshell { return (await this._exec({ commandString, silent: true, strict: true })) as IExecResult; } - public async execStreaming( - commandString: string, - silent: boolean = false - ): Promise { + public async execStreaming(commandString: string, silent: boolean = false): Promise { return (await this._exec({ commandString, silent, streaming: true })) as IExecResultStreaming; } public async execStreamingSilent(commandString: string): Promise { - return (await this._exec({ - commandString, - silent: true, - streaming: true, - })) as IExecResultStreaming; + return (await this._exec({ commandString, silent: true, streaming: true })) as IExecResultStreaming; } - public async execInteractive(commandString: string) { + public async execInteractive(commandString: string): Promise { await this._exec({ commandString, interactive: true }); } public async execAndWaitForLine( commandString: string, - regexArg: RegExp, - silentArg: boolean = false - ) { - let done = plugins.smartpromise.defer(); - let execStreamingResult = await this.execStreaming(commandString, silentArg); - execStreamingResult.childProcess.stdout.on('data', (stdOutChunk: string) => { - if (regexArg.test(stdOutChunk)) { - done.resolve(); - } + regex: RegExp, + silent: boolean = false + ): Promise { + const execStreamingResult = await this.execStreaming(commandString, silent); + return new Promise((resolve) => { + execStreamingResult.childProcess.stdout.on('data', (chunk: Buffer | string) => { + const data = typeof chunk === 'string' ? chunk : chunk.toString(); + if (regex.test(data)) { + resolve(); + } + }); }); - return done.promise; } - public async execAndWaitForLineSilent(commandString: string, regexArg: RegExp) { - return this.execAndWaitForLine(commandString, regexArg, true); + public async execAndWaitForLineSilent(commandString: string, regex: RegExp): Promise { + return this.execAndWaitForLine(commandString, regex, true); } } \ No newline at end of file