From 771ef54e22d9e29b9b78282a006ce2de24fc84d9 Mon Sep 17 00:00:00 2001 From: Damien Bezborodov Date: Thu, 5 Nov 2015 00:33:18 +1030 Subject: [PATCH 1/5] Commands that fail should return a non-zero exit status. This resolves #10. - Raise an exception when a system command fails with a non-zero exception. (This could be a custom exception.) - bin/karo should return a non-zero exit status when an exception is rescued. - Error messages should be printed on STDERR, not STDOUT. - Inform the user when a system interrupt is triggered on STDERR; this emulates the behaviour of most other UNIX commands. --- bin/karo | 5 +++-- lib/karo/common.rb | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/bin/karo b/bin/karo index d5c9067..b5350c3 100755 --- a/bin/karo +++ b/bin/karo @@ -5,7 +5,8 @@ require 'karo' begin Karo::CLI.start(ARGV) rescue SystemExit, Interrupt - puts "Exiting" + STDERR.puts "Exiting" rescue Exception => e - puts e + STDERR.puts "karo: Error: #{e}" + return 1 end diff --git a/lib/karo/common.rb b/lib/karo/common.rb index 5bc97d8..3966455 100644 --- a/lib/karo/common.rb +++ b/lib/karo/common.rb @@ -22,6 +22,9 @@ def make_command(configuration, namespace, command, extras) def run_it(cmd, verbose=false) say cmd, :green if verbose system cmd unless options[:dryrun] + if $?.exitstatus + raise "Non-zero exit code (#{$?.exitstatus}) returned from #{cmd.strip}" + end end def git_repo From 10e966141eedf6ee62fcf0ab633c7ec2efddf651 Mon Sep 17 00:00:00 2001 From: br3nt Date: Wed, 19 Apr 2017 14:16:25 +0930 Subject: [PATCH 2/5] Fix for `unexpected return (LocalJumpError)` error Was receiving an error: ``` bin/karo:11:in `rescue in ': unexpected return (LocalJumpError) ``` Have changed `return 1` to `exit 1`. The purpose of this line is to exit the program with a non-zero exit status if an unhandled exception is encountered. --- bin/karo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/karo b/bin/karo index b5350c3..1c01973 100755 --- a/bin/karo +++ b/bin/karo @@ -8,5 +8,5 @@ rescue SystemExit, Interrupt STDERR.puts "Exiting" rescue Exception => e STDERR.puts "karo: Error: #{e}" - return 1 + exit 1 end From 7f45a5c933733622becf323a5e0e414db6bc5990 Mon Sep 17 00:00:00 2001 From: br3nt Date: Wed, 19 Apr 2017 14:22:03 +0930 Subject: [PATCH 3/5] Removed rescue of `Exception` This change means the `rescue` is now only for `StandardError` and its subclasses. For more info about why this change is necessary, see: [This SO answer][1] [1]: http://stackoverflow.com/a/10048406/848668 --- bin/karo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/karo b/bin/karo index 1c01973..14fcb40 100755 --- a/bin/karo +++ b/bin/karo @@ -6,7 +6,7 @@ begin Karo::CLI.start(ARGV) rescue SystemExit, Interrupt STDERR.puts "Exiting" -rescue Exception => e +rescue => e STDERR.puts "karo: Error: #{e}" exit 1 end From c4660823ecfce433f3e66b6959b59fdbbf62a44b Mon Sep 17 00:00:00 2001 From: br3nt Date: Wed, 19 Apr 2017 14:29:40 +0930 Subject: [PATCH 4/5] Add backtrace to STDERR for unhandled exceptions The backtrace has been added to the STDERR output so that the user can identify any issues that may have arisen in the karo code base. A suggested improvement would be to create a custom `KaroException` error class that can be used to handle any known exception cases within the karo codebase. --- bin/karo | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/karo b/bin/karo index 14fcb40..e602a91 100755 --- a/bin/karo +++ b/bin/karo @@ -8,5 +8,6 @@ rescue SystemExit, Interrupt STDERR.puts "Exiting" rescue => e STDERR.puts "karo: Error: #{e}" + STDERR.puts e.backtrace exit 1 end From a50bac6eb11e8dbdbe7608b247c6e0b94ee78b9e Mon Sep 17 00:00:00 2001 From: br3nt Date: Wed, 19 Apr 2017 14:38:27 +0930 Subject: [PATCH 5/5] Test for non-zero exit status after running cmd After running a command, `$?.exitstatus` will be set to the exit status of the last executed process. If the process finishes without errors,`$?.exitstatus` will be `0`. For all other values, we will raise an exception. --- lib/karo/common.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/karo/common.rb b/lib/karo/common.rb index 3966455..65a3463 100644 --- a/lib/karo/common.rb +++ b/lib/karo/common.rb @@ -22,7 +22,7 @@ def make_command(configuration, namespace, command, extras) def run_it(cmd, verbose=false) say cmd, :green if verbose system cmd unless options[:dryrun] - if $?.exitstatus + if $?.exitstatus != 0 raise "Non-zero exit code (#{$?.exitstatus}) returned from #{cmd.strip}" end end