I've made a simple bash script to check if a web server responds, and to send emails to a list of addresses if the website is down. Any suggestions as to how to improve it/ edge cases that I missed/ better styling/ etc? I'm not very familiar with idiomatic bash.
#!/bin/sh
output=$(wget http://lon3213:8111 2>&1)
pattern="connected"
path="/remote/users/my_user/projects/cmb_pinger/"
# File where I check and write the status of the server
# so I don't end up sending more e-mails for the same
# downtime. It writes 0 if no email has been sent yet
# and 1 if the email has been already sent.
log="check"
emails="[email protected],[email protected]"
# Create file if it doesn't exist
if ! [[ -f "$path$log" ]]
then
echo "0" > "$path$log"
fi
# Website cannot be reached
if [[ ! "$output" =~ "$pattern" ]]
then
# Emails have not yet been sent
if [[ $(cat "$path$log") == "0" ]]; then
echo "$output" | mail -s "Dashboard is down" $emails
echo "1" >| "$path$log"
fi
else
echo "0" >| "$path$log"
fi
2 Answers 2
I am impressed with the consistency of your code. You have obviously put effort in to making sure it is well presented, etc. It makes it easy to read.
The first thing I notice is the she-bang line, you have #!/bin/sh
this should be #!/bin/bash
... right? The script uses Bash specific features that don't exist in classic /bin/sh
, such as [[
. Although it may appear to work with #!/bin/sh
in your system, chances are that your /bin/sh
is symlinked to /bin/bash
(typically in Linux). This is not something you can rely on (it will break spectacularly in Solaris, for example), so when using Bash-specific features, then you should use #!/bin/bash
in the she-bang.
Then, there are some shortcuts you are missing though, and these are standard processes available on most Unix commands.... wget has an exit code! Use it.
wget
saves the web content to a file in the local directory. It will not override files, so you may end up with many copies of the same content. I recommend redirecting the URL contents to the stdout
by using -O -
argument for wget.
Error handling is a critical part of any script. It's not always easy in bash, but it can be done.
You have used the redirection >|
in a few places. This caught me by surprise. I admit I looked it up. In my experience the noclobber setting is seldom set, but that may just be for me. That's just an observation. Your site may be set up differently.
Finally, bash has function support, which allows you simplify logic substantially
Consider the following:
#!/bin/bash
url="http://lon3213:8111"
path="/tmp/cmb_pinger/"
# File where I check and write the status of the server
# so I don't end up sending more e-mails for the same
# downtime. It writes 0 if no email has been sent yet
# and 1 if the email has been already sent.
log="check"
emails="[email protected],[email protected]"
error() {
local message="$@"
>&2 echo $message
exit 1
}
setFlag() {
local flag=1ドル
echo $flag >| "$path$log" || error "Unable to set flag to $flag"
}
# Create file if it doesn't exist
[[ -d "$path" ]] || mkdir -p "$path" || error "Unable to create directory $path"
[[ -f "$path$log" ]] || setFlag 0
# Website cannot be reached
output=$(wget -O - $url 2>&1)
success=$?
if [[ $success -eq 0 ]]
then
setFlag $success
else
# Emails have not yet been sent
if [[ $(cat "$path$log") == "0" ]]; then
echo $output | mail -s "Dashboard is down" $emails || error "Unable to send mail to $emails"
setFlag $success
fi
fi
It's good to quote things to avoid unintended glob expansions or paths with spaces, but when you are typing literal strings that don't contain anything special, then it's a bit overkill. I would drop the double-quotes in all these examples:
pattern="connected" path="/remote/users/my_user/projects/cmb_pinger/" log="check" echo "0"
To give a counter-example too, this on the other hand is good as it is, the right thing to do:
if ! [[ -f "$path$log" ]]
One tip though,
in my experience it's often more ergonomic to not include trailing slashes in path names, and put the /
later when concatenating.
So like this:
path=/remote/users/my_user/projects/cmb_pinger
log=check
if ! [[ -f "$path/$log" ]]
then
# ...
The reason I like this better is because the directory boundary between $path
and $log
is now clear, and I don't need to worry if $path
had a trailing slash or not, the script will work either way.
It's a minor thing though, I've tried both ways many times,
and this way has proven to be more practical for me.
Finally, the biggest issue with the script,
is I think the storing and checking of 0 and 1 values in the log file.
It's awkward to cat
a file to compare its content to a simple value,
and I think you can replace this logic by a check on whether the file exists or not,
like this:
markerfile="$path/$log"
# Website cannot be reached
if [[ ! "$output" =~ "$pattern" ]]
then
# Emails have not yet been sent
if [[ ! -f "$markerfile" ]]; then
echo "$output" | mail -s "Dashboard is down" $emails
touch "$markerfile"
fi
else
rm -f "$markerfile"
fi
I also replaced "$path$log"
used in multiple places with "$markerfile"
,
which eliminates some duplicated logic.