Merge pull request #32 from Fatsoma/ENG-549-log-channel-error

Hutch will log and notify on channel error

Author: brentfatsoma

lib/hutch/channel_broker.rb

@@ -63,6 +63,25 @@ def open_channel
           logger.info 'enabling publisher confirms'
           ch.confirm_select
         end
+
+        # on_error handler logs and notifies any unhandled channel errors
+        ch.on_error do |channel, method|
+          logger.error "Channel error [channel=#{channel.inspect}, method=#{method.inspect}, active=#{active}]"
+
+          context = {method: method.inspect}
+          if method.is_a?(AMQ::Protocol::Channel::Close)
+            error_message = method.reply_text
+            context[:reply_code] = method.reply_code
+          else
+            error_message = 'Channel error'
+          end
+
+          ::Honeybadger.notify(
+            error_class: 'Hutch::ChannelBroker::OnError',
+            error_message: error_message,
+            context: context
+          ) if defined?(::Honeybadger)
+        end
       end
     end
 

spec/hutch/channel_broker_spec.rb

@@ -11,12 +11,13 @@
     Hutch::Config.instance_variable_set(:@config, nil)
     Hutch::Config.initialize
   end
-  let(:connection) { double('Connection') }
-  let(:channel) { double('Channel') }
+  let(:connection) { double(:connection) }
+  let(:channel) { double(:channel) }
+
   subject(:channel_broker) { Hutch::ChannelBroker.new(connection, config) }
 
   shared_examples 'an empty channel broker' do
-    %i(channel exchange default_wait_exchange wait_exchanges).each do |name|
+    %i[channel exchange default_wait_exchange wait_exchanges].each do |name|
       it { expect(channel_broker.instance_variable_get("@#{name}")).to be_nil }
     end
   end
@@ -46,8 +47,8 @@
   end
 
   describe '#reconnect' do
-    let(:new_channel) { double('Channel') }
-    let(:new_exchange) { double('Exchange') }
+    let(:new_channel) { double(:new_channel) }
+    let(:new_exchange) { double(:new_exchange) }
 
     before do
       allow(channel_broker).to receive(:disconnect)
@@ -97,8 +98,8 @@
   describe '#declare_wait_exchange' do
     let(:expiration) { rand(1000..1_000_000) }
     let(:suffix) { expiration.to_s }
-    let(:new_exchange) { double('Exchange') }
-    let(:new_queue) { double('Queue') }
+    let(:new_exchange) { double(:new_exchange) }
+    let(:new_queue) { double(:new_queue) }
     let(:exchange_name) { "wait-exchange_#{suffix}" }
     let(:queue_name) { "wait-queue_#{suffix}" }
     let(:main_exchange_name) { 'main-exchange' }
@@ -129,4 +130,115 @@
       is_expected.to eq(new_exchange)
     end
   end
+
+  describe '#open_channel' do
+    let(:channel) do
+      Class.new do
+        def on_error(&block)
+          @on_error = block
+        end
+        def test_on_error(*args)
+          @on_error.call(*args)
+        end
+        def confirm_select; end
+      end.new
+    end
+    let(:honey_badger) { double(:honey_badger) }
+    let(:config) { {} }
+    let(:method) { Class.new }
+
+    before do
+      stub_const('Honeybadger', honey_badger)
+      allow(honey_badger).to receive(:add_breadcrumb)
+      allow(honey_badger).to receive(:notify)
+      allow(connection).to receive(:create_channel).and_return(channel)
+      allow(connection).to receive(:prefetch_channel)
+      allow(channel).to receive(:confirm_select)
+    end
+
+    subject { channel_broker.open_channel }
+
+    context 'when no publisher_confirms or force_publisher_confirms option' do
+      let(:config) { {} }
+
+      it do
+        is_expected.to eq(channel)
+        expect(channel).to_not have_received(:confirm_select)
+      end
+    end
+
+    context 'when publisher_confirms option' do
+      let(:config) { { publisher_confirms: true } }
+
+      it do
+        is_expected.to eq(channel)
+        expect(channel).to have_received(:confirm_select)
+      end
+    end
+
+    context 'when force_publisher_confirms option' do
+      let(:config) { { force_publisher_confirms: true } }
+
+      it do
+        is_expected.to eq(channel)
+        expect(channel).to have_received(:confirm_select)
+      end
+    end
+
+    context 'when on_error block is called' do
+      let(:reply_code) { 406 }
+      let(:reply_text) { 'delivery acknowledgement on channel 1 timed out' }
+      let(:class_id) { 'test-class-id' }
+      let(:method_id) { 'test-method-id' }
+
+      subject do
+        channel_broker.open_channel.tap do |channel|
+          channel.test_on_error(channel, method)
+        end
+      end
+
+      context 'when method is AMQ::Protocol::Channel::Close' do
+        let(:method) do
+          AMQ::Protocol::Channel::Close.new(reply_code, reply_text, class_id, method_id)
+        end
+        let(:context) do
+          {
+            reply_code: reply_code,
+            method: method.inspect
+          }
+        end
+
+        it do
+          is_expected.to eq(channel)
+          expect(honey_badger).to have_received(:notify)
+            .with(
+              error_class: 'Hutch::ChannelBroker::OnError',
+              error_message: reply_text,
+              context: context
+            )
+        end
+      end
+
+      context 'when method is AMQ::Protocol::Channel::CloseOk' do
+        let(:method) do
+          AMQ::Protocol::Channel::CloseOk.new
+        end
+        let(:context) do
+          {
+            method: method.inspect
+          }
+        end
+
+        it do
+          is_expected.to eq(channel)
+          expect(honey_badger).to have_received(:notify)
+            .with(
+              error_class: 'Hutch::ChannelBroker::OnError',
+              error_message: 'Channel error',
+              context: context
+            )
+        end
+      end
+    end
+  end
 end