NPE-475: Fix AWS instance credential failures leaving Chore in an unrecoverable state

CHANGELOG.md

@@ -11,6 +11,18 @@
 **Cleanups**
 - N/A
 
+## [v4.7.0](https://github.com/Tapjoy/chore/tree/v4.7.0)
+
+**Features**
+- N/A
+
+**Fixed bugs**
+- Fix AWS instance credential fetching never retrying on failure
+- Fix SQS consumer never recovering from certain AWS client errors such as authentication failures
+
+**Cleanups**
+- N/A
+
 ## [v4.6.0](https://github.com/Tapjoy/chore/tree/v4.6.0)
 
 **Cleanups**

Gemfile.latest.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    chore-core (4.6.0)
+    chore-core (4.7.0)
       aws-sdk-sqs (>= 1)
       get_process_mem (>= 0.2.0)
       multi_json

Gemfile.ruby27.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    chore-core (4.6.0)
+    chore-core (4.7.0)
       aws-sdk-sqs (>= 1)
       get_process_mem (>= 0.2.0)
       multi_json

Gemfile.ruby31.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    chore-core (4.6.0)
+    chore-core (4.7.0)
       aws-sdk-sqs (>= 1)
       get_process_mem (>= 0.2.0)
       multi_json

Gemfile.ruby32.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    chore-core (4.6.0)
+    chore-core (4.7.0)
       aws-sdk-sqs (>= 1)
       get_process_mem (>= 0.2.0)
       multi_json

lib/chore/queues/sqs.rb

@@ -2,7 +2,12 @@ module Chore
   module Queues
     module SQS
       def self.sqs_client
-        Aws::SQS::Client.new(logger: Chore.logger, log_level: Chore.log_level_to_sym)
+        Aws::SQS::Client.new(
+          logger: Chore.logger,
+          log_level: Chore.log_level_to_sym,
+          instance_profile_credentials_timeout: 5, # default: 1
+          instance_profile_credentials_retries: 5, # default: 0
+        )
       end
 
       # Helper method to create queues based on the currently known list as provided by your configured Chore::Jobs

lib/chore/queues/sqs/consumer.rb

@@ -14,16 +14,21 @@ class Consumer < Chore::Consumer
         Chore::CLI.register_option 'aws_secret_key', '--aws-secret-key KEY', 'Valid AWS Secret Key'
         Chore::CLI.register_option 'dedupe_servers', '--dedupe-servers SERVERS', 'List of mememcache compatible server(s) to use for storing SQS Message Dedupe cache'
 
+        # Resets the API client connection and provides @@reset_at so we know when the last time that was done
+        def self.reset_connection!
+          @@reset_at = Time.now
+        end
+
         # @param [String] queue_name Name of SQS queue
         # @param [Hash] opts Options
         def initialize(queue_name, opts={})
           super(queue_name, opts)
           raise Chore::TerribleMistake, "Cannot specify a queue polling size greater than 10" if sqs_polling_amount > 10
         end
 
-        # Resets the API client connection and provides @@reset_at so we know when the last time that was done
-        def self.reset_connection!
-          @@reset_at = Time.now
+        # Ensure that that consumer is capable of running
+        def verify_connection!
+          queue.data
         end
 
         # Begins requesting messages from SQS, which will invoke the +&handler+ over each message
@@ -36,9 +41,6 @@ def consume(&handler)
             begin
               messages = handle_messages(&handler)
               sleep (Chore.config.consumer_sleep_interval) if messages.empty?
-            rescue Aws::SQS::Errors::NonExistentQueue => e
-              Chore.logger.error "You specified a queue '#{queue_name}' that does not exist. You must create the queue before starting Chore. Shutting down..."
-              raise Chore::TerribleMistake
             rescue => e
               Chore.logger.error { "SQSConsumer#Consume: #{e.inspect} #{e.backtrace * "\n"}" }
             end
@@ -86,6 +88,18 @@ def delay(item, backoff_calc)
         #
         # @return [Array<Aws::SQS::Message>]
         def handle_messages(&block)
+          begin
+            verify_connection!
+          rescue => e
+            # We shut down on connection failures for a few reasons:
+            # * The AWS SQS client has already been configured to retry on temporal issues like authentication failures
+            # * We rely on the operating system to re-run chore if it shuts down
+            # * We don't want chore to keep spinning if there's an unrecoverable exception with the client;
+            #   it's safest to restart chore in these situations
+            Chore.logger.error "There was a problem verifying the connection to the queue: #{e.message}. Shutting down..."
+            raise Chore::TerribleMistake
+          end
+
           msg = queue.receive_messages(:max_number_of_messages => sqs_polling_amount, :attribute_names => ['ApproximateReceiveCount'])
           messages = *msg
           received_timestamp = Time.now

lib/chore/version.rb

@@ -1,7 +1,7 @@
 module Chore
   module Version #:nodoc:
     MAJOR = 4
-    MINOR = 6
+    MINOR = 7
     PATCH = 0
 
     STRING = [ MAJOR, MINOR, PATCH ].join('.')

spec/chore/queues/sqs/consumer_spec.rb

@@ -31,6 +31,7 @@ def consume(&block)
   before do
     allow(Aws::SQS::Client).to receive(:new).and_return(sqs)
     allow(Aws::SQS::Queue).to receive(:new).and_return(queue)
+    allow(queue).to receive(:data).and_return(nil)
     allow(queue).to receive(:receive_messages).and_return(receive_message_result)
     allow(message).to receive(:attributes).and_return({ 'ApproximateReceiveCount' => rand(10) })
   end
@@ -135,6 +136,36 @@ def consume(&block)
         end
       end
     end
+
+    context 'on queue lookup failure' do
+      before(:each) do
+        allow(consumer).to receive(:verify_connection!).and_raise(Aws::SQS::Errors::NonExistentQueue)
+      end
+
+      it 'should raise exception' do
+        expect { consume }.to raise_error(Chore::TerribleMistake)
+      end
+    end
+
+    context 'on aws credential failure' do
+      before(:each) do
+        allow(consumer).to receive(:verify_connection!).and_raise(Aws::Errors::MissingCredentialsError)
+      end
+
+      it 'should raise exception' do
+        expect { consume }.to raise_error(Chore::TerribleMistake)
+      end
+    end
+
+    context 'on unexpected failure' do
+      before(:each) do
+        allow(consumer).to receive(:verify_connection!).and_raise(Seahorse::Client::NetworkingError)
+      end
+
+      it 'should raise exception' do
+        expect { consume }.to raise_error(Chore::TerribleMistake)
+      end
+    end
   end
 
   describe "completing work" do