From 892595aea8022e03cf3c9492faba0806e1958351 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti <97394622+Gabriel-Ladzaretti@users.noreply.github.com> Date: Fri, 22 Jul 2022 18:43:28 +0300 Subject: [PATCH] feat(util/exec): use spawn instead of exec (#16414) * refactor(util): use spawn instead of exec - Use child_process.spawn instead of child_process.exec * refactor(util): use spawn instead of exec - Use child_process.spawn instead of child_process.exec * refactor(util): use spawn instead of exec - Use child_process.spawn instead of child_process.exec * refactor(util): use spawn instead of exec - Use child_process.spawn instead of child_process.exec * refactor(util): use spawn instead of exec - init spawn-util * refactor(util): use spawn instead of exec - spawn-util * refactor(util): use spawn instead of exec - init index-spawn.spec.ts * refactor(util): use spawn instead of exec - fixed various tests * refactor(util): use spawn instead of exec - fix all artifacts.spec.ts * refactor(util): use spawn instead of exec - fix all artifacts.spec.ts * refactor(util): use spawn instead of exec - fix npm post update imports * refactor(util): use spawn instead of exec - revert renaming to minimize PR diff * refactor(util): use spawn instead of exec - revert renaming to minimize PR diff * refactor(util): use spawn instead of exec - revert renaming to minimize PR diff * refactor(util): use spawn instead of exec - revert renaming to minimize PR diff * refactor(util): use spawn instead of exec - revert renaming to minimize PR diff - destroy stdio when terminating child process * refactor(util): use spawn instead of exec - delete and revert dev related changes * refactor(util): use spawn instead of exec - fix support for windows * refactor(util): use spawn instead of exec - handle SIGSTOP and such - add test coverage * refactor(util): use spawn instead of exec - now converts to strings when resolving/rejecting * refactor(util): use spawn instead of exec - logs improvements - force shell (exec like) - fix tests * refactor(util): use spawn instead of exec - strongly type listeners * refactor(util): use spawn instead of exec - create helper mock for spawn * refactor(util): use spawn instead of exec - cr changes * Update lib/util/exec/common.ts Co-authored-by: Sergei Zharinov * refactor(util): use spawn instead of exec - documentation * refactor(util): use spawn instead of exec - revert unnecessary formatting * refactor(util): use spawn instead of exec * refactor(util): use spawn instead of exec - added ExecError class * refactor(util): use spawn instead of exec - exec-error.ts restructure * refactor(util): use spawn instead of exec * Apply suggestions from code review Co-authored-by: Sergei Zharinov * refactor(util): use spawn instead of exec * refactor(util): use spawn instead of exec * refactor(util): use spawn instead of exec - deprecated RawExecOptions.encoding property * refactor(util): use spawn instead of exec * refactor(util): use spawn instead of exec * refactor(util): use spawn instead of exec Co-authored-by: Sergei Zharinov --- lib/util/exec/common.spec.ts | 285 +++++++++++++++++++++++++++++++++++ lib/util/exec/common.ts | 142 ++++++++++++++++- lib/util/exec/exec-error.ts | 44 ++++++ lib/util/exec/types.ts | 12 +- test/exec-util.ts | 5 + 5 files changed, 483 insertions(+), 5 deletions(-) create mode 100644 lib/util/exec/common.spec.ts create mode 100644 lib/util/exec/exec-error.ts diff --git a/lib/util/exec/common.spec.ts b/lib/util/exec/common.spec.ts new file mode 100644 index 00000000000000..16f86685d4c4ce --- /dev/null +++ b/lib/util/exec/common.spec.ts @@ -0,0 +1,285 @@ +import { spawn as _spawn } from 'child_process'; +import type { ChildProcess, SendHandle, Serializable } from 'child_process'; +import { Readable } from 'stream'; +import { mockedFunction, partial } from '../../../test/util'; +import { exec } from './common'; +import type { RawExecOptions } from './types'; + +jest.mock('child_process'); +const spawn = mockedFunction(_spawn); + +type MessageListener = (message: Serializable, sendHandle: SendHandle) => void; +type NoArgListener = () => void; +type EndListener = (code: number | null, signal: NodeJS.Signals | null) => void; +type ErrorListener = (err: Error) => void; + +type Listener = MessageListener | NoArgListener | EndListener | ErrorListener; + +interface Events { + close?: EndListener; + disconnect?: NoArgListener; + error?: ErrorListener; + exit?: EndListener; + message?: MessageListener; + spawn?: NoArgListener; +} + +interface StubArgs { + cmd: string; + exitCode: number | null; + exitSignal: NodeJS.Signals | null; + encoding?: BufferEncoding; + error?: Error; + stdout?: string; + stderr?: string; + timeout?: number; +} + +function getReadable( + data: string | undefined, + encoding: BufferEncoding +): Readable { + const readable = new Readable(); + readable._read = (size: number): void => { + /*do nothing*/ + }; + + readable.destroy = (error?: Error | undefined): Readable => { + return readable; + }; + + if (data !== undefined) { + readable.push(data, encoding); + readable.push(null); + } + + return readable; +} + +function getSpawnStub(args: StubArgs): ChildProcess { + const { + cmd, + error, + exitCode, + exitSignal, + stdout, + stderr, + encoding, + timeout, + } = args; + const listeners: Events = {}; + + // init listeners + const on = (name: string, cb: Listener) => { + const event = name as keyof Events; + if (listeners[event]) { + return; + } + switch (event) { + case 'exit': + listeners.exit = cb as EndListener; + break; + case 'error': + listeners.error = cb as ErrorListener; + break; + default: + break; + } + }; + + // init readable streams + const stdoutStream = getReadable(stdout, encoding ?? 'utf8'); + const stderrStream = getReadable(stderr, encoding ?? 'utf8'); + + // define class methods + const emit = (name: string, ...arg: (string | number | Error)[]): boolean => { + const event = name as keyof Events; + + switch (event) { + case 'error': + listeners.error?.(arg[0] as Error); + break; + case 'exit': + listeners.exit?.(arg[0] as number, arg[1] as NodeJS.Signals); + break; + default: + break; + } + + return !!listeners[event]; + }; + + const unref = (): void => { + /* do nothing*/ + }; + + const kill = (signal?: number | NodeJS.Signals | undefined): boolean => { + /* do nothing*/ + return true; + }; + + // queue events and wait for event loop to clear + setTimeout(() => { + if (error) { + listeners.error?.(error); + } + listeners.exit?.(exitCode, exitSignal); + }, 0); + + if (timeout) { + setTimeout(() => { + listeners.exit?.(null, 'SIGTERM'); + }, timeout); + } + + return { + on, + spawnargs: cmd.split(/\s+/), + stdout: stdoutStream, + stderr: stderrStream, + emit, + unref, + kill, + } as ChildProcess; +} + +describe('util/exec/common', () => { + const cmd = 'ls -l'; + const stdout = 'out message'; + const stderr = 'err message'; + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('rawExec', () => { + it('command exits with code 0', async () => { + const cmd = 'ls -l'; + const stub = getSpawnStub({ + cmd, + exitCode: 0, + exitSignal: null, + stdout, + stderr, + }); + spawn.mockImplementationOnce((cmd, opts) => stub); + await expect( + exec( + cmd, + partial({ encoding: 'utf8', shell: 'bin/bash' }) + ) + ).resolves.toEqual({ + stderr, + stdout, + }); + }); + + it('command exits with code 1', async () => { + const cmd = 'ls -l'; + const stderr = 'err'; + const exitCode = 1; + const stub = getSpawnStub({ cmd, exitCode, exitSignal: null, stderr }); + spawn.mockImplementationOnce((cmd, opts) => stub); + await expect( + exec(cmd, partial({ encoding: 'utf8' })) + ).rejects.toMatchObject({ + cmd, + message: 'Process exited with exit code "1"', + exitCode, + stderr, + }); + }); + + it('process terminated with SIGTERM', async () => { + const cmd = 'ls -l'; + const exitSignal = 'SIGTERM'; + const stub = getSpawnStub({ cmd, exitCode: null, exitSignal }); + spawn.mockImplementationOnce((cmd, opts) => stub); + await expect( + exec(cmd, partial({ encoding: 'utf8' })) + ).rejects.toMatchObject({ + cmd, + signal: exitSignal, + message: 'Process signaled with "SIGTERM"', + }); + }); + + it('process does nothing when signaled with SIGSTOP and eventually times out', async () => { + const cmd = 'ls -l'; + const stub = getSpawnStub({ + cmd, + exitCode: null, + exitSignal: 'SIGSTOP', + timeout: 500, + }); + spawn.mockImplementationOnce((cmd, opts) => stub); + await expect( + exec(cmd, partial({ encoding: 'utf8' })) + ).toReject(); + }); + + it('process exits due to error', async () => { + const cmd = 'ls -l'; + const errMsg = 'error message'; + const stub = getSpawnStub({ + cmd, + exitCode: null, + exitSignal: null, + error: new Error(errMsg), + }); + spawn.mockImplementationOnce((cmd, opts) => stub); + await expect( + exec(cmd, partial({ encoding: 'utf8' })) + ).rejects.toMatchObject({ cmd: 'ls -l', message: 'error message' }); + }); + + it('process exits with error due to exceeded stdout maxBuffer', async () => { + const cmd = 'ls -l'; + const stub = getSpawnStub({ + cmd, + exitCode: null, + exitSignal: null, + stdout: 'some message', + }); + spawn.mockImplementationOnce((cmd, opts) => stub); + await expect( + exec( + cmd, + partial({ + encoding: 'utf8', + maxBuffer: 5, + }) + ) + ).rejects.toMatchObject({ + cmd: 'ls -l', + message: 'stdout maxBuffer exceeded', + stderr: '', + stdout: '', + }); + }); + + it('process exits with error due to exceeded stderr maxBuffer', async () => { + const stub = getSpawnStub({ + cmd, + exitCode: null, + exitSignal: null, + stderr: 'some message', + }); + spawn.mockImplementationOnce((cmd, opts) => stub); + await expect( + exec( + cmd, + partial({ + encoding: 'utf8', + maxBuffer: 5, + }) + ) + ).rejects.toMatchObject({ + cmd: 'ls -l', + message: 'stderr maxBuffer exceeded', + stderr: '', + stdout: '', + }); + }); + }); +}); diff --git a/lib/util/exec/common.ts b/lib/util/exec/common.ts index 80e3be2e7dfe04..d0a5aa156f5c79 100644 --- a/lib/util/exec/common.ts +++ b/lib/util/exec/common.ts @@ -1,8 +1,144 @@ -import { exec } from 'child_process'; -import { promisify } from 'util'; +import { ChildProcess, spawn } from 'child_process'; +import { ExecError, ExecErrorData } from './exec-error'; import type { ExecResult, RawExecOptions } from './types'; +// https://man7.org/linux/man-pages/man7/signal.7.html#NAME +// Non TERM/CORE signals +// The following is step 3. in https://github.com/renovatebot/renovate/issues/16197#issuecomment-1171423890 +const NONTERM = [ + 'SIGCHLD', + 'SIGCLD', + 'SIGCONT', + 'SIGSTOP', + 'SIGTSTP', + 'SIGTTIN', + 'SIGTTOU', + 'SIGURG', + 'SIGWINCH', +]; + +const encoding = 'utf8'; + +function stringify(list: Buffer[]): string { + return Buffer.concat(list).toString(encoding); +} + +function initStreamListeners( + cp: ChildProcess, + opts: RawExecOptions & { maxBuffer: number } +): [Buffer[], Buffer[]] { + const stdout: Buffer[] = []; + const stderr: Buffer[] = []; + let stdoutLen = 0; + let stderrLen = 0; + + cp.stdout?.on('data', (chunk: Buffer) => { + // process.stdout.write(data.toString()); + const len = Buffer.byteLength(chunk, encoding); + stdoutLen += len; + if (stdoutLen > opts.maxBuffer) { + cp.emit('error', new Error('stdout maxBuffer exceeded')); + } else { + stdout.push(chunk); + } + }); + + cp.stderr?.on('data', (chunk: Buffer) => { + // process.stderr.write(data.toString()); + const len = Buffer.byteLength(chunk, encoding); + stderrLen += len; + if (stderrLen > opts.maxBuffer) { + cp.emit('error', new Error('stderr maxBuffer exceeded')); + } else { + stderr.push(chunk); + } + }); + return [stdout, stderr]; +} + +export function exec(cmd: string, opts: RawExecOptions): Promise { + return new Promise((resolve, reject) => { + const maxBuffer = opts.maxBuffer ?? 10 * 1024 * 1024; // Set default max buffer size to 10MB + const cp = spawn(cmd, { + ...opts, + // force detached on non WIN platforms + // https://github.com/nodejs/node/issues/21825#issuecomment-611328888 + detached: process.platform !== 'win32', + shell: typeof opts.shell === 'string' ? opts.shell : true, // force shell + }); + + // handle streams + const [stdout, stderr] = initStreamListeners(cp, { + ...opts, + maxBuffer, + }); + + // handle process events + cp.on('error', (error) => { + kill(cp, 'SIGTERM'); + reject(new ExecError(error.message, rejectInfo(), error)); + }); + + cp.on('exit', (code: number, signal: NodeJS.Signals) => { + if (NONTERM.includes(signal)) { + return; + } + + if (signal) { + const message = `Process signaled with "${signal}"`; + kill(cp, signal); + reject(new ExecError(message, { ...rejectInfo(), signal })); + return; + } + if (code !== 0) { + const message = `Process exited with exit code "${code}"`; + reject(new ExecError(message, { ...rejectInfo(), exitCode: code })); + return; + } + resolve({ + stderr: stringify(stderr), + stdout: stringify(stdout), + }); + }); + + function rejectInfo(): ExecErrorData { + return { + cmd: cp.spawnargs.join(' '), + options: opts, + stdout: stringify(stdout), + stderr: stringify(stderr), + }; + } + }); +} + +function kill(cp: ChildProcess, signal: NodeJS.Signals): boolean { + try { + // TODO: will be enabled in #16654 + /** + * If `pid` is negative, but not `-1`, signal shall be sent to all processes + * (excluding an unspecified set of system processes), + * whose process group ID (pgid) is equal to the absolute value of pid, + * and for which the process has permission to send a signal. + */ + // process.kill(-(cp.pid as number), signal); + + // destroying stdio is needed for unref to work + // https://nodejs.org/api/child_process.html#subprocessunref + // https://github.com/nodejs/node/blob/4d5ff25a813fd18939c9f76b17e36291e3ea15c3/lib/child_process.js#L412-L426 + cp.stderr?.destroy(); + cp.stdout?.destroy(); + cp.unref(); + return cp.kill(signal); + } catch (err) { + // cp is a single node tree, therefore -pid is invalid as there is no such pgid, + // istanbul ignore next: will be covered once we use process.kill + return false; + } +} + +// TODO: rename #16653 export const rawExec: ( cmd: string, opts: RawExecOptions -) => Promise = promisify(exec); +) => Promise = exec; // TODO: rename #16653 diff --git a/lib/util/exec/exec-error.ts b/lib/util/exec/exec-error.ts new file mode 100644 index 00000000000000..b3cd7254139ffc --- /dev/null +++ b/lib/util/exec/exec-error.ts @@ -0,0 +1,44 @@ +import type { RawExecOptions } from './types'; + +export interface ExecErrorData { + cmd: string; + stderr: string; + stdout: string; + options: RawExecOptions; + exitCode?: number; + signal?: NodeJS.Signals; +} + +export class ExecError extends Error { + cmd: string; + stderr: string; + stdout: string; + options: RawExecOptions; + exitCode?: number; + signal?: NodeJS.Signals; + err?: Error; + + constructor(message: string, data: ExecErrorData, err?: Error) { + const { cmd, exitCode, stderr, stdout, options, signal } = data; + + super(message); + + this.name = this.constructor.name; + this.cmd = cmd; + this.stderr = stderr; + this.stdout = stdout; + this.options = options; + + if (exitCode) { + this.exitCode = exitCode; + } + + if (signal) { + this.signal = signal; + } + + if (err) { + this.err = err; + } + } +} diff --git a/lib/util/exec/types.ts b/lib/util/exec/types.ts index b97279b2af8675..cfdced685c0264 100644 --- a/lib/util/exec/types.ts +++ b/lib/util/exec/types.ts @@ -1,4 +1,4 @@ -import type { ExecOptions as ChildProcessExecOptions } from 'child_process'; +import type { SpawnOptions as ChildProcessSpawnOptions } from 'child_process'; export interface ToolConstraint { toolName: string; @@ -27,10 +27,17 @@ export interface DockerOptions { cwd?: Opt; } -export interface RawExecOptions extends ChildProcessExecOptions { +// TODO: rename #16653 +export interface RawExecOptions extends ChildProcessSpawnOptions { + // TODO: to be removed in #16655 + /** + * @deprecated renovate uses utf8, encoding property is ignored. + */ encoding: string; + maxBuffer?: number | undefined; } +// TODO: rename #16653 export interface ExecResult { stdout: string; stderr: string; @@ -38,6 +45,7 @@ export interface ExecResult { export type ExtraEnv = Record; +// TODO: rename #16653 export interface ExecOptions { cwd?: string; cwdFile?: string; diff --git a/test/exec-util.ts b/test/exec-util.ts index ff9a2269eba81c..35dc99bfbca57d 100644 --- a/test/exec-util.ts +++ b/test/exec-util.ts @@ -8,6 +8,7 @@ import { mockedFunction } from './util'; jest.mock('../lib/util/exec/common'); +// TODO: rename #16653 export type ExecResult = { stdout: string; stderr: string } | Error; export const exec = mockedFunction(_exec); @@ -17,8 +18,10 @@ export interface ExecSnapshot { options?: RawExecOptions | null | undefined; } +// TODO: rename #16653 export type ExecSnapshots = ExecSnapshot[]; +// TODO: rename #16653 function execSnapshot(cmd: string, options?: RawExecOptions): ExecSnapshot { const snapshot = { cmd, @@ -39,6 +42,7 @@ function execSnapshot(cmd: string, options?: RawExecOptions): ExecSnapshot { const defaultExecResult = { stdout: '', stderr: '' }; +// TODO: rename #16653 export function mockExecAll( execResult: ExecResult = defaultExecResult ): ExecSnapshots { @@ -53,6 +57,7 @@ export function mockExecAll( return snapshots; } +// TODO: rename #16653 export function mockExecSequence(execResults: ExecResult[]): ExecSnapshots { const snapshots: ExecSnapshots = []; execResults.forEach((execResult) => {