To address issue 149,150 and 155 for simple deploy, Capistrano error handling

[keviny22]

This change addresses issue https://github.com/intuit/simple_deploy/issues/150

Prior to this change a user wasn't given feedback if the command executed successfully. If command passed was valid or invalid, the response was the same.

After the code change if a valid command was executed, an info message is displayed to stdout, if an invalid command was executed, an error message is displayed to stdout.

Prior to code change (invalid command):

$ simple_deploy execute -e default -n kyoung-testapp08 -c uptme
2013-02-13 15:51:00 -0800 INFO : Setting command: 'uptme'.
$ 

After code change (valid command):

$ simple_deploy execute -e default -n kyoung-testapp08 -c uptime
2013-02-13 15:51:53 -0800 INFO : Setting command: 'uptime'.
2013-02-13 15:51:55 -0800 INFO : Command executed against instance(s) successfully.
$ 

After code change (invalid command):

$ simple_deploy execute -e default -n kyoung-testapp08 -c uptme
2013-02-13 15:55:13 -0800 INFO : Setting command: 'uptme'.
2013-02-13 15:55:14 -0800 ERROR : Command executed against instances unsuccessfully.

[keviny22]

I added trapping for stderr rather then the naked rescue.

I did run into an issue setting the exit inside the rescue block, and moving the success logger command outside the begin block: when executed with an incorrect command, it exposes the NoInstances exception bug....

Keeping the else clause avoids exposing this bug by letting program exit naturally. I personally don't like using exit statements inside blocks, but it may be more of a preferred Ruby way of doing things.... let me know if you still want to use the forced exit.

$ simple_deploy execute -e default -n kyoung-testapp08 -c uptme
2013-02-14 10:06:55 -0800 INFO : Setting command: 'uptme'.
2013-02-14 10:06:58 -0800 ERROR : Error running execute statement: failed: "sh -c 'uptme'" on 54.241.194.63,54.241.102.88

Updated suggested code.......

        begin
          @task.execute
        rescue StandardError => bang
          @logger.error "Error running execute statement: #{bang}"
        else
          @logger.info "Command executed against instances successfully."
        end

[weavenet]

You'll need to exit with a code 1 somewhere when the command fails.

I can understand your concern about exiting in different parts of a program. The current setup is not ideal. In an ideal world, we would have a method called something like log_and_exit that we would call throughout the program which would provide that one way to exit.

Right now, we make sure all exits happen within the cli level classes, not deep in the program. For example:

https://github.com/intuit/simple_deploy/blob/master/lib/simple_deploy/cli/execute.rb

[keviny22]

Sample run:

[kyoung@localhost simple_deploy]$ simple_deploy execute -e default -n kyoung-testapp08 -c uptime
2013-02-14 13:37:19 -0800 INFO : Setting command: 'uptime'.
2013-02-14 13:37:21 -0800 INFO : Command executed against instances successfully.

[kyoung@localhost simple_deploy]$ simple_deploy execute -e default -n kyoung-testapp08 -c uptme
2013-02-14 13:37:31 -0800 INFO : Setting command: 'uptme'.
2013-02-14 13:37:32 -0800 ERROR : Error running execute statement: failed: "sh -c 'uptme'" on 54.241.194.63,54.241.102.88
/home/kyoung/.rvm/gems/ruby-1.9.3-p374/gems/simple_deploy-0.6.6/lib/simple_deploy/cli/execute.rb:55:in `rescue in block in execute': uninitialized constant SimpleDeploy::Exceptions::NoInstances (NameError)
    from /home/kyoung/.rvm/gems/ruby-1.9.3-p374/gems/simple_deploy-0.6.6/lib/simple_deploy/cli/execute.rb:52:in `block in execute'
    from /home/kyoung/.rvm/gems/ruby-1.9.3-p374/gems/simple_deploy-0.6.6/lib/simple_deploy/cli/execute.rb:42:in `each'
    from /home/kyoung/.rvm/gems/ruby-1.9.3-p374/gems/simple_deploy-0.6.6/lib/simple_deploy/cli/execute.rb:42:in `execute'
    from /home/kyoung/.rvm/gems/ruby-1.9.3-p374/gems/simple_deploy-0.6.6/lib/simple_deploy/cli.rb:45:in `start'
    from /home/kyoung/.rvm/gems/ruby-1.9.3-p374/gems/simple_deploy-0.6.6/bin/simple_deploy:6:in `<top (required)>'
    from /home/kyoung/.rvm/gems/ruby-1.9.3-p374/bin/simple_deploy:23:in `load'
    from /home/kyoung/.rvm/gems/ruby-1.9.3-p374/bin/simple_deploy:23:in `<main>'
[kyoung@localhost simple_deploy]$ 

[weavenet]

This is an unrelated bug, to fix it, add the following to lib/simple_deploy/exceptions.rb under the Base class.

class Exceptions::NoInstances < Base
end

[keviny22]

This pull will also address issue https://github.com/intuit/simple_deploy/issues/155

[keviny22]

This pull will also address issue https://github.com/intuit/simple_deploy/issues/149 which brings up the issue with SSH Auth errors, since auth is being done through Capistrano I added a rescue to specifically address Capistrano connection errors as well as any other capistrano error.

        begin
          @task.execute

        rescue ::Capistrano::CommandError => error
          @logger.error "Error running execute statement: #{error}"
          return false
        rescue ::Capistrano::ConnectionError => error
          @logger.error "Error connecting to instances: #{error}"
          return false
        rescue ::Capistrano::Error => error
          @logger.error "Error: #{error}"
          return false
        end
        @logger.info "Command executed against instances successfully."

[weavenet]

This looks good, @thbishop can you do a final review?

[weavenet]

@thbishop before you merge, Kevin and I will add specs.

[thbishop]

@keviny22 is this ready?

[keviny22]

@thbishop yes it is ready for review.