zx icon indicating copy to clipboard operation
zx copied to clipboard

Awkward behavior on error with pipe

Open mighty1231 opened this issue 2 years ago • 3 comments

single exit 1 process - throws error

exit 1 process with piped - does not throw error

Is it intended behavior?

async function checkPipe() {
  try {
    await $`exit 1`.pipe($`echo hello`);
    console.log("not error at first");
  } catch (e) {
    console.log("it does not reach here (1)");
  }

  try {
    await $`exit 1`;
    console.log("it does not reach here (2)");
  } catch (e) {
    console.log("error catched!");
  }
}

await checkPipe();

Expected Stdout

$ exit 1
$ echo hello
hello
it does not reach here (1)
error catched!

Actual Behavior

$ exit 1
$ echo hello
hello
not error at first
file:///wd/temp.ts:106
    await $`exit 1`.pipe($`echo hello`);
           ^
ProcessOutput [Error]: 
    at checkPipe (file:///wd/temp.ts:106:12)
    exit code: 1
    at ChildProcess.<anonymous> (file:///wd/node_modules/zx/build/core.js:146:26)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess.emit (node:domain:489:12)
    at maybeClose (node:internal/child_process:1100:16)
    at Socket.<anonymous> (node:internal/child_process:458:11)
    at Socket.emit (node:events:513:28)
    at Socket.emit (node:domain:489:12)
    at Pipe.<anonymous> (node:net:301:12)
    at Pipe.callbackTrampoline (node:internal/async_hooks:130:17) {
  _code: 1,
  _signal: null,
  _stdout: '',
  _stderr: '',
  _combined: ''
}

Specifications

  • Version: 7.2.2
  • Platform: Linux (Ubuntu 18.04)

mighty1231 avatar Jun 11 '23 04:06 mighty1231

@antongolub what do you think is a proper behavior here?

antonmedv avatar Sep 12 '23 14:09 antonmedv

Well, the queue is pretty clear:

  1. $`exit 1` creates the first delayed action (setImmediate)
  2. $`echo hello` creates the second
  3. The pipe method bounds processes via internal postrun / prerun hooks.
  4. Then the 1st deferred setImmediate triggers internal _run, it fails and drowns the bound dest._prerun = this.run.bind(this).

I think, this is fine. Just imagine a case, when some piped process awaits for the input and meets exit 1 (like cat). I'd expect it to fall.

antongolub avatar Mar 26 '24 20:03 antongolub

@antonmedv Is it fine that none of the log executed after the second $ call?

neither these logs shown

  • it does not reach here (2)
  • error catched!

Let's rewrite the issue in more detailed way with 3 examples.

case 1 (normal)

just a single $ statement handles exception in nice way

import { $ } from "zx/core";

async function checkPipe() {
  try {
    await $`exit 1`;
    console.log("it does not reach here (2)");
  } catch (e) {
    console.log("error catched!");
  }
}

await checkPipe();

result

$ node abc.mjs 2>/dev/null
error catched!

case 2 (weird)

import { $ } from "zx/core";

async function checkPipe() {
  // run a piped process that fails
  try {
    await $`exit 1`.pipe($`echo hello`);
  } catch (e) {
    // do nothing
  }

  console.log('no matter for intermediate log');

  // same as case 1 below
  try {
    await $`exit 1`;
    console.log("it does not reach here (2)");
  } catch (e) {
    console.log("error catched!");
  }
}

await checkPipe();

result

$ node abc.mjs 2>/dev/null
no matter for intermediate log

where does the log error catched! go?

case 3

How about pipe in bash instead zx's pipe?

import { $ } from "zx/core";

async function checkPipe() {
  try {
    // bash's pipe
    await $`exit 1 | echo hello`;
  } catch (e) {
    // do nothing
  }

  console.log('no matter for intermediate log');

  try {
    await $`exit 1`;
    console.log("it does not reach here (2)");
  } catch (e) {
    console.log("error catched!");
  }
}

await checkPipe();

result

$ node abc.mjs 2>/dev/null
no matter for intermediate log
error catched!

Hello error catched!

Specification

  • Version: 7.2.2
  • Platform: Linux (Ubuntu 22.04)
  • Node version: v16.20.2

mighty1231 avatar Mar 29 '24 00:03 mighty1231