Render gitaly-unavailable error for Tags page

Changelog: performance

app/controllers/projects/tags_controller.rb

@@ -18,17 +18,21 @@ class Projects::TagsController < Projects::ApplicationController
     params[:sort] = params[:sort].presence || sort_value_recently_updated
 
     @sort = params[:sort]
-    @tags = TagsFinder.new(@repository, params).execute
-    @tags = Kaminari.paginate_array(@tags).page(params[:page])
 
+    @tags, @tags_loading_error = TagsFinder.new(@repository, params).execute
+
+    @tags = Kaminari.paginate_array(@tags).page(params[:page])
     tag_names = @tags.map(&:name)
     @tags_pipelines = @project.ci_pipelines.latest_successful_for_refs(tag_names)
+
     @releases = project.releases.where(tag: tag_names)
     @tag_pipeline_statuses = Ci::CommitStatusesFinder.new(@project, @repository, current_user, @tags).execute
 
     respond_to do |format|
-      format.html
-      format.atom { render layout: 'xml.atom' }
+      status = @tags_loading_error ? :service_unavailable : :ok
+
+      format.html { render status: status }
+      format.atom { render layout: 'xml.atom', status: status }
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/controllers/projects_controller.rb

@@ -284,9 +284,9 @@ class ProjectsController < Projects::ApplicationController
     end
 
     if find_tags && @repository.tag_count.nonzero?
-      tags = TagsFinder.new(@repository, params).execute.take(100).map(&:name)
+      tags, _ = TagsFinder.new(@repository, params).execute
 
-      options['Tags'] = tags
+      options['Tags'] = tags.take(100).map(&:name)
     end
 
     # If reference is commit id - we should add it to branch/tag selectbox

app/finders/tags_finder.rb

@@ -7,6 +7,9 @@ class TagsFinder < GitRefsFinder
 
   def execute
     tags = repository.tags_sorted_by(sort)
-    by_search(tags)
+
+    [by_search(tags), nil]
+  rescue Gitlab::Git::CommandError => e
+    [[], e]
   end
 end

app/views/projects/tags/index.html.haml

@@ -18,6 +18,9 @@
 
   = render_if_exists 'projects/commits/mirror_status'
 
+  - if @tags_loading_error
+    = render 'shared/errors/gitaly_unavailable', reason: s_('TagsPage|Unable to load tags')
+
   .tags
     - if @tags.any?
       %ul.flex-list.content-list

app/views/shared/errors/_gitaly_unavailable.html.haml

+.gl-alert.gl-alert-danger.gl-mb-5.gl-mt-5
+  .gl-alert-container
+    = sprite_icon('error', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
+    .gl-alert-content
+      .gl-alert-title
+        = reason
+      .gl-alert-body
+        = s_('The git server, Gitaly, is not available at this time. Please contact your administrator.')

lib/api/tags.rb

@@ -24,7 +24,7 @@ module API
         use :pagination
       end
       get ':id/repository/tags', feature_category: :source_code_management do
-        tags = ::TagsFinder.new(user_project.repository,
+        tags, _ = ::TagsFinder.new(user_project.repository,
                                 sort: "#{params[:order_by]}_#{params[:sort]}",
                                 search: params[:search]).execute
 

locale/gitlab.pot

@@ -33197,6 +33197,9 @@ msgstr ""
 msgid "TagsPage|This tag has no release notes."
 msgstr ""
 
+msgid "TagsPage|Unable to load tags"
+msgstr ""
+
 msgid "TagsPage|Use git tag command to add a new one:"
 msgstr ""
 
@@ -33779,6 +33782,9 @@ msgstr ""
 msgid "The form contains the following warning:"
 msgstr ""
 
+msgid "The git server, Gitaly, is not available at this time. Please contact your administrator."
+msgstr ""
+
 msgid "The global settings require you to enable Two-Factor Authentication for your account."
 msgstr ""
 

spec/controllers/projects/tags_controller_spec.rb

@@ -17,6 +17,25 @@ RSpec.describe Projects::TagsController do
       expect(assigns(:tags).map(&:name)).to include('v1.1.0', 'v1.0.0')
     end
 
+    context 'when Gitaly is unavailable' do
+      where(:format) do
+        [:html, :atom]
+      end
+
+      with_them do
+        it 'returns 503 status code' do
+          expect_next_instance_of(TagsFinder) do |finder|
+            expect(finder).to receive(:execute).and_return([[], Gitlab::Git::CommandError.new])
+          end
+
+          get :index, params: { namespace_id: project.namespace.to_param, project_id: project }, format: format
+
+          expect(assigns(:tags)).to eq([])
+          expect(response).to have_gitlab_http_status(:service_unavailable)
+        end
+      end
+    end
+
     it 'returns releases matching those tags' do
       subject
 

spec/finders/ci/commit_statuses_finder_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Ci::CommitStatusesFinder, '#execute' do
   let_it_be(:user) { create(:user) }
 
   context 'tag refs' do
-    let_it_be(:tags) { TagsFinder.new(project.repository, {}).execute }
+    let_it_be(:tags) { project.repository.tags }
 
     let(:subject) { described_class.new(project, project.repository, user, tags).execute }
 
@@ -131,7 +131,7 @@ RSpec.describe Ci::CommitStatusesFinder, '#execute' do
   end
 
   context 'CI pipelines visible to' do
-    let_it_be(:tags) { TagsFinder.new(project.repository, {}).execute }
+    let_it_be(:tags) { project.repository.tags }
 
     let(:subject) { described_class.new(project, project.repository, user, tags).execute }
 
@@ -161,7 +161,7 @@ RSpec.describe Ci::CommitStatusesFinder, '#execute' do
 
     context 'when not a member of a private project' do
       let(:private_project) { create(:project, :private, :repository) }
-      let(:private_tags) { TagsFinder.new(private_tags.repository, {}).execute }
+      let(:private_tags) { private_tags.repository.tags }
       let(:private_subject) { described_class.new(private_project, private_project.repository, user, tags).execute }
 
       before do

spec/finders/tags_finder_spec.rb

@@ -3,93 +3,76 @@
 require 'spec_helper'
 
 RSpec.describe TagsFinder do
-  let(:user) { create(:user) }
-  let(:project) { create(:project, :repository) }
-  let(:repository) { project.repository }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:repository) { project.repository }
+
+  def load_tags(params)
+    tags_finder = described_class.new(repository, params)
+    tags, error = tags_finder.execute
+
+    expect(error).to eq(nil)
+
+    tags
+  end
 
   describe '#execute' do
     context 'sort only' do
       it 'sorts by name' do
-        tags_finder = described_class.new(repository, {})
-
-        result = tags_finder.execute
-
-        expect(result.first.name).to eq("v1.0.0")
+        expect(load_tags({}).first.name).to eq("v1.0.0")
       end
 
       it 'sorts by recently_updated' do
-        tags_finder = described_class.new(repository, { sort: 'updated_desc' })
-
-        result = tags_finder.execute
         recently_updated_tag = repository.tags.max do |a, b|
           repository.commit(a.dereferenced_target).committed_date <=> repository.commit(b.dereferenced_target).committed_date
         end
 
-        expect(result.first.name).to eq(recently_updated_tag.name)
+        params = { sort: 'updated_desc' }
+
+        expect(load_tags(params).first.name).to eq(recently_updated_tag.name)
       end
 
       it 'sorts by last_updated' do
-        tags_finder = described_class.new(repository, { sort: 'updated_asc' })
+        params = { sort: 'updated_asc' }
 
-        result = tags_finder.execute
-
-        expect(result.first.name).to eq('v1.0.0')
+        expect(load_tags(params).first.name).to eq('v1.0.0')
       end
     end
 
     context 'filter only' do
       it 'filters tags by name' do
-        tags_finder = described_class.new(repository, { search: '1.0.0' })
-
-        result = tags_finder.execute
+        result = load_tags({ search: '1.0.0' })
 
         expect(result.first.name).to eq('v1.0.0')
         expect(result.count).to eq(1)
       end
 
       it 'does not find any tags with that name' do
-        tags_finder = described_class.new(repository, { search: 'hey' })
-
-        result = tags_finder.execute
-
-        expect(result.count).to eq(0)
+        expect(load_tags({ search: 'hey' }).count).to eq(0)
       end
 
       it 'filters tags by name that begins with' do
-        params = { search: '^v1.0' }
-        tags_finder = described_class.new(repository, params)
-
-        result = tags_finder.execute
+        result = load_tags({ search: '^v1.0' })
 
         expect(result.first.name).to eq('v1.0.0')
         expect(result.count).to eq(1)
       end
 
       it 'filters tags by name that ends with' do
-        params = { search: '0.0$' }
-        tags_finder = described_class.new(repository, params)
-
-        result = tags_finder.execute
+        result = load_tags({ search: '0.0$' })
 
         expect(result.first.name).to eq('v1.0.0')
         expect(result.count).to eq(1)
       end
 
       it 'filters tags by nonexistent name that begins with' do
-        params = { search: '^nope' }
-        tags_finder = described_class.new(repository, params)
-
-        result = tags_finder.execute
+        result = load_tags({ search: '^nope' })
 
         expect(result.count).to eq(0)
       end
 
       it 'filters tags by nonexistent name that ends with' do
-        params = { search: 'nope$' }
-        tags_finder = described_class.new(repository, params)
-
-        result = tags_finder.execute
-
+        result = load_tags({ search: 'nope$' })
         expect(result.count).to eq(0)
       end
     end
@@ -97,7 +80,7 @@ RSpec.describe TagsFinder do
     context 'filter and sort' do
       let(:tags_to_compare) { %w[v1.0.0 v1.1.0] }
 
-      subject { described_class.new(repository, params).execute.select { |tag| tags_to_compare.include?(tag.name) } }
+      subject { load_tags(params).select { |tag| tags_to_compare.include?(tag.name) } }
 
       context 'when sort by updated_desc' do
         let(:params) { { sort: 'updated_desc', search: 'v1' } }
@@ -117,5 +100,17 @@ RSpec.describe TagsFinder do
         end
       end
     end
+
+    context 'when Gitaly is unavailable' do
+      it 'returns empty list of tags' do
+        expect(Gitlab::GitalyClient).to receive(:call).and_raise(GRPC::Unavailable)
+
+        tags_finder = described_class.new(repository, {})
+        tags, error = tags_finder.execute
+
+        expect(error).to be_a(Gitlab::Git::CommandError)
+        expect(tags).to eq([])
+      end
+    end
   end
 end

spec/lib/gitlab/git/wraps_gitaly_errors_spec.rb

@@ -12,6 +12,7 @@ RSpec.describe Gitlab::Git::WrapsGitalyErrors do
     mapping = {
       GRPC::NotFound => Gitlab::Git::Repository::NoRepository,
       GRPC::InvalidArgument => ArgumentError,
+      GRPC::DeadlineExceeded => Gitlab::Git::CommandTimedOut,
       GRPC::BadStatus => Gitlab::Git::CommandError
     }
 

spec/views/projects/tags/index.html.haml_spec.rb

@@ -3,10 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe 'projects/tags/index.html.haml' do
-  let(:project)  { create(:project, :repository) }
-  let(:tags)     { TagsFinder.new(project.repository, {}).execute }
-  let(:git_tag)  { project.repository.tags.last }
-  let(:release)  { create(:release, project: project, sha: git_tag.target_commit.sha) }
+  let_it_be(:project)  { create(:project, :repository) }
+  let_it_be(:tags)     { project.repository.tags }
+  let_it_be(:git_tag)  { project.repository.tags.last }
+  let_it_be(:release)  { create(:release, project: project, sha: git_tag.target_commit.sha) }
+
   let(:pipeline) { create(:ci_pipeline, :success, project: project, ref: git_tag.name, sha: release.sha) }
 
   before do
@@ -86,4 +87,17 @@ RSpec.describe 'projects/tags/index.html.haml' do
       expect(page.all('.tags .content-list li')).not_to have_css 'svg.s24'
     end
   end
+
+  context 'when Gitaly is unavailable' do
+    it 'renders an error' do
+      assign(:tags_loading_error, GRPC::Unavailable.new)
+
+      content = render
+
+      expect(content).to include("Unable to load tags")
+      expect(content).to include(
+        "The git server, Gitaly, is not available at this time. Please contact your administrator."
+      )
+    end
+  end
 end