Do not retry sending out posts on SSL errors. See #4728

Refactor HydraWrapper so the when-to-retry logic is in the worker

app/workers/http_multi.rb

@@ -7,7 +7,10 @@ module Workers
     sidekiq_options queue: :http
 
     MAX_RETRIES = 3
-
+    ABANDON_ON_CODES=[:peer_failed_verification, # Certificate does not match URL
+                      :ssl_connect_error, # Problem negotiating ssl version or Cert couldn't be verified (often self-signed)
+                      :ssl_cacert, # Expired SSL cert
+                      ]
     def perform(user_id, encoded_object_xml, person_ids, dispatcher_class_as_string, retry_count=0)
       user = User.find(user_id)
       people = Person.where(:id => person_ids)
@@ -16,11 +19,17 @@ module Workers
       hydra = HydraWrapper.new(user, people, encoded_object_xml, dispatcher)
 
       hydra.enqueue_batch
+
+      hydra.keep_for_retry_if do |response|
+        !ABANDON_ON_CODES.include?(response.return_code)
+      end
+
       hydra.run
 
-      unless hydra.failed_people.empty?
+
+      unless hydra.people_to_retry.empty?
         if retry_count < MAX_RETRIES
-          Workers::HttpMulti.perform_in(1.hour, user_id, encoded_object_xml, hydra.failed_people, dispatcher_class_as_string, retry_count + 1)
+          Workers::HttpMulti.perform_in(1.hour, user_id, encoded_object_xml, hydra.people_to_retry, dispatcher_class_as_string, retry_count + 1)
         else
           Rails.logger.info("event=http_multi_abandon sender_id=#{user_id} failed_recipient_ids='[#{person_ids.join(', ')}] '")
         end

lib/hydra_wrapper.rb

@@ -17,16 +17,19 @@ class HydraWrapper
     }
   }
 
-  attr_reader :failed_people, :user, :encoded_object_xml
+  attr_reader :people_to_retry , :user, :encoded_object_xml
   attr_accessor :dispatcher_class, :people
   delegate :run, to: :hydra
 
   def initialize user, people, encoded_object_xml, dispatcher_class
     @user = user
-    @failed_people = []
+    @people_to_retry = []
     @people = people
     @dispatcher_class = dispatcher_class
     @encoded_object_xml = encoded_object_xml
+    @keep_for_retry_proc = Proc.new do |response|
+      true
+    end
   end
 
   # Inserts jobs for all @people
@@ -38,6 +41,14 @@ class HydraWrapper
     end
   end
 
+  # This method can be used to tell the hydra whether or not to
+  # retry a request that it made which failed.
+  # @yieldparam response [Typhoeus::Response] The response object for the failed request.
+  # @yieldreturn [Boolean] Whether the request whose response was passed to the block should be retried.
+  def keep_for_retry_if &block
+    @keep_for_retry_proc = block
+  end
+
   private
 
   def hydra
@@ -83,11 +94,14 @@ class HydraWrapper
           event: "http_multi_fail",
           sender_id: @user.id,
           url: response.effective_url,
-          response_code: response.code
+          return_code: response.return_code
         }
-        message[:response_message] = response.return_message if response.code == 0
         Rails.logger.info message.to_a.map { |k,v| "#{k}=#{v}" }.join(' ')
-        @failed_people += people_for_receive_url.map(&:id)
+
+        if @keep_for_retry_proc.call(response)
+          @people_to_retry += people_for_receive_url.map(&:id)
+        end
+
       end
     end
   end

spec/workers/http_multi_spec.rb

@@ -26,13 +26,29 @@ describe Workers::HttpMulti do
       code: 200,
       body: "",
       time: 0.2,
-      effective_url: 'http://foobar.com'
+      effective_url: 'http://foobar.com',
+      return_code: :ok
     )
     @failed_response = Typhoeus::Response.new(
       code: 504,
       body: "",
       time: 0.2,
-      effective_url: 'http://foobar.com'
+      effective_url: 'http://foobar.com',
+      return_code: :ok
+    )
+    @ssl_error_response = Typhoeus::Response.new(
+      code: 0,
+      body: "",
+      time: 0.2,
+      effective_url: 'http://foobar.com',
+      return_code: :ssl_connect_error
+    )
+    @unable_to_resolve_response = Typhoeus::Response.new(
+      code: 0,
+      body: "",
+      time: 0.2,
+      effective_url: 'http://foobar.com',
+      return_code: :couldnt_resolve_host
     )
   end
 
@@ -56,6 +72,24 @@ describe Workers::HttpMulti do
     Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
   end
 
+  it 'retries if it could not resolve the server' do
+    person = @people.first
+
+    Typhoeus.stub(person.receive_url).and_return @unable_to_resolve_response
+
+    Workers::HttpMulti.should_receive(:perform_in).with(1.hour, bob.id, @post_xml, [person.id], anything, 1).once
+    Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
+  end
+
+  it 'does not retry on an SSL error' do
+    person = @people.first
+
+    Typhoeus.stub(person.receive_url).and_return @ssl_error_response
+
+    Workers::HttpMulti.should_not_receive(:perform_in)
+    Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
+  end
+
   it 'max retries' do
     person = @people.first