From da8d4d596cefe99c11a57291c2e14bb19cccaf6a Mon Sep 17 00:00:00 2001
From: Pablo Cantero <pablo@pablocantero.com>
Date: Thu, 25 May 2017 07:35:22 -0400
Subject: [PATCH] Logout when batch delete returns any failure

---
 bin/cli/sqs.rb               |  5 ++++-
 lib/shoryuken/queue.rb       | 10 ++++++++-
 spec/shoryuken/queue_spec.rb | 40 ++++++++++++++++++++++++++++++------
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/bin/cli/sqs.rb b/bin/cli/sqs.rb
index 68cf9882..43305879 100644
--- a/bin/cli/sqs.rb
+++ b/bin/cli/sqs.rb
@@ -32,7 +32,10 @@ def batch_delete(url, messages)
               queue_url: url,
               entries: batch.map { |message| { id: message.message_id, receipt_handle: message.receipt_handle } }
             ).failed.any? do |failure|
-              say "Could not delete #{failure.id}, code: #{failure.code}", :yellow
+              say(
+                "Could not delete #{failure.id}, code: '#{failure.code}', message: '#{failure.message}', sender_fault: #{failure.sender_fault}",
+                :yellow
+              )
             end
           end
         end
diff --git a/lib/shoryuken/queue.rb b/lib/shoryuken/queue.rb
index 95c49b6a..324b2e89 100644
--- a/lib/shoryuken/queue.rb
+++ b/lib/shoryuken/queue.rb
@@ -1,5 +1,7 @@
 module Shoryuken
   class Queue
+    include Util
+
     FIFO_ATTR               = 'FifoQueue'
     MESSAGE_GROUP_ID        = 'ShoryukenMessage'
     VISIBILITY_TIMEOUT_ATTR = 'VisibilityTimeout'
@@ -19,7 +21,13 @@ def visibility_timeout
     end
 
     def delete_messages(options)
-      client.delete_message_batch(options.merge(queue_url: url))
+      client.delete_message_batch(
+        options.merge(queue_url: url)
+      ).failed.any? do |failure|
+        logger.error do
+          "Could not delete #{failure.id}, code: '#{failure.code}', message: '#{failure.message}', sender_fault: #{failure.sender_fault}"
+        end
+      end
     end
 
     def send_message(options)
diff --git a/spec/shoryuken/queue_spec.rb b/spec/shoryuken/queue_spec.rb
index 741f6aed..5d7c7f5e 100644
--- a/spec/shoryuken/queue_spec.rb
+++ b/spec/shoryuken/queue_spec.rb
@@ -4,19 +4,47 @@
   let(:credentials) { Aws::Credentials.new('access_key_id', 'secret_access_key') }
   let(:sqs) { Aws::SQS::Client.new(stub_responses: true, credentials: credentials) }
   let(:queue_name) { 'shoryuken' }
-  let(:queue_url) { 'https://eu-west-1.amazonaws.com:6059/123456789012/shoryuken' }
+  let(:queue_url) { "https://eu-west-1.amazonaws.com:6059/0123456789/#{queue_name}" }
 
   subject { described_class.new(sqs, queue_name) }
-  before {
+
+  before do
     # Required as Aws::SQS::Client.get_queue_url returns 'String' when responses are stubbed,
     # which is not accepted by Aws::SQS::Client.get_queue_attributes for :queue_name parameter.
     allow(subject).to receive(:url).and_return(queue_url)
-  }
+  end
+
+  describe '#delete_messages' do
+    let(:entries) do
+      [
+        { id: '1', receipt_handle:  '1' },
+        { id: '2', receipt_handle:  '2' }
+      ]
+    end
+
+    it 'deletes' do
+      expect(sqs).to receive(:delete_message_batch).with(entries: entries, queue_url: queue_url).and_return(double(failed: []))
+
+      subject.delete_messages(entries: entries)
+    end
+
+    context 'when it fails' do
+      it 'logs the reason' do
+        failure = double(id: 'id', code: 'code', message: '...', sender_fault: false)
+        logger = double 'Logger'
+
+        expect(sqs).to receive(:delete_message_batch).with(entries: entries, queue_url: queue_url).and_return(double(failed: [failure]))
+        expect(subject).to receive(:logger).and_return(logger)
+        expect(logger).to receive(:error)
+
+        subject.delete_messages(entries: entries)
+      end
+    end
+  end
 
   describe '#send_message' do
-    before {
-      allow(subject).to receive(:fifo?).and_return(false)
-    }
+    before { allow(subject).to receive(:fifo?).and_return(false) }
+
     it 'accepts SQS request parameters' do
       # https://docs.aws.amazon.com/sdkforruby/api/Aws/SQS/Client.html#send_message-instance_method
       expect(sqs).to receive(:send_message).with(hash_including(message_body: 'msg1'))