Don't use logger.error with capistrano 2 (#1043)

* fix: don't use logger.error with capistrano 2

* chore: pin rexml to ruby v2.0 compatible version for Rails 3 ci config

* style: refactor for line length

Author: waltjones

gemfiles/rails30.gemfile

@@ -30,6 +30,7 @@ platforms :rbx do
 end
 
 gem 'capistrano', :require => false
+gem 'rexml', '<= 3.2.4'
 gem 'sucker_punch', '~> 2.0'
 gem 'shoryuken'
 gem 'codacy-coverage'

gemfiles/rails31.gemfile

@@ -29,6 +29,7 @@ platforms :rbx do
 end
 
 gem 'capistrano', :require => false
+gem 'rexml', '<= 3.2.4'
 gem 'sucker_punch'
 gem 'shoryuken'
 gem 'codacy-coverage'

gemfiles/rails32.gemfile

@@ -31,6 +31,7 @@ platforms :rbx do
 end
 
 gem 'capistrano', :require => false
+gem 'rexml', '<= 3.2.4'
 gem 'sucker_punch', '~> 2.0'
 gem 'shoryuken'
 gem 'codacy-coverage'

lib/rollbar/capistrano_tasks.rb

@@ -16,7 +16,9 @@ def deploy_started(capistrano, logger, dry_run)
             if result[:success] && (deploy_id = result[:data] && result[:data][:deploy_id])
               capistrano.set :rollbar_deploy_id, deploy_id
             else
-              logger.error 'Unable to report deploy to Rollbar' + (result[:message] ? ': ' + result[:message] : '')
+              message = format_message('Unable to report deploy to Rollbar',
+                                       result[:message])
+              log_error(logger, message)
             end
           end
         end
@@ -42,7 +44,7 @@ def deploy_task(logger, opts = {})
         yield
 
       rescue StandardError => e
-        logger.error "Error reporting to Rollbar: #{e.inspect}"
+        log_error logger, "Error reporting to Rollbar: #{e.inspect}"
       end
 
       def deploy_update(capistrano, logger, dry_run, opts = {})
@@ -56,7 +58,9 @@ def deploy_update(capistrano, logger, dry_run, opts = {})
               if result[:success]
                 logger.info 'Updated deploy status in Rollbar'
               else
-                logger.error 'Unable to update deploy status in Rollbar' + (result[:message] ? ': ' + result[:message] : '')
+                message = format_message('Unable to update deploy status in Rollbar',
+                                         result[:message])
+                log_error(logger, message)
               end
             end
           end
@@ -117,7 +121,7 @@ def depend_on_deploy_id(capistrano, logger)
         if capistrano.fetch(:rollbar_deploy_id)
           yield
         else
-          logger.error 'Failed to update the deploy in Rollbar. No deploy id available.'
+          log_error logger, 'Failed to update the deploy in Rollbar. No deploy id available.'
         end
       end
 
@@ -134,6 +138,20 @@ def debug_request_response(logger, result)
         logger.debug result[:request_info]
         logger.debug result[:response_info] if result[:response_info]
       end
+
+      def format_message(*args)
+        args.compact.join(': ')
+      end
+
+      def log_error(logger, message)
+        # Capistrano 2.x doesn't have the #error method,
+        # so we use #important if #error isn't present
+        if logger.respond_to?(:error)
+          logger.error message
+        elsif logger.respond_to?(:important)
+          logger.important message
+        end
+      end
     end
   end
 end

spec/rollbar/capistrano_tasks_spec.rb

@@ -14,6 +14,12 @@
   let(:rollbar_revision) { 'sha123' }
   let(:dry_run) { false }
 
+  class Capistrano2LoggerStub
+    def important(message, line_prefix=nil) end
+    def info(message, line_prefix=nil) end
+    def debug(message, line_prefix=nil) end
+  end
+
   before do
     allow(logger).to receive(:info)
     allow(logger).to receive(:error)
@@ -88,6 +94,19 @@
 
         subject.deploy_started(capistrano, logger, dry_run)
       end
+
+      context 'when using Capistrano 2.x logger' do
+        let(:logger2) { Capistrano2LoggerStub.new }
+
+        it "logs the error to the logger" do
+          expect(::Rollbar::Deploy).to receive(:report)
+            .and_raise('an API exception')
+
+          expect(logger2).to receive(:important).with(/an API exception/)
+
+          subject.deploy_started(capistrano, logger2, dry_run)
+        end
+      end
     end
 
     context 'with --dry-run provided' do
@@ -174,6 +193,19 @@
 
           subject.deploy_succeeded(capistrano, logger, dry_run)
         end
+
+        context 'when using Capistrano 2.x logger' do
+          let(:logger2) { Capistrano2LoggerStub.new }
+
+          it "logs the error to the logger" do
+            expect(::Rollbar::Deploy).to receive(:update)
+              .and_raise('an API exception')
+
+            expect(logger2).to receive(:important).with(/an API exception/)
+
+            subject.deploy_succeeded(capistrano, logger2, dry_run)
+          end
+        end
       end
 
       context 'with --dry-run provided' do
@@ -282,6 +314,19 @@
 
           subject.deploy_failed(capistrano, logger, dry_run)
         end
+
+        context 'when using Capistrano 2.x logger' do
+          let(:logger2) { Capistrano2LoggerStub.new }
+
+          it "logs the error to the logger" do
+            expect(::Rollbar::Deploy).to receive(:update)
+              .and_raise('an API exception')
+
+            expect(logger2).to receive(:important).with(/an API exception/)
+
+            subject.deploy_failed(capistrano, logger2, dry_run)
+          end
+        end
       end
 
       context 'with --dry-run provided' do
@@ -324,6 +369,18 @@
 
         subject.deploy_failed(capistrano, logger, dry_run)
       end
+
+      context 'when using Capistrano 2.x logger' do
+        let(:logger2) { Capistrano2LoggerStub.new }
+
+        it 'displays an error message and exits' do
+          expect(Rollbar::Deploy).to_not receive(:report)
+
+          expect(logger2).to receive(:important).with(/Failed(.*)No deploy id available/)
+
+          subject.deploy_failed(capistrano, logger2, dry_run)
+        end
+      end
     end
   end