isTerminated false after kill()

[mifi]

execa@npm:9.5.1

Looking at the docs it seems to me that when calling subprocess.kill execa should set isTerminated: true on the result. However I've found a case where this does not happen:

// test.js
import { execaNode } from 'execa'

const subprocess = execaNode('subprocess.js');

setTimeout(() => {
  subprocess.kill();
}, 2000)

await subprocess.catch((err) => console.log('err', err));
console.log('done');

// subprocess.js
setInterval(() => {
  console.log('tick')
}, 1000);

process.on('SIGTERM', () => process.exit(1));

When test.js is run it logs this:

err ExecaError: Command failed with exit code 1: subprocess.js

tick
    at getFinalError (file:///redacted/node_modules/execa/lib/return/final-error.js:6:9)
    at makeError (file:///redacted/node_modules/execa/lib/return/result.js:108:16)
    at getAsyncResult (file:///redacted/node_modules/execa/lib/methods/main-async.js:167:4)
    at handlePromise (file:///redacted/node_modules/execa/lib/methods/main-async.js:150:17)
    at async file:///redacted/test.mjs:9:1 {
  shortMessage: 'Command failed with exit code 1: subprocess.js',
  command: 'subprocess.js',
  escapedCommand: 'subprocess.js',
  cwd: '/redacted',
  durationMs: 2024.689,
  failed: true,
  timedOut: false,
  isCanceled: false,
  isGracefullyCanceled: false,
  isTerminated: false,
  isMaxBuffer: false,
  isForcefullyTerminated: false,
  exitCode: 1,
  stdout: 'tick',
  stderr: '',
  stdio: [ undefined, 'tick', '', undefined ],
  ipcOutput: [],
  pipedFrom: []
}
done

Not sure if this is a bug or expected behaviour. If it's expeced, should we update the docs to make it more clear? And how would you know I the process was killed in this case when catching the error?

Note: this is not related to execaNode, it happens for me with execa too (but I coudln't find a way to reproduce).

[ehmicky]

Hi @mifi,

error.isTerminated indicates that the subprocess ended due to a signal.

When you do process.on('SIGTERM'), Node.js handles SIGTERM which prevents the OS from terminating the subprocess.

Instead, you call process.exit(1), which is not a signal termination, but an "exit code termination". That type of termination is correctly reported with error.failed: true, error.exitCode set, and error.isTerminated: false.

[mifi]

Hi. In my real world use case it's ffmpeg that returns a non-zero exit code when receiving a SIGTERM, causing this scenario, so I cannot control how it exits. I'm not sure how I can determine in my catch block that ffmpeg exited due to kill() having been called or due to some other error (e.g. corrupt video file) that caused it to exit with a non-0 code. Maybe we could introduce a new property isKilled on the ExecaError which is true if kill() was ever called on the process? If that's not feasible, I'm not sure how's the best way to know whether the process was killed. Should we update the docs to say something like:

When a subprocess was terminated by a signal, error.isTerminated is true. Note that if the process exits with a non-zero exit code, isTerminated will be false even though the process was killed with a signal.

[ehmicky]

If a subprocess is terminated with a signal like SIGTERM (without being handled like your initial example), error.signal will be set and error.isTerminated will be true.

If not, but if error.exitCode is not undefined, then it means the subprocess failed with a non-0 exit code.

There are also a few other reasons why a subprocess might fail, documented here.

[mifi]

I did another test with cancelSignal:

// test.js
// test.js
import { execaNode } from 'execa'

const abortController = new AbortController()
const subprocess = execaNode('subprocess.js', { cancelSignal: abortController.signal });

setTimeout(() => {
  abortController.abort()
}, 2000)

await subprocess.catch((err) => console.log('err', err));
console.log('done');

am now getting:

err ExecaError: Command was canceled: subprocess.js
This operation was aborted

tick
    at getFinalError (file:///redacted/node_modules/execa/lib/return/final-error.js:6:9)
    at makeError (file:///redacted/node_modules/execa/lib/return/result.js:108:16)
    at getAsyncResult (file:///redacted/node_modules/execa/lib/methods/main-async.js:167:4)
    at handlePromise (file:///redacted/node_modules/execa/lib/methods/main-async.js:150:17)
    at async file:///redacted/test.mjs:11:1 {
  shortMessage: 'Command was canceled: subprocess.js\nThis operation was aborted',
  originalMessage: 'This operation was aborted',
  command: 'subprocess.js',
  escapedCommand: 'subprocess.js',
  cwd: '/redacted',
  durationMs: 2022.155792,
  failed: true,
  timedOut: false,
  isCanceled: true,
  isGracefullyCanceled: false,
  isTerminated: false,
  isMaxBuffer: false,
  isForcefullyTerminated: false,
  exitCode: 1,
  code: 20,
  stdout: 'tick',
  stderr: '',
  stdio: [ undefined, 'tick', '', undefined ],
  ipcOutput: [],
  pipedFrom: [],
  [cause]: DOMException [AbortError]: This operation was aborted
      at new DOMException (node:internal/per_context/domexception:53:5)
      at AbortController.abort (node:internal/abort_controller:401:18)
      at Timeout._onTimeout (file:///redacted/test.mjs:8:19)
      at listOnTimeout (node:internal/timers:569:17)
      at process.processTimers (node:internal/timers:512:7)
}
done

isCanceled is now true as expected. So I guess I should just use that instead of kill in my case.

[ehmicky]

This is expected as well. 👍