TypeError: cb is not a function when child exits with error

[aalexgabi]

I'm using Child_Process like this:

await pipeline([
  got.stream(url),
  DuplexChildProcess.spawn('convert', convertArgs),
  DuplexChildProcess.spawn('pngquant', quantArgs),
  ...
])

If pngquant command fails I have an unhandled error:

/home/.../node_modules/duplex-child-process/index.js:146
    cb && cb()
          ^

TypeError: cb is not a function
    at Child_Process.kill (/home/.../node_modules/duplex-child-process/index.js:146:11)
    at internal/streams/pipeline.js:47:61
    at internal/streams/pipeline.js:83:11
    at internal/util.js:392:14
    at Child_Process.<anonymous> (internal/streams/pipeline.js:33:21)
    at Child_Process.<anonymous> (internal/util.js:392:14)
    at Child_Process.onerror (internal/streams/end-of-stream.js:54:14)
    at Child_Process.emit (events.js:321:20)
    at ChildProcess.onExit (/home/.../node_modules/duplex-child-process/index.js:119:12)
    at Object.onceWrapper (events.js:428:26)
    at ChildProcess.emit (events.js:321:20)
    at maybeClose (internal/child_process.js:1026:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:286:5)

readable.destroy() and writable.destroy() expect an error argument: https://nodejs.org/api/stream.html#stream_readable_destroy_error

But Child_Process sets this function for destroy method:

this.kill = this.destroy = kill

And this function expects a cb:

  function kill(cb) {
    that._stdout.destroy()
    that._stderr.destroy()

    killed = true

    try {
      that._process.kill((options && options.killSignal) || 'SIGTERM')
    } catch (e) {
      ex = e
      onExit()
    }
    console.log('cb', cb, new Error().stack)
    cb && cb()
  }

[jeromew]

Hello,

Thanks for the report it looks indeed like there is a mismatch in the API.

Do you by chance have a simple error throwing example that we could add in the tests ? (without got and external resources, and with usual commands that are available in linux) ?

[aalexgabi]

@jeromew I guess something like:

echo hello | bash -c 'exit 1' | cat

[aalexgabi]

@jeromew I did a "fix" in a fork like this because it was crashing the process (the error was not passed inside the pipeline):

  function kill(err) {
    that._stdout.destroy()
    that._stderr.destroy()

    killed = true

    try {
      if (err) {
        ex = err
      }

      that._process.kill((options && options.killSignal) || 'SIGTERM')
    } catch (e) {
      ex = e
      onExit()
    }
  }

I'm not sure if it's correct though. I wonder if I should pass the err to that._stdout.destroy() for example.