Merge branch 'cat-geo-rescue-after-commit-hook-errors' into 'master'

Rescue Geo after_{create,destroy} hooks on regular models

See merge request gitlab-org/gitlab!82905

ee/app/models/concerns/geo/replicable_model.rb

@@ -4,11 +4,20 @@ module Geo
   module ReplicableModel
     extend ActiveSupport::Concern
     include Checksummable
+    include ::Gitlab::Geo::LogHelpers
 
     included do
       # If this hook turns out not to apply to all Models, perhaps we should extract a `ReplicableBlobModel`
-      after_create_commit -> { replicator.handle_after_create_commit if replicator.respond_to?(:handle_after_create_commit) }
-      after_destroy -> { replicator.handle_after_destroy if replicator.respond_to?(:handle_after_destroy) }
+      after_create_commit -> do
+        replicator.handle_after_create_commit if replicator.respond_to?(:handle_after_create_commit)
+      rescue StandardError => err
+        log_error("Geo replicator after_create_commit failed", err)
+      end
+      after_destroy -> do
+        replicator.handle_after_destroy if replicator.respond_to?(:handle_after_destroy)
+      rescue StandardError => err
+        log_error("Geo replicator after_destroy failed", err)
+      end
 
       # Temporarily defining `verification_succeeded` and
       # `verification_failed` for unverified models while verification is

ee/spec/models/concerns/geo/replicable_model_spec.rb

@@ -34,6 +34,40 @@ RSpec.describe Geo::ReplicableModel do
     let(:replicator_class) { Geo::DummyReplicator }
   end
 
+  describe 'after_create_commit hook' do
+    context 'when the replicator raises an error' do
+      let(:error) { StandardError.new("testing error") }
+
+      before do
+        expect_next_instance_of(Geo::DummyReplicator) do |instance|
+          expect(instance).to receive(:handle_after_create_commit).and_raise(error)
+        end
+      end
+
+      it 'saves the model' do
+        expect { subject.save! }.to change { DummyModel.count }.by(1)
+      end
+    end
+  end
+
+  describe 'after_destroy hook' do
+    context 'when the replicator raises an error' do
+      let(:error) { StandardError.new("testing error") }
+
+      before do
+        expect_next_instance_of(Geo::DummyReplicator) do |instance|
+          expect(instance).to receive(:handle_after_destroy).and_raise(error)
+        end
+      end
+
+      it 'destroys the model' do
+        subject.save!
+
+        expect { subject.destroy! }.to change { DummyModel.count }.by(-1)
+      end
+    end
+  end
+
   describe '.verifiables' do
     context 'when the model can be filtered by locally stored files' do
       it 'filters by locally stored files' do

ee/spec/support/helpers/ee/geo_helpers.rb

@@ -67,6 +67,10 @@ module EE
           true
         end
 
+        def handle_after_destroy
+          true
+        end
+
         protected
 
         def consume_event_test(user:, other:)