Skip to content

Commit

Permalink
Rename forceKillAfterTimeout to forceKillAfterDelay (#723)
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Jan 24, 2024
1 parent 195369f commit 0ff8b34
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 50 deletions.
2 changes: 1 addition & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ type CommonOptions<IsSync extends boolean = boolean> = {
@default 5000
*/
forceKillAfterTimeout?: number | false;
forceKillAfterDelay?: number | false;

/**
If `true`, no quoting or escaping of arguments is done on Windows. Ignored on other platforms. This is set to `true` automatically when the `shell` option is `true`.
Expand Down
8 changes: 4 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {makeError} from './lib/error.js';
import {handleInputAsync, pipeOutputAsync} from './lib/stdio/async.js';
import {handleInputSync, pipeOutputSync} from './lib/stdio/sync.js';
import {normalizeStdioNode} from './lib/stdio/normalize.js';
import {spawnedKill, validateTimeout, normalizeForceKillAfterTimeout} from './lib/kill.js';
import {spawnedKill, validateTimeout, normalizeForceKillAfterDelay} from './lib/kill.js';
import {addPipeMethods} from './lib/pipe.js';
import {getSpawnedResult, makeAllStream} from './lib/stream.js';
import {mergePromise} from './lib/promise.js';
Expand Down Expand Up @@ -53,7 +53,7 @@ const handleArguments = (rawFile, rawArgs, rawOptions = {}) => {
validateTimeout(options);
options.shell = normalizeFileUrl(options.shell);
options.env = getEnv(options);
options.forceKillAfterTimeout = normalizeForceKillAfterTimeout(options.forceKillAfterTimeout);
options.forceKillAfterDelay = normalizeForceKillAfterDelay(options.forceKillAfterDelay);

if (process.platform === 'win32' && path.basename(file, '.exe') === 'cmd') {
// #116
Expand Down Expand Up @@ -81,7 +81,7 @@ const addDefaultOptions = ({
windowsHide = true,
verbose = verboseDefault,
killSignal = 'SIGTERM',
forceKillAfterTimeout = true,
forceKillAfterDelay = true,
...options
}) => ({
...options,
Expand All @@ -100,7 +100,7 @@ const addDefaultOptions = ({
windowsHide,
verbose,
killSignal,
forceKillAfterTimeout,
forceKillAfterDelay,
});

const handleOutput = (options, value) => {
Expand Down
16 changes: 8 additions & 8 deletions index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1187,14 +1187,14 @@ execa('unicorns', {killSignal: 'SIGTERM'});
execaSync('unicorns', {killSignal: 'SIGTERM'});
execa('unicorns', {killSignal: 9});
execaSync('unicorns', {killSignal: 9});
execa('unicorns', {forceKillAfterTimeout: false});
execaSync('unicorns', {forceKillAfterTimeout: false});
execa('unicorns', {forceKillAfterTimeout: 42});
execaSync('unicorns', {forceKillAfterTimeout: 42});
execa('unicorns', {forceKillAfterTimeout: undefined});
execaSync('unicorns', {forceKillAfterTimeout: undefined});
expectError(execa('unicorns', {forceKillAfterTimeout: 'true'}));
expectError(execaSync('unicorns', {forceKillAfterTimeout: 'true'}));
execa('unicorns', {forceKillAfterDelay: false});
execaSync('unicorns', {forceKillAfterDelay: false});
execa('unicorns', {forceKillAfterDelay: 42});
execaSync('unicorns', {forceKillAfterDelay: 42});
execa('unicorns', {forceKillAfterDelay: undefined});
execaSync('unicorns', {forceKillAfterDelay: undefined});
expectError(execa('unicorns', {forceKillAfterDelay: 'true'}));
expectError(execaSync('unicorns', {forceKillAfterDelay: 'true'}));
execa('unicorns', {signal: new AbortController().signal});
expectError(execaSync('unicorns', {signal: new AbortController().signal}));
execa('unicorns', {windowsVerbatimArguments: true});
Expand Down
28 changes: 14 additions & 14 deletions lib/kill.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,44 @@ import {onExit} from 'signal-exit';

const DEFAULT_FORCE_KILL_TIMEOUT = 1000 * 5;

// Monkey-patches `childProcess.kill()` to add `forceKillAfterTimeout` behavior
export const spawnedKill = (kill, {forceKillAfterTimeout}, controller, signal) => {
// Monkey-patches `childProcess.kill()` to add `forceKillAfterDelay` behavior
export const spawnedKill = (kill, {forceKillAfterDelay}, controller, signal) => {
const killResult = kill(signal);
setKillTimeout({kill, signal, forceKillAfterTimeout, killResult, controller});
setKillTimeout({kill, signal, forceKillAfterDelay, killResult, controller});
return killResult;
};

const setKillTimeout = async ({kill, signal, forceKillAfterTimeout, killResult, controller}) => {
if (!shouldForceKill(signal, forceKillAfterTimeout, killResult)) {
const setKillTimeout = async ({kill, signal, forceKillAfterDelay, killResult, controller}) => {
if (!shouldForceKill(signal, forceKillAfterDelay, killResult)) {
return;
}

try {
await setTimeout(forceKillAfterTimeout, undefined, {signal: controller.signal});
await setTimeout(forceKillAfterDelay, undefined, {signal: controller.signal});
kill('SIGKILL');
} catch {}
};

const shouldForceKill = (signal, forceKillAfterTimeout, killResult) => isSigterm(signal) && forceKillAfterTimeout !== false && killResult;
const shouldForceKill = (signal, forceKillAfterDelay, killResult) => isSigterm(signal) && forceKillAfterDelay !== false && killResult;

const isSigterm = signal => signal === undefined
|| signal === os.constants.signals.SIGTERM
|| (typeof signal === 'string' && signal.toUpperCase() === 'SIGTERM');

export const normalizeForceKillAfterTimeout = forceKillAfterTimeout => {
if (forceKillAfterTimeout === false) {
return forceKillAfterTimeout;
export const normalizeForceKillAfterDelay = forceKillAfterDelay => {
if (forceKillAfterDelay === false) {
return forceKillAfterDelay;
}

if (forceKillAfterTimeout === true) {
if (forceKillAfterDelay === true) {
return DEFAULT_FORCE_KILL_TIMEOUT;
}

if (!Number.isFinite(forceKillAfterTimeout) || forceKillAfterTimeout < 0) {
throw new TypeError(`Expected the \`forceKillAfterTimeout\` option to be a non-negative integer, got \`${forceKillAfterTimeout}\` (${typeof forceKillAfterTimeout})`);
if (!Number.isFinite(forceKillAfterDelay) || forceKillAfterDelay < 0) {
throw new TypeError(`Expected the \`forceKillAfterDelay\` option to be a non-negative integer, got \`${forceKillAfterDelay}\` (${typeof forceKillAfterDelay})`);
}

return forceKillAfterTimeout;
return forceKillAfterDelay;
};

const killAfterTimeout = async ({spawned, timeout, killSignal, context, controller}) => {
Expand Down
4 changes: 2 additions & 2 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ This package improves [`child_process`](https://nodejs.org/api/child_process.htm
- Redirect [`stdin`](#stdin)/[`stdout`](#stdout-1)/[`stderr`](#stderr-1) from/to files, streams, iterables, strings, `Uint8Array` or [objects](docs/transform.md#object-mode).
- [Transform](docs/transform.md) `stdin`/`stdout`/`stderr` with simple functions.
- Iterate over [each text line](docs/transform.md#binary-data) output by the process.
- [Fail-safe process termination](#forcekillaftertimeout).
- [Fail-safe process termination](#forcekillafterdelay).
- Get [interleaved output](#all) from `stdout` and `stderr` similar to what is printed on the terminal.
- [Strips the final newline](#stripfinalnewline) from the output so you don't have to do `stdout.trim()`.
- Convenience methods to pipe processes' [input](#input) and [output](#redirect-output-to-a-file).
Expand Down Expand Up @@ -754,7 +754,7 @@ Default: `SIGTERM`

Signal value to be used when the spawned process will be killed.

#### forceKillAfterTimeout
#### forceKillAfterDelay

Type: `number | false`\
Default: `5000`
Expand Down
42 changes: 21 additions & 21 deletions test/kill.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ setFixtureDir();

const TIMEOUT_REGEXP = /timed out after/;

const spawnNoKillable = async (forceKillAfterTimeout, options) => {
const spawnNoKillable = async (forceKillAfterDelay, options) => {
const subprocess = execa('no-killable.js', {
stdio: ['pipe', 'pipe', 'pipe', 'ipc'],
forceKillAfterTimeout,
forceKillAfterDelay,
...options,
});
await pEvent(subprocess, 'message');
Expand All @@ -33,8 +33,8 @@ test('kill("SIGKILL") should terminate cleanly', async t => {
// `SIGTERM` cannot be caught on Windows, and it always aborts the process (like `SIGKILL` on Unix).
// Therefore, this feature and those tests do not make sense on Windows.
if (process.platform !== 'win32') {
const testNoForceKill = async (t, forceKillAfterTimeout, killArgument) => {
const {subprocess} = await spawnNoKillable(forceKillAfterTimeout);
const testNoForceKill = async (t, forceKillAfterDelay, killArgument) => {
const {subprocess} = await spawnNoKillable(forceKillAfterDelay);

subprocess.kill(killArgument);

Expand All @@ -47,11 +47,11 @@ if (process.platform !== 'win32') {
t.is(signal, 'SIGKILL');
};

test('`forceKillAfterTimeout: false` should not kill after a timeout', testNoForceKill, false);
test('`forceKillAfterTimeout` should not kill after a timeout with other signals', testNoForceKill, true, 'SIGINT');
test('`forceKillAfterDelay: false` should not kill after a timeout', testNoForceKill, false);
test('`forceKillAfterDelay` should not kill after a timeout with other signals', testNoForceKill, true, 'SIGINT');

const testForceKill = async (t, forceKillAfterTimeout, killArgument) => {
const {subprocess} = await spawnNoKillable(forceKillAfterTimeout);
const testForceKill = async (t, forceKillAfterDelay, killArgument) => {
const {subprocess} = await spawnNoKillable(forceKillAfterDelay);

subprocess.kill(killArgument);

Expand All @@ -60,21 +60,21 @@ if (process.platform !== 'win32') {
t.is(signal, 'SIGKILL');
};

test('`forceKillAfterTimeout: number` should kill after a timeout', testForceKill, 50);
test('`forceKillAfterTimeout: true` should kill after a timeout', testForceKill, true);
test('`forceKillAfterTimeout: undefined` should kill after a timeout', testForceKill, undefined);
test('`forceKillAfterTimeout` should kill after a timeout with the killSignal', testForceKill, 50, 'SIGTERM');
test('`forceKillAfterDelay: number` should kill after a timeout', testForceKill, 50);
test('`forceKillAfterDelay: true` should kill after a timeout', testForceKill, true);
test('`forceKillAfterDelay: undefined` should kill after a timeout', testForceKill, undefined);
test('`forceKillAfterDelay` should kill after a timeout with the killSignal', testForceKill, 50, 'SIGTERM');

const testInvalidForceKill = async (t, forceKillAfterTimeout) => {
const testInvalidForceKill = async (t, forceKillAfterDelay) => {
t.throws(() => {
execa('empty.js', {forceKillAfterTimeout});
execa('empty.js', {forceKillAfterDelay});
}, {instanceOf: TypeError, message: /non-negative integer/});
};

test('`forceKillAfterTimeout` should not be NaN', testInvalidForceKill, Number.NaN);
test('`forceKillAfterTimeout` should not be negative', testInvalidForceKill, -1);
test('`forceKillAfterDelay` should not be NaN', testInvalidForceKill, Number.NaN);
test('`forceKillAfterDelay` should not be negative', testInvalidForceKill, -1);

test('`forceKillAfterTimeout` works with the "signal" option', async t => {
test('`forceKillAfterDelay` works with the "signal" option', async t => {
const abortController = new AbortController();
const {subprocess} = await spawnNoKillable(1, {signal: abortController.signal});
abortController.abort();
Expand All @@ -84,30 +84,30 @@ if (process.platform !== 'win32') {
t.true(isCanceled);
});

test.serial('`forceKillAfterTimeout` works with the "timeout" option', async t => {
test.serial('`forceKillAfterDelay` works with the "timeout" option', async t => {
const {subprocess} = await spawnNoKillable(1, {timeout: 2e3});
const {isTerminated, signal, timedOut} = await t.throwsAsync(subprocess);
t.true(isTerminated);
t.is(signal, 'SIGTERM');
t.true(timedOut);
});

test('`forceKillAfterTimeout` works with the "maxBuffer" option', async t => {
test('`forceKillAfterDelay` works with the "maxBuffer" option', async t => {
const {subprocess} = await spawnNoKillable(1, {maxBuffer: 1});
const {isTerminated, signal} = await t.throwsAsync(subprocess);
t.false(isTerminated);
t.is(signal, undefined);
});

test('`forceKillAfterTimeout` works with "error" events on childProcess', async t => {
test('`forceKillAfterDelay` works with "error" events on childProcess', async t => {
const {subprocess} = await spawnNoKillable(1);
subprocess.emit('error', new Error('test'));
const {isTerminated, signal} = await t.throwsAsync(subprocess);
t.false(isTerminated);
t.is(signal, undefined);
});

test('`forceKillAfterTimeout` works with "error" events on childProcess.stdout', async t => {
test('`forceKillAfterDelay` works with "error" events on childProcess.stdout', async t => {
const {subprocess} = await spawnNoKillable(1);
subprocess.stdout.destroy(new Error('test'));
const {isTerminated, signal} = await t.throwsAsync(subprocess);
Expand Down

0 comments on commit 0ff8b34

Please sign in to comment.