piotrmurach / tty-command

Execute shell commands with pretty output logging and capture stdout, stderr and exit status.
https://ttytoolkit.org
MIT License
400 stars 34 forks source link

Throwing exceptions if command fails by default #7

Closed piotrmurach closed 8 years ago

piotrmurach commented 8 years ago

I think that commands should throw exceptions if they fail by default. This is one of the biggest design flaws in bash, and I can't tell you the number of problems it's caused me (and people who work with me) over the past decade. When I write scripts I almost always want to halt on failure. I recognize that if we make this change we can't use execute!, sadly.

piotrmurach commented 8 years ago

I think designing api we need to be careful assuming too much. I completely understand the deficiency of bash but also believe in providing the means for handling the rough edges to the user. Raising error is not always the best way to handle application logic flow. I can easily see circumstance where a person will want to examine exit status and then decide whether raising error is what's needed or maybe simply aborting script with some useful debug info. I find raising errors for a generic library too rigid. Hence, I would still think of having dangerous version of the command running. Best practice can be explained in readme and ways to handle it?

gurgeous commented 8 years ago

Please? Pretty please? I really think that commands should throw by default. I've never seen a situation where someone wrote a script and they were excited when commands would fail silently. I've worked on many projects like this with many people at many companies, including ops, back end, and front end...

I just looked in the bin/ directory for my previous company. There are 222 scripts. Of those, 12 of them explicitly allowed failure.

If we make this the default we can have all kinds of great syntax for allowing failure if desired... The API we used was like this:

run(cmd, args = nil) succeeds?(cmd, args = nil) fails?(cmd, args = nil)

piotrmurach commented 8 years ago

Hehe. By no means I'm belittling the seriousness of silent failures. My perspective takes on the consumer view of a script. If, lets say, I'm executing some binary e.i. bla and then get TTY::Command::FailedError I may be less than impressed. I would rather library author do:

result = cmd.execute("dangerous stuff")
if result.failure?
  cmd.execute("echo 'ohh dear this failure means .....'")
  abort( specific code to my needs that means something )
end

The TTY::Command::Result allows you to query few execution properties, one of them being the status. I think my inspiration comes directly from Go language where error condition is just value that you either handle or ignore.

piotrmurach commented 8 years ago

More terse syntax would be

if cmd.execute("dangerous stuff").failure?
....
end

which kind of mimics bash test command.

piotrmurach commented 8 years ago

Hi Adam, it took me a while to mule over this but I agree with you that execute should throw by default hence the change.

gurgeous commented 8 years ago

Hey, this is great! I approve. Your users will thank you, trust me. :)