Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge forceKill and forceKillAfter options #285

Merged
merged 4 commits into from Jun 14, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 3 additions & 8 deletions index.d.ts
Expand Up @@ -280,19 +280,14 @@ declare namespace execa {
}

interface KillOptions {
/**
If the first signal does not terminate the child process after a specified timeout, a `SIGKILL` signal will be sent to the process.

@default true
*/
forceKill?: boolean;

/**
Milliseconds to wait for the child process to terminate before sending `SIGKILL`.

Can be disabled with `false`.

@default 5000
*/
forceKillAfter?: number;
forceKillAfterTimeout?: boolean | number;
ehmicky marked this conversation as resolved.
Show resolved Hide resolved
}

interface ExecaChildPromise<StdoutErrorType> {
Expand Down
16 changes: 10 additions & 6 deletions index.js
Expand Up @@ -231,21 +231,25 @@ function setKillTimeout(kill, signal, options, killResult) {
}, timeout).unref();
}

function shouldForceKill(signal, {forceKill}, killResult) {
return isSigterm(signal) && forceKill !== false && killResult;
function shouldForceKill(signal, {forceKillAfterTimeout}, killResult) {
return isSigterm(signal) && forceKillAfterTimeout !== false && killResult;
}

function isSigterm(signal) {
return signal === os.constants.signals.SIGTERM ||
(typeof signal === 'string' && signal.toUpperCase() === 'SIGTERM');
}

function getForceKillAfterTimeout({forceKillAfter = DEFAULT_FORCE_KILL_TIMEOUT}) {
if (!Number.isInteger(forceKillAfter) || forceKillAfter < 0) {
throw new TypeError(`Expected the \`forceKillAfter\` option to be a non-negative integer, got \`${forceKillAfter}\` (${typeof forceKillAfter})`);
function getForceKillAfterTimeout({forceKillAfterTimeout = true}) {
if (forceKillAfterTimeout === true) {
return DEFAULT_FORCE_KILL_TIMEOUT;
}

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

return forceKillAfterTimeout;
}

const execa = (file, args, options) => {
Expand Down
8 changes: 4 additions & 4 deletions index.test-d.ts
Expand Up @@ -123,10 +123,10 @@ execa('unicorns').kill();
execa('unicorns').kill('SIGKILL');
execa('unicorns').kill(undefined);
execa('unicorns').kill('SIGKILL', {});
execa('unicorns').kill('SIGKILL', {forceKill: true});
execa('unicorns').kill('SIGKILL', {forceKill: false});
execa('unicorns').kill('SIGKILL', {forceKillAfter: 42});
execa('unicorns').kill('SIGKILL', {forceKillAfter: undefined});
execa('unicorns').kill('SIGKILL', {forceKillAfterTimeout: true});
execa('unicorns').kill('SIGKILL', {forceKillAfterTimeout: false});
execa('unicorns').kill('SIGKILL', {forceKillAfterTimeout: 42});
execa('unicorns').kill('SIGKILL', {forceKillAfterTimeout: undefined});

expectType<ExecaChildProcess<string>>(execa('unicorns'));
expectType<ExecaReturnValue<string>>(await execa('unicorns'));
Expand Down
15 changes: 5 additions & 10 deletions readme.md
Expand Up @@ -117,7 +117,7 @@ try {
const subprocess = execa('node');
setTimeout(() => {
subprocess.kill('SIGTERM', {
forceKillAfter: 2000
forceKillAfterTimeout: 2000
});
}, 1000);
```
Expand All @@ -140,20 +140,15 @@ Returns a [`child_process` instance](https://nodejs.org/api/child_process.html#c

Same as the original [`child_process#kill()`](https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal) except: if `signal` is `SIGTERM` (the default value) and the child process is not terminated after 5 seconds, force it by sending `SIGKILL`.

##### options.forceKill
##### options.forceKillAfterTimeout

Type: `boolean`<br>
Default: `true`

If the first signal does not terminate the child process after a specified timeout, a `SIGKILL` signal will be sent to the process.

##### options.forceKillAfter

Type: `string`<br>
Type: `boolean | number`<br>
ehmicky marked this conversation as resolved.
Show resolved Hide resolved
Default: `5000`

Milliseconds to wait for the child process to terminate before sending `SIGKILL`.

Can be disabled with `false`.

#### cancel()

Similar to [`childProcess.kill()`](https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal). This is preferred when cancelling the child process execution as the error is more descriptive and [`childProcessResult.isCanceled`](#iscanceled) is set to `true`.
Expand Down
20 changes: 10 additions & 10 deletions test.js
Expand Up @@ -129,31 +129,31 @@ 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') {
test('`forceKill: false` should not kill after a timeout', async t => {
test('`forceKillAfterTimeout: false` should not kill after a timeout', async t => {
const subprocess = execa('node', ['fixtures/no-killable'], {stdio: ['ipc']});
await pEvent(subprocess, 'message');

subprocess.kill('SIGTERM', {forceKill: false, forceKillAfter: 50});
subprocess.kill('SIGTERM', {forceKillAfterTimeout: false});

t.true(isRunning(subprocess.pid));
subprocess.kill('SIGKILL');
});

test('`forceKillAfter: number` should kill after a timeout', async t => {
test('`forceKillAfterTimeout: number` should kill after a timeout', async t => {
const subprocess = execa('node', ['fixtures/no-killable'], {stdio: ['ipc']});
await pEvent(subprocess, 'message');

subprocess.kill('SIGTERM', {forceKillAfter: 50});
subprocess.kill('SIGTERM', {forceKillAfterTimeout: 50});

const {signal} = await t.throwsAsync(subprocess);
t.is(signal, 'SIGKILL');
});

test('`forceKill: true` should kill after a timeout', async t => {
test('`forceKillAfterTimeout: true` should kill after a timeout', async t => {
const subprocess = execa('node', ['fixtures/no-killable'], {stdio: ['ipc']});
await pEvent(subprocess, 'message');

subprocess.kill('SIGTERM', {forceKill: true});
subprocess.kill('SIGTERM', {forceKillAfterTimeout: true});

const {signal} = await t.throwsAsync(subprocess);
t.is(signal, 'SIGKILL');
Expand All @@ -169,15 +169,15 @@ if (process.platform !== 'win32') {
t.is(signal, 'SIGKILL');
});

test('`forceKillAfter` should not be a float', t => {
test('`forceKillAfterTimeout` should not be a float', t => {
t.throws(() => {
execa('noop').kill('SIGTERM', {forceKillAfter: 0.5});
execa('noop').kill('SIGTERM', {forceKillAfterTimeout: 0.5});
}, {instanceOf: TypeError, message: /non-negative integer/});
});

test('`forceKillAfter` should not be negative', t => {
test('`forceKillAfterTimeout` should not be negative', t => {
t.throws(() => {
execa('noop').kill('SIGTERM', {forceKillAfter: -1});
execa('noop').kill('SIGTERM', {forceKillAfterTimeout: -1});
}, {instanceOf: TypeError, message: /non-negative integer/});
});
}
Expand Down