Added support for :on_error => :continue in task definitions, allowing tasks to effectively ignore connection and execution errors that occur as they run

git-svn-id: http://svn.rubyonrails.org/rails/tools/capistrano@7145 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Added support for :on_error => :continue in task definitions, allowing tasks to effectively ignore connection and execution errors that occur as they run [Rob Holland]
+
 * Use correct parameters for Logger constructor in the SCM and Strategy base initializers [Jamis Buck]
 
 * Set LC_ALL=C before querying the revision, to make sure the output is in a predictable locale and can be parsed predictably [via Leandro Nunes dos Santos]

lib/capistrano/configuration/connections.rb

@@ -30,6 +30,18 @@ module Capistrano
       def initialize_with_connections(*args) #:nodoc:
         initialize_without_connections(*args)
         @sessions = {}
+        @failed_sessions = []
+      end
+
+      # Indicate that the given server could not be connected to.
+      def failed!(server)
+        @failed_sessions << server
+      end
+
+      # Query whether previous connection attempts to the given server have
+      # failed.
+      def has_failed?(server)
+        @failed_sessions.include?(server)
       end
 
       # Used to force connections to be made to the current task's servers.
@@ -56,8 +68,22 @@ module Capistrano
 
       # Ensures that there are active sessions for each server in the list.
       def establish_connections_to(servers)
-        threads = Array(servers).map { |server| establish_connection_to(server) }
+        failed_servers = []
+
+        # force the connection factory to be instantiated synchronously,
+        # otherwise we wind up with multiple gateway instances, because
+        # each connection is done in parallel.
+        connection_factory
+
+        threads = Array(servers).map { |server| establish_connection_to(server, failed_servers) }
         threads.each { |t| t.join }
+
+        if failed_servers.any?
+          errors = failed_servers.map { |h| "#{h[:server]} (#{h[:error].class}: #{h[:error].message})" }
+          error = ConnectionError.new("connection failed for: #{errors.join(', ')}")
+          error.hosts = failed_servers.map { |h| h[:server] }
+          raise error
+        end
       end
 
       # Determines the set of servers within the current task's scope and
@@ -72,6 +98,11 @@ module Capistrano
           if servers.empty?
             raise ScriptError, "`#{task.fully_qualified_name}' is only run for servers matching #{task.options.inspect}, but no servers matched"
           end
+
+          if task.continue_on_error?
+            servers.delete_if { |s| has_failed?(s) }
+            return if servers.empty?
+          end
         else
           servers = find_servers(options)
           raise ScriptError, "no servers found to match #{options.inspect}" if servers.empty?
@@ -81,8 +112,22 @@ module Capistrano
         logger.trace "servers: #{servers.map { |s| s.host }.inspect}"
 
         # establish connections to those servers, as necessary
-        establish_connections_to(servers)
-        yield servers
+        begin
+          establish_connections_to(servers)
+        rescue ConnectionError => error
+          raise error unless task.continue_on_error?
+          error.hosts.each do |h|
+            servers.delete(h)
+            failed!(h)
+          end
+        end
+
+        begin
+          yield servers
+        rescue RemoteError => error
+          raise error unless task.continue_on_error?
+          error.hosts.each { |h| failed!(h) }
+        end
       end
 
       private
@@ -90,12 +135,15 @@ module Capistrano
         # We establish the connection by creating a thread in a new method--this
         # prevents problems with the thread's scope seeing the wrong 'server'
         # variable if the thread just happens to take too long to start up.
-        def establish_connection_to(server)
-          # force the connection factory to be instantiated synchronously,
-          # otherwise we wind up with multiple gateway instances, because
-          # each connection is done in parallel.
-          connection_factory
-          Thread.new { sessions[server] ||= connection_factory.connect_to(server) }
+        def establish_connection_to(server, failures=nil)
+          Thread.new do
+            begin
+              sessions[server] ||= connection_factory.connect_to(server)
+            rescue Exception => err
+              raise unless failures
+              failures << { :server => server, :error => err }
+            end
+          end
         end
     end
   end

lib/capistrano/errors.rb

@@ -2,13 +2,13 @@ module Capistrano
   class Error < RuntimeError; end
 
   class CaptureError < Error; end
-  class ConnectionError < Error; end
   class NoSuchTaskError < Error; end
-
+  
   class RemoteError < Error
     attr_accessor :hosts
   end
 
+  class ConnectionError < RemoteError; end
   class UploadError < RemoteError; end
   class CommandError < RemoteError; end
 end

lib/capistrano/gateway.rb

@@ -104,7 +104,13 @@ module Capistrano
       end
 
       thread.join
-      connection or raise ConnectionError, "could not establish connection to `#{server}'"
+      if connection.nil?
+        error = ConnectionError.new("could not establish connection to `#{server}'")
+        error.hosts = [server]
+        raise error
+      end
+    
+      connection
     end
 
     private

lib/capistrano/task_definition.rb

@@ -57,5 +57,11 @@ module Capistrano
 
       brief
     end
+
+    # Indicates whether the task wants to continue, even if a server has failed
+    # previously
+    def continue_on_error?
+      options[:on_error] == :continue
+    end
   end
 end

test/cli/execute_test.rb

@@ -36,7 +36,8 @@ class CLIExecuteTest < Test::Unit::TestCase
 
   def test_execute_should_set_prevars_before_loading
     @config.expects(:load).never
-    @config.expects(:set).with(:stage, "foobar").returns(Proc.new { @config.expects(:load).with("standard") })
+    @config.expects(:set).with(:stage, "foobar")
+    @config.expects(:load).with("standard")
     @cli.options[:pre_vars] = { :stage => "foobar" }
     @cli.execute!
   end

test/command_test.rb

@@ -207,7 +207,7 @@ class CommandTest < Test::Unit::TestCase
     new_channel = Proc.new do |times|
       ch = mock("channel")
       returns = [false] * (times-1)
-      ch.stubs(:[]).with(:closed).returns(lambda { returns.empty? ? true : returns.pop })
+      ch.stubs(:[]).with(:closed).returns(*(returns + [true]))
       con = mock("connection")
       con.expects(:process).with(true).times(times-1)
       ch.expects(:connection).times(times-1).returns(con)
@@ -227,8 +227,14 @@ class CommandTest < Test::Unit::TestCase
 
     new_channel = Proc.new do
       ch = mock("channel")
-      ch.stubs(:[]).with(:closed).returns(lambda { Time.now - now < 1.1 ? false : true })
-      ch.stubs(:[]).with(:status).returns(0)
+      ch.stubs(:now => now)
+      def ch.[](key)
+        case key
+        when :status then 0
+        when :closed then Time.now - now < 1.1 ? false : true
+        else raise "unknown key: #{key}"
+        end
+      end
       con = mock("connection")
       con.stubs(:process)
       con.expects(:ping!)

test/configuration/callbacks_test.rb

@@ -198,10 +198,9 @@ class ConfigurationCallbacksTest < Test::Unit::TestCase
     @config.after("any:old:thing", :and_then_this, :lastly_this)
 
     [:first_this, :then_this, :and_then_this, :lastly_this].each do |t|
-      @config.expects(:find_and_execute_task).with(t).returns(Proc.new { @config.called << t })
+      @config.expects(:find_and_execute_task).with(t)
     end
 
     @config.execute_task(task)
-    assert_equal [before_task, :first_this, :then_this, task, :and_then_this, :lastly_this, after_task], @config.called
   end
 end

test/configuration/connections_test.rb

@@ -92,6 +92,36 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
     @config.establish_connections_to(%w(cap1 cap2 cap3).map { |s| server(s) })
     assert %w(cap1 cap2 cap3), @config.sessions.keys.sort.map { |s| s.host }
   end
+  
+  def test_establish_connections_to_should_raise_one_connection_error_on_failure
+    Capistrano::SSH.expects(:connect).times(2).raises(Exception)
+    assert_raises(Capistrano::ConnectionError) {
+      @config.establish_connections_to(%w(cap1 cap2)).map { |s| servers(s) }
+    }
+  end
+
+  def test_connection_error_should_include_accessor_with_host_array
+    Capistrano::SSH.expects(:connect).times(2).raises(Exception)
+
+    begin
+      @config.establish_connections_to(%w(cap1 cap2)).map { |s| servers(s) }      
+      flunk "expected an exception to be raised"
+    rescue Capistrano::ConnectionError => e
+      assert e.respond_to?(:hosts)
+      assert_equal %w(cap1 cap2), e.hosts.map { |h| h.to_s }
+    end
+  end
+  
+  def test_connection_error_should_only_include_failed_hosts
+    Capistrano::SSH.expects(:connect).times(2).raises(Exception).then.returns(:success)
+
+    begin
+      @config.establish_connections_to(%w(cap1 cap2)).map { |s| servers(s) }      
+      flunk "expected an exception to be raised"
+    rescue Capistrano::ConnectionError => e
+      assert_equal %w(cap1), e.hosts.map { |h| h.to_s }
+    end
+  end
 
   def test_execute_on_servers_should_require_a_block
     assert_raises(ArgumentError) { @config.execute_on_servers }
@@ -111,8 +141,8 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
     assert_raises(ScriptError) { @config.execute_on_servers(:a => :b, :c => :d) { |list| } }
   end
 
-  def test_execute_on_servers_should_raise_an_error_if_the_current_task_has_no_matching_servers
-    @config.current_task = stub("task", :fully_qualified_name => "name", :options => {})
+  def test_execute_on_servers_should_raise_an_error_if_the_current_task_has_no_matching_servers_by_default
+    @config.current_task = mock_task
     @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([])
     assert_raises(ScriptError) do
       @config.execute_on_servers do
@@ -120,10 +150,10 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
       end
     end
   end
-
+  
   def test_execute_on_servers_should_determine_server_list_from_active_task
     assert @config.sessions.empty?
-    @config.current_task = stub("task")
+    @config.current_task = mock_task
     @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([server("cap1"), server("cap2"), server("cap3")])
     Capistrano::SSH.expects(:connect).times(3).returns(:success)
     @config.execute_on_servers {}
@@ -132,7 +162,7 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
 
   def test_execute_on_servers_should_yield_server_list_to_block
     assert @config.sessions.empty?
-    @config.current_task = stub("task")
+    @config.current_task = mock_task
     @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([server("cap1"), server("cap2"), server("cap3")])
     Capistrano::SSH.expects(:connect).times(3).returns(:success)
     block_called = false
@@ -148,7 +178,7 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
 
   def test_execute_on_servers_with_once_option_should_establish_connection_to_and_yield_only_the_first_server
     assert @config.sessions.empty?
-    @config.current_task = stub("task")
+    @config.current_task = mock_task
     @config.expects(:find_servers_for_task).with(@config.current_task, :once => true).returns([server("cap1"), server("cap2"), server("cap3")])
     Capistrano::SSH.expects(:connect).returns(:success)
     block_called = false
@@ -159,10 +189,81 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
     assert block_called
     assert_equal %w(cap1), @config.sessions.keys.sort.map { |s| s.host }
   end
+  
+  def test_execute_servers_should_raise_connection_error_on_failure_by_default
+    @config.current_task = mock_task
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([server("cap1")])
+    Capistrano::SSH.expects(:connect).raises(Exception)
+    assert_raises(Capistrano::ConnectionError) {
+      @config.execute_on_servers do
+        flunk "expected an exception to be raised"
+      end
+    }
+  end
+  
+  def test_execute_servers_should_not_raise_connection_error_on_failure_with_on_errors_continue
+    @config.current_task = mock_task(:on_error => :continue)
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([server("cap1"), server("cap2")])
+    Capistrano::SSH.expects(:connect).times(2).raises(Exception).then.returns(:success)
+    assert_nothing_raised {
+      @config.execute_on_servers do |servers|
+        assert_equal %w(cap2), servers.map { |s| s.host }
+      end
+    }
+  end
+  
+  def test_execute_on_servers_should_not_try_to_connect_to_hosts_with_connection_errors_with_on_errors_continue
+    list = [server("cap1"), server("cap2")]
+    @config.current_task = mock_task(:on_error => :continue)
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns(list)
+    Capistrano::SSH.expects(:connect).times(2).raises(Exception).then.returns(:success)
+    @config.expects(:failed!).with(server("cap1"))
+    @config.execute_on_servers do |servers|
+      assert_equal %w(cap2), servers.map { |s| s.host }
+    end
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns(list)
+    @config.execute_on_servers do |servers|
+      assert_equal %w(cap2), servers.map { |s| s.host }
+    end
+  end
+  
+  def test_execute_on_servers_should_not_try_to_connect_to_hosts_with_command_errors_with_on_errors_continue
+    cap1 = server("cap1")
+    cap2 = server("cap2")
+    @config.current_task = mock_task(:on_error => :continue)
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([cap1, cap2])
+    Capistrano::SSH.expects(:connect).times(2).returns(:success)
+    @config.execute_on_servers do |servers|
+      error = Capistrano::CommandError.new
+      error.hosts = [cap1]
+      raise error
+    end
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([cap1, cap2])
+    @config.execute_on_servers do |servers|
+      assert_equal %w(cap2), servers.map { |s| s.host }
+    end
+  end
+  
+  def test_execute_on_servers_should_not_try_to_connect_to_hosts_with_upload_errors_with_on_errors_continue
+    cap1 = server("cap1")
+    cap2 = server("cap2")
+    @config.current_task = mock_task(:on_error => :continue)
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([cap1, cap2])
+    Capistrano::SSH.expects(:connect).times(2).returns(:success)
+    @config.execute_on_servers do |servers|
+      error = Capistrano::UploadError.new
+      error.hosts = [cap1]
+      raise error
+    end
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([cap1, cap2])
+    @config.execute_on_servers do |servers|
+      assert_equal %w(cap2), servers.map { |s| s.host }
+    end
+  end
 
   def test_connect_should_establish_connections_to_all_servers_in_scope
     assert @config.sessions.empty?
-    @config.current_task = stub("task")
+    @config.current_task = mock_task
     @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([server("cap1"), server("cap2"), server("cap3")])
     Capistrano::SSH.expects(:connect).times(3).returns(:success)
     @config.connect!
@@ -171,10 +272,17 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
 
   def test_connect_should_honor_once_option
     assert @config.sessions.empty?
-    @config.current_task = stub("task")
+    @config.current_task = mock_task
     @config.expects(:find_servers_for_task).with(@config.current_task, :once => true).returns([server("cap1"), server("cap2"), server("cap3")])
     Capistrano::SSH.expects(:connect).returns(:success)
     @config.connect! :once => true
     assert_equal %w(cap1), @config.sessions.keys.sort.map { |s| s.host }
   end
+
+  private
+
+    def mock_task(options={})
+      continue_on_error = options[:on_error] == :continue
+      stub("task", :fully_qualified_name => "name", :options => options, :continue_on_error? => continue_on_error)
+    end
 end

test/gateway_test.rb

@@ -77,6 +77,20 @@ class GatewayTest < Test::Unit::TestCase
     gateway.expects(:warn).times(2)
     assert_raises(Capistrano::ConnectionError) { gateway.connect_to(server("app1")) }
   end
+  
+  def test_connection_error_should_include_accessor_with_host_array
+    gateway = new_gateway
+    expect_connect_to(:host => "127.0.0.1").raises(RuntimeError)
+    gateway.expects(:warn).times(2)
+  
+    begin
+      gateway.connect_to(server("app1"))
+      flunk "expected an exception to be raised"
+    rescue Capistrano::ConnectionError => e
+      assert e.respond_to?(:hosts)
+      assert_equal %w(app1), e.hosts.map { |h| h.to_s }
+    end
+  end
 
   private
 

test/upload_test.rb

@@ -51,7 +51,23 @@ class UploadTest < Test::Unit::TestCase
     assert_equal 1, upload.failed
     assert_equal 1, upload.completed
   end
-
+  
+  def test_upload_error_should_include_accessor_with_host_array
+    sftp = mock_sftp
+    sftp.expects(:open).with("test.txt", @mode, 0660).yields(mock("status1", :code => Net::SFTP::Session::FX_OK), :file_handle)
+    sftp.expects(:write).with(:file_handle, "data").yields(mock("status2", :code => "bad status", :message => "bad status"))
+    session = mock("session", :sftp => sftp, :xserver => server("capistrano"))
+    upload = Capistrano::Upload.new([session], "test.txt", :data => "data", :logger => stub_everything)
+    
+    begin
+      upload.process!
+      flunk "expected an exception to be raised"
+    rescue Capistrano::UploadError => e
+      assert e.respond_to?(:hosts)
+      assert_equal %w(capistrano), e.hosts.map { |h| h.to_s }
+    end
+  end
+  
   def test_process_when_sftp_succeeds_should_raise_nothing
     sftp = mock_sftp
     sftp.expects(:open).with("test.txt", @mode, 0660).yields(mock("status1", :code => Net::SFTP::Session::FX_OK), :file_handle)