Merge branch 'wiki_git_exception_handling' into 'master'

Handle git exceptions in wiki create and update services

See merge request gitlab-org/gitlab!64873

app/models/wiki.rb

@@ -278,6 +278,19 @@ class Wiki
     @repository = nil
   end
 
+  def capture_git_error(action, &block)
+    yield block
+  rescue Gitlab::Git::Index::IndexError,
+         Gitlab::Git::CommitError,
+         Gitlab::Git::PreReceiveError,
+         Gitlab::Git::CommandError,
+         ArgumentError => error
+
+    Gitlab::ErrorTracking.log_exception(error, action: action, wiki_id: id)
+
+    false
+  end
+
   private
 
   def multi_commit_options(action, message = nil, title = nil)
@@ -311,19 +324,6 @@ class Wiki
     "#{user.username} #{action} page: #{title}"
   end
 
-  def capture_git_error(action, &block)
-    yield block
-  rescue Gitlab::Git::Index::IndexError,
-         Gitlab::Git::CommitError,
-         Gitlab::Git::PreReceiveError,
-         Gitlab::Git::CommandError,
-         ArgumentError => error
-
-    Gitlab::ErrorTracking.log_exception(error, action: action, wiki_id: id)
-
-    false
-  end
-
   def change_head_to_default_branch
     repository.raw_repository.write_ref('HEAD', "refs/heads/#{default_branch}")
   end

app/services/wiki_pages/create_service.rb

@@ -6,11 +6,12 @@ module WikiPages
       wiki = Wiki.for_container(container, current_user)
       page = WikiPage.new(wiki)
 
-      if page.create(@params)
-        execute_hooks(page)
+      wiki.capture_git_error(event_action) do
+        page.create(@params)
       end
 
       if page.persisted?
+        execute_hooks(page)
         ServiceResponse.success(payload: { page: page })
       else
         ServiceResponse.error(message: _('Could not create wiki page'), payload: { page: page })

app/services/wiki_pages/update_service.rb

@@ -8,7 +8,7 @@ module WikiPages
       # this class is not thread safe!
       @old_slug = page.slug
 
-      if page.update(@params)
+      if page.wiki.capture_git_error(event_action) { page.update(@params) }
         execute_hooks(page)
         ServiceResponse.success(payload: { page: page })
       else

spec/services/wiki_pages/create_service_spec.rb

@@ -4,4 +4,24 @@ require 'spec_helper'
 
 RSpec.describe WikiPages::CreateService do
   it_behaves_like 'WikiPages::CreateService#execute', :project
+
+  describe '#execute' do
+    let_it_be(:project) { create(:project) }
+
+    subject(:service) { described_class.new(container: project) }
+
+    context 'when wiki create fails due to git error' do
+      let(:wiki_git_error) { 'Could not create wiki page' }
+
+      it 'catches the thrown error and returns a ServiceResponse error' do
+        allow_next_instance_of(WikiPage) do |instance|
+          allow(instance).to receive(:create).and_raise(Gitlab::Git::CommandError.new(wiki_git_error))
+        end
+
+        result = service.execute
+        expect(result).to be_error
+        expect(result.message).to eq(wiki_git_error)
+      end
+    end
+  end
 end

spec/services/wiki_pages/update_service_spec.rb

@@ -4,4 +4,26 @@ require 'spec_helper'
 
 RSpec.describe WikiPages::UpdateService do
   it_behaves_like 'WikiPages::UpdateService#execute', :project
+
+  describe '#execute' do
+    let_it_be(:project) { create(:project) }
+
+    let(:page) { create(:wiki_page, project: project) }
+
+    subject(:service) { described_class.new(container: project) }
+
+    context 'when wiki create fails due to git error' do
+      let(:wiki_git_error) { 'Could not update wiki page' }
+
+      it 'catches the thrown error and returns a ServiceResponse error' do
+        allow_next_instance_of(WikiPage) do |instance|
+          allow(instance).to receive(:update).and_raise(Gitlab::Git::CommandError.new(wiki_git_error))
+        end
+
+        result = service.execute(page)
+        expect(result).to be_error
+        expect(result.message).to eq(wiki_git_error)
+      end
+    end
+  end
 end