Xonsh returns zero exit code when a command fails in a script

[certik]

I haven't had time to triage it yet, but here is what I executed:

$ xonsh ci/build.xsh && echo "OK"
...
Interactive Fortran. Experimental prototype, not ready for end users.
  * Use Ctrl-D to exit
  * Use Enter to submit
Try: function f(); f = 42; end function
>>> Input:
fxunction f(); f = 42; end function
input:1:11 syntax error: token type 'identifier' is unexpected here
fxunction f(); f = 42; end function
          ^
terminate called after throwing an instance of 'LFortran::ParserError'
  what():  syntax error
Aborted (core dumped)
OK

The C++ code exited with an uncaught exception which generates a non-zero exit code as can be verified by:

$ ./lfortran < ../../../src/bin/example_input.txt  && echo "OK"
Interactive Fortran. Experimental prototype, not ready for end users.
  * Use Ctrl-D to exit
  * Use Enter to submit
Try: function f(); f = 42; end function
>>> Input:
fxunction f(); f = 42; end function
input:1:11 syntax error: token type 'identifier' is unexpected here
fxunction f(); f = 42; end function
          ^
terminate called after throwing an instance of 'LFortran::ParserError'
  what():  syntax error
Aborted (core dumped)
$

And yet Xonsh returns zero exit code and the "OK" gets printed.

This is a serious issue as I use Xonsh on a CI and I need it to fail when an exception occurs in my C++ program.

Here is the ci/build.xsh (as you can see I enabled the equivalent of set -e):

#!/usr/bin/env xonsh

$RAISE_SUBPROC_ERROR = True
trace on

$lfortran_version=$(cat version).strip()
cd lfortran-$lfortran_version

cd test-bld
if $WIN == "1":
    ./src/lfortran/tests/Release/test_llvm -s
else:
    ./src/lfortran/tests/test_llvm -s
ctest --output-on-failure
src/bin/lfortran < ../src/bin/example_input.txt
cd ..

[gforsyth]

Closed via #3307

[certik]

#3307 couldn't have possibly fixed this problem. I can't reopen this issue, but the bug is most probably still there.

[gforsyth]

Whoops. Thanks @certik! I was confusing this with #3292

[certik]

@gforsyth no worries! I figured that was the case.

[scopatz]

Hi @certik - I can't for the life of me reproduce this with a simpler example. Consider the following bad.xsh scripts:

#!/usr/bin/env xonsh

$RAISE_SUBPROC_ERROR = True

aliases['bad'] = lambda: 42
![bad]

print("You can't see me")

or

#!/usr/bin/env xonsh

$RAISE_SUBPROC_ERROR = True

bash -c "exit 42"

print("You can't see me")

or

#!/usr/bin/env xonsh

$RAISE_SUBPROC_ERROR = True

python -c "import sys; sys.exit(42)"

print("You can't see me")

---

At the command line, all of these produce:

scopatz@artemis ~/temp $ xonsh bad.xsh && echo OK
Traceback (most recent call last):
  File "/home/scopatz/.local/bin/xonsh", line 4, in <module>
    main()
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 24011, in main
    _failback_to_other_shells(args, err)
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 23975, in _failback_to_other_shells
    raise err
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 24009, in main
    return main_xonsh(args)
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 24053, in main_xonsh
    args.file, shell.execer, glb=shell.ctx, loc=None, mode="exec"
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 3038, in run_script_with_cache
    run_compiled_code(ccode, glb, loc, mode)
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 2943, in run_compiled_code
    func(code, glb, loc)
  File "bad.xsh", line 9, in <module>
    python -c "import sys; sys.exit(42)"
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 22466, in subproc_captured_hiddenobject
    return run_subproc(cmds, captured="hiddenobject")
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 22429, in run_subproc
    command.end()
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 15671, in end
    self._end(tee_output=tee_output)
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 15690, in _end
    self._raise_subproc_error()
  File "/home/scopatz/.local/lib/python3.7/site-packages/xonsh/__amalgam__.py", line 15816, in _raise_subproc_error
    raise subprocess.CalledProcessError(rtn, spec.args, output=self.output)
subprocess.CalledProcessError: Command '['python', '-c', 'import sys; sys.exit(42)']' returned non-zero exit status 42.
scopatz@artemis ~/temp $ _.returncode
1

Are you sure the lfortran actually has the proper returncode?

[certik]

Here is how to reproduce this very important bug. Create this file a.cpp:

#include <iostream>

int main() {
    throw std::runtime_error("xx");
    return 0;
}

Compile using:

g++ a.cpp

And test that it returns a non-zero exit code:

$ ./a.out && echo "OK"
terminate called after throwing an instance of 'std::runtime_error'
  what():  xx
Aborted (core dumped)
$

Now call this as part of a Xonsh script:

#!/usr/bin/env xonsh

$RAISE_SUBPROC_ERROR = True
trace on

./a.out

as follows:

$ xonsh build.xsh && echo "OK"
build.xsh:6:./a.out
terminate called after throwing an instance of 'std::runtime_error'
  what():  xx
Aborted (core dumped)
OK
$

As you can see, Xonsh returns a zero exit value.

[scopatz]

@certik - we figured this out, right? but did a fix go in?

[certik]

I don't think a fix went in.

…

On Sat, Feb 29, 2020, at 5:17 PM, Anthony Scopatz wrote: @certik <https://github.com/certik> - we figured this out, right? but did a fix go in? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3309?email_source=notifications&email_token=AAAFAWD5CVXSQQ3CMVEBJVLRFGSQ5A5CNFSM4IVBEIRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENMKWPA#issuecomment-593013564>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWHTRRDCIVEYNYCX23TRFGSQ5ANCNFSM4IVBEIRA>.

[scopatz]

I found the fix on a branch! Just need to add tests

[scopatz]

Potential fix in #3465