Add recover_from_failure hook to SynchronizeBrokerCatalogJob

When a create-service-broker job fails during a database outage, the
  broker can get stuck in SYNCHRONIZING state. The failure occurs after
  the state was set but before the rescue block can revert it, leaving
  the broker permanently unusable.

  This adds a recover_from_failure hook that runs when the delayed_job
  transitions to FAILED state (after all retries are exhausted). At this
  point the database is likely back up, allowing the broker state to be
  set to SYNCHRONIZATION_FAILED. The WHERE clause ensures we only update
  brokers still in SYNCHRONIZING state, preventing overwrites of newer
  state changes.

  This mirrors the fix applied to UpdateBrokerJob and ensures both
  create and update operations handle database failures gracefully.

Author: kathap

app/jobs/pollable_job_wrapper.rb

@@ -72,9 +72,9 @@ def error(job, exception)
       def failure(job)
         begin
           unwrapped_job = Enqueuer.unwrap_job(@handler)
-          unwrapped_job.recover_from_failure if unwrapped_job.respond_to?(:recover_from_failure)
+          unwrapped_job.recover_from_failure
         rescue StandardError => e
-          logger.error("failure recovery failed for #{unwrapped_job.class.name}: #{e.class}: #{e.message}")
+          logger.error("failure recovery failed: #{e.class}: #{e.message}")
         end
 
         change_state(job, PollableJobModel::FAILED_STATE)

app/jobs/v3/services/synchronize_broker_catalog_job.rb

@@ -34,6 +34,14 @@ def display_name
         'service_broker.catalog.synchronize'
       end
 
+      def recover_from_failure
+        ServiceBroker.where(guid: broker_guid, state: ServiceBrokerStateEnum::SYNCHRONIZING).
+          update(state: ServiceBrokerStateEnum::SYNCHRONIZATION_FAILED)
+      rescue StandardError => e
+        logger = Steno.logger('cc.background')
+        logger.error("Failed to recover broker state for #{broker_guid}: #{e.class}: #{e.message}")
+      end
+
       private
 
       attr_reader :broker_guid, :user_audit_info

app/jobs/v3/services/update_broker_job.rb

@@ -39,7 +39,7 @@ def display_name
 
       def recover_from_failure
         ServiceBroker.where(guid: broker_guid, state: ServiceBrokerStateEnum::SYNCHRONIZING).
-          update(state: previous_broker_state)
+          update(state: ServiceBrokerStateEnum::SYNCHRONIZATION_FAILED)
       rescue StandardError => e
         logger = Steno.logger('cc.background')
         logger.error("Failed to recover broker state for #{broker_guid}: #{e.class}: #{e.message}")

app/jobs/wrapping_job.rb

@@ -51,6 +51,10 @@ def error(job, e)
         handler.error(job, e) if handler.respond_to?(:error)
       end
 
+      def recover_from_failure
+        handler.recover_from_failure if handler.respond_to?(:recover_from_failure)
+      end
+
       def display_name
         handler.respond_to?(:display_name) ? handler.display_name : handler.class.name
       end

spec/unit/jobs/pollable_job_wrapper_spec.rb

@@ -235,7 +235,7 @@ class BigException < StandardError
             execute_all_jobs(expected_successes: 0, expected_failures: 1)
 
             broker.reload
-            expect(broker.state).to eq('AVAILABLE')
+            expect(broker.state).to eq('SYNCHRONIZATION_FAILED')
           end
         end
 

spec/unit/jobs/v3/services/update_broker_job_spec.rb

@@ -496,11 +496,11 @@ module V3
                 broker.update(state: ServiceBrokerStateEnum::SYNCHRONIZING)
               end
 
-              it 'reverts the broker to the previous state' do
+              it 'sets the broker to SYNCHRONIZATION_FAILED' do
                 job.recover_from_failure
 
                 broker.reload
-                expect(broker.state).to eq(ServiceBrokerStateEnum::AVAILABLE)
+                expect(broker.state).to eq(ServiceBrokerStateEnum::SYNCHRONIZATION_FAILED)
               end
             end
 

spec/unit/jobs/wrapping_job_spec.rb

@@ -198,6 +198,28 @@ module Jobs
           end
         end
       end
+
+      describe '#recover_from_failure' do
+        context 'when the wrapped job has the recover_from_failure method defined' do
+          it 'delegates to the handler' do
+            handler = double('Job', recover_from_failure: nil)
+            job     = WrappingJob.new(handler)
+
+            expect(handler).to receive(:recover_from_failure)
+            job.recover_from_failure
+          end
+        end
+
+        context 'when the wrapped job does not have the recover_from_failure method defined' do
+          it 'does not raise an exception' do
+            handler = Object.new
+            job     = WrappingJob.new(handler)
+            expect do
+              job.recover_from_failure
+            end.not_to raise_error
+          end
+        end
+      end
     end
   end
 end