oils-for-unix / oils

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.83k stars 155 forks source link

Write "the ultimate guide to errexit" #709

Closed andychu closed 2 years ago

andychu commented 4 years ago

This is going to take a lot of time, but I think it should be done because a lot of these threads are like "groundhog day"

https://lobste.rs/s/ajoaje/first_two_statements_your_bash_script

e.g. discovering -e for the first time, then seeing all the problems in the Wooledge Wiki, etc.

https://mywiki.wooledge.org/BashFAQ/105

Threads:

https://oilshell.zulipchat.com/#narrow/stream/121540-oil-discuss/topic/Things.20that.20surprised.20me.20about.20shell

https://oilshell.zulipchat.com/#narrow/stream/121540-oil-discuss/topic/more.20errexit.20links

related: #476

andychu commented 4 years ago

Off the top of my head, here are the problems:

  1. The confusing rule that errexit is disabled with if, || && ! (it's in POSIX and all shells implement it)
    • strict_errexit in Oil disables some of this, but needs to be tweaked #476
  2. The local x=$(false) vs. x=$(false) problem
    • more_errexit in Oil fixes this
  3. The bash specific command sub problem. x=$(false). I think inherit_errexit fixes this.

Related: set -o pipefail, SIGPIPE, etc.

andychu commented 4 years ago

A very related problem is that the $? exit status is pretty confusing itself.

e.g. for command subs and process subs.


Repeating crazily comprehensive links from Zulip:

https://www.fvue.nl/wiki/Bash:_Error_handling

https://www.in-ulm.de/~mascheck/various/set-e/

andychu commented 4 years ago

474 is also very related.

Crestwave commented 4 years ago

I'm not sure if this is really problematic, but one interesting gotcha I encountered today was something to the effect of:

set -e
f() { (( var )) && echo foo; }
f
echo bar

This doesn't print anything if var evaluates to 0 because even though errexit was disabled for (( var )), it defined the function's return code since it was the last command executed. And since it is enabled in the caller, it then aborts.

o11c commented 4 years ago

Note also that set -E (errtrace) is still completely unknown to osh. This forced me to add some || is-oil to get my script to run at all (and unsafely at that).

Whereas trap 'blah' ERR is just a warning.

andychu commented 4 years ago

OK I haven't used errtrace, but it sounds like it's in scope. Do you have an example of your usage?

Generally speaking Oil development is very test driven, and I accept PRs with failing spec tests if they pass with other shells (including bash).

Example for ble.sh features:

https://www.oilshell.org/release/0.8.pre4/test/spec.wwz/ble-idioms.html

Either way some examples would help. I generally prioritize features based on "real" usage, since there is a deep well of bash features to go through...

andychu commented 4 years ago

@Crestwave

Sorry for the late response. I think there are two problems there, and I think we should address them. I mentioned (( )) here, and the && problem is well known.

https://oilshell.zulipchat.com/#narrow/stream/121540-oil-discuss/topic/Ultimate.20errexit.20guide/near/194360428

The exit code of (( )) is problematic and I'm not sure it can be fixed... It is another case of confusing booleans and success/fail, e.g. (( a = 0 )) vs. if (( a == b )).

Oil has a different expression language that doesn't have that confusion and has more types besides integers. But I think && will be fixed by unconditionally causing a fatal runtime error when it's confusing, so maybe that will include (( )) on the left.

Crestwave commented 4 years ago

The main thing I found interesting was that it exited even though && disabled it because it defined the function's return code. Which makes sense given the mechanics of errexit, but I just didn't consider it before.

andychu commented 4 years ago

Yeah I have run into this before and been confused, and it's mentioned on Wooledge's wiki. It happens on code as simple as:

set -e

f()  {
  test -f foo && echo "foo exists"
}

f   # fails here!
echo "I expected to get here, but no"

That is better written:

set -e

f() {
  test -f foo
  echo "foo exists"
}

Or use if test -f foo.

Basically && is useless and confusing when set -e is on. So I think that shopt -s strict_errexit should simply disallow && that's not in a condition?

if test -f foo && test -f bar; then ...     # allowed

test -f foo && ...  # not allowed
andychu commented 4 years ago

FWIW I changed my style to always use if statements because of this (because I always have set -e on.)

I don't use the test x && foo idiom at all. But we shouldn't have that "footgun" there waiting for other people to step on it...

o11c commented 4 years ago

Frankly, I don't understand how my code works anymore. I do know that:

My startup code looks like:

set -e
set -E

trap 'echo "$BASH_SOURCE:$LINENO: error: failure during early startup! Details unavailable."' ERR

magic_exitvalue=$(($(kill -l CONT)+128))

backtrace()
{
    {
        local status=$?
        if [ "$status" -eq "$magic_exitvalue" ]
        then
            echo '(omit backtrace)'
            exit "$magic_exitvalue"
        fi
        local max file line func argc argvi i j
        echo
        echo 'Panic! Something failed unexpectedly.' "(status $status)"
        echo 'While executing' "$BASH_COMMAND"
        echo
        echo Backtrace:
        echo
        max=${#BASH_LINENO[@]}
        let max-- # The top-most frame is "special".
        argvi=${BASH_ARGC[0]}
        for ((i=1;i<max;++i))
        do
            file=${BASH_SOURCE[i]}
            line=${BASH_LINENO[i-1]}
            func=${FUNCNAME[i]}
            argc=${BASH_ARGC[i]}
            printf '%s:%d: ... in %q' "$file" "$line" "$func"
            # BASH_ARGV: ... bar foo ...
            # argvi          ^
            # argvi+argc             ^
            for ((j=argc-1; j>=0; --j))
            do
                printf ' %q' ${BASH_ARGV[argvi+j]}
            done
            let argvi+=argc || true
            printf '\n'
        done

        if true
        then
            file=${BASH_SOURCE[i]}
            line=${BASH_LINENO[i-1]}
            printf '%s:%d: ... at top level\n' "$file" "$line"
        fi
    } >&2
    exit "$magic_exitvalue"
    unreachable
}
shopt -s extdebug
trap 'backtrace' ERR

# main code, involving many functions, follows.
andychu commented 4 years ago

OK thanks, this looks a lot like #708 . It's probably in scope but I don't know all the details of how it works either....

andychu commented 4 years ago

Comment listing all the problems concisely: https://news.ycombinator.com/item?id=24740842

This issue is still very important! I have some ideas for it -- I think we need a builtin called catch for error checking functions. (Formerly called invoke).

andychu commented 4 years ago

Quote from HN:

Wait until you realize

  set -e; (false && true); echo hi

does nothing, but

  set -e;  false && true ; echo hi

prints hi.

This is the same problem as #2, the f() { test -d /tmp && echo "exists" } problem, except with subshells, not with functions.

Good point! This is not solved by catch / invoke... So we still need a solution for this. I thought we would discourage && because it's not necessary.

However something like if test -d / && test -d /tmp still makes sense

andychu commented 4 years ago

Refreshing my memory on what we have:

  1. inherit_errexit from bash 4.4. Oil also implements it.

  2. Oil's more_errexit to fix the local problem

  3. Oil's strict_errexit to detect if myfunc or ! myfunc dynamically, and force you to write it as if catch myfunc or ! catch myfunc.

Although I wasn't entirely happy with the dynamic detection here. It still needs another iteration. Problem: one line functions could still be allowed?

The catch (invoke) builtin complements this, but isn't implemented yet. (Note that it can't be implemented as a shell function!)

Now what's the solution to the "&& at the end of function" problem?

I think the best answer so far was that when strict_errexit is on, it will disallow

foo && bar

You can just write it

foo
bar

But it will allow:

if foo && bar {
  echo boolean context
}

I think that makes sense, but we'll have to test it out more to be sure.

Comments appreciated!

andychu commented 4 years ago

Important example:

https://news.ycombinator.com/edit?id=24744146

(edit: removed incorrect code)

andychu commented 3 years ago

OK I think everything here is settled, but we just need to document it ...

andychu commented 2 years ago

This is finally done !

https://www.oilshell.org/release/0.10.0/doc/error-handling.html --- all pitfalls described here!

https://www.oilshell.org/blog/2022/05/release-0.10.0.html