set -e silently does nothing inside a function called from an if, and I only found out in production

[u/ClickOk5811]

I had a cleanup script that always started with the usual set -euo pipefail. One day it reported success while half the work hadn’t happened. Here’s a stripped-down version of what bit me:

#!/usr/bin/env bash
set -euo pipefail

cleanup_old() {
  rm "$1/old.log"        # fails if the file is missing
  echo "cleaned $1"
}

if cleanup_old /tmp/nope; then
  echo "all good"
fi

Output:

rm: cannot remove '/tmp/nope/old.log': No such file or directory
cleaned /tmp/nope
all good

The rm fails, but the function keeps going, and the echo at the end gives it a zero exit status. The bash manual says -e is ignored for any command in a context where the exit status is being tested (if, while, &&/|| lists, !). That also applies to everything inside a function called from such a context. set -e at the top of the file doesn’t help, and nothing warns you.

What I do now:

cleanup_old() {
  rm "$1/old.log" || return 1
  echo "cleaned $1"
}

The explicit || return is noisier, but it works regardless of how the function gets called. For anything where partial success is dangerous, I also stopped using functions as if conditions and run them as plain statements, checking $? or using || on the call site deliberately.

Related one that bit me the same week: local out=$(some_cmd) always returns the exit status of local, not some_cmd. Splitting it into local out; out=$(some_cmd) fixes it.

I’ve been reading a lot of generated and copy-pasted scripts lately, and nearly all of them treat set -euo pipefail as a safety net. After this, I think of it as a default that works in the simple cases and quietly stops working in exactly the places where functions get composed.

For those who write larger bash scripts: do you rely on set -e at all, or do you handle errors explicitly everywhere and keep -e only as a backstop?

[u/AutoModerator]

Don't blindly use set -euo pipefail.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[u/AutoMativeX]

When you do not explicitly set a function's return code, it will return `$?` from the last call that occurred within it. You can simplify the snippet by moving your echo to the `if` itself:

cleanup_old() {
  rm "$1/old.log" >/dev/null
}

if cleanup_old /tmp/nope; then
  echo "cleaned /tmp/nope"
fi

The condition will function how you'd expect now. 😉

[u/stinkybass]

https://www.gnu.org/software/bash/manual/html_node/Conditional-Constructs.html

[u/zeekar]

Your "fix" doesn't seem to help much?

The function is still called from an if. And the if has no else clause. The only thing that changes if it goes wrong is that it doesn't say "cleaned $1" or "all good".

[u/bikes-n-math]

Personally, I almost never use set -e. Instead, I tend to check for failures after every command and print explicit messages to stderr. It gets tedious at times, but most common commands can be wrapped in a function to simplify things up.

[u/No-Razzmatazz7197 → u/bikes-n-math]

yeah the difference between a bash script and a bash script that always works is explicit error handling in damn near every line

[u/cupcakeheavy → u/No-Razzmatazz7197]

this is why i love go

[u/Atomfried_Ungemach]

The only case I use set -e is together with the -x option (display all commands and arguments while running the script) is while debugging to see where the script exits prematurely. Other than that I find it easier to handle commands that I expect to fail or variables remaining empty in certain cases, with conditional tests against files, variables and strings and the boolean '!', unless I explicitly need the exit code of a certain command or pipe. In those test conditionals you can handle reporting error messages to the user or to a log file and put alternative commands or logic to let the script finish with the desired outcome. Best is to not let commands fail in the first place by making sure that they receive the proper data to run and if not, to let the script do something else.

https://devhints.io/bash

[u/SamirTheGreat]

Usually when removing files safer less prone to error option I like to use is

'''
if [ -f "/path/to/file" ]; then
echo "File exists"
else
echo "File does not exist"
fi
'''

[u/HommeMusical]

Big upvote for this one, but to be honest, if I need any error handling above the most basic, I switch into Python, because bitter experience has shown me how easy it is to drop an error on the floor in bash.

[u/Castafolt]

Also took me a while to notice this behavior! Since then, I tried to not create function that return the error code directly because it encourages the use of 'if function'. Instead, I return the error code in a global variable. Reasoning here :

https://jcaillon.github.io/valet/docs/bash-best-practices/#errexit-bash-option-and-err-trap-in-bash

[u/Kreout]

One trap in the last paragraph: putting the call in cleanup_old ... || handle_error also disables errexit inside cleanup_old. Moving it out of if is not enough if it remains on the left of ||. The explicit guard inside the function is what makes it safe in both calling contexts. I would use rm -- "$1/old.log" || return "$?" so the function preserves the actual failure status, and let the caller decide whether to log it or exit.

[u/a-dot-in-the-sky]

As what others suggested
- check first if directory exist
- even though is quite obvious that it will be there just a protection that the script will not renegade lol
if directory not exist exit the script or stop the execution

- check if file exist (if file does not exist, stop the script)
- if the file exist
- create the command with the variable from the directory that has been verified
combined with another variable that verified that the file exist
- supply it to whatever action you want to
- best if before executing the command, log the whole command somewhere with date and time
- at least in this way, you also have an execution record of the script

[u/wroteit_reddit]

I'm not a bash expert, so I don't understand why you are providing just the directory and not the full file path ... That $1/old.log seems odd to me..

[u/Atomfried_Ungemach]

When you run a function, it always will return 0 (regardless if the code inside runs successful or not) unless you put a condition inside to return another value if something inside fails. You did that with 'II' which will return 1 when 'rm' fails, but it will never the less echo "cleaned $1" because it's not part of the condition.

Edit: 'return 1' will transmit (indirectly) that 'rm' failed when the function is run, but 'echo' gets executed non the less. Still in my opinion not a good use for a function in Bash when simpler methods for error testing exist.

Why not simply run 'rm' only if the file you want to erase is detected?

if [[ -f "$1/old.log" ]]; then
  rm -f "$1/old.log"
  echo "Cleaned $1, all good."
else
  echo "Nothing found to clean."
fi

That way you avoid running 'rm' for nothing and get an actual status whether the file is still there or not. If you do tests like these, you also don't need this daft 'set -euo pipefail' (well, pipefail is useful though)

[u/sector2000 → u/Atomfried_Ungemach]

That is not true. The function will return the exit code of the last command executed within the function:

$ cat test.sh
#!/usr/bin/env bash

test_function() {
  ls /nonexstingpath
}

test_function
echo "test function returned $?"
echo "script completed"

$ ./test.sh
ls: cannot access '/nonexstingpath': No such file or directory
test function returned 2
script completed

[u/Atomfried_Ungemach → u/sector2000]

You're right. Got that wrong in my memory because I do error handling mainly with test-statements so functions shouldn't fail in my scripts. Still, making a function and running rm against something that may be not there makes no real sense in OP's case imo.