Merge branch 'ajk-catch-wiki-timeouts' into 'master'

Resolve "Catch wiki timeouts when rendering pages"

See merge request gitlab-org/gitlab!46627

app/controllers/concerns/wiki_actions.rb

@@ -8,6 +8,8 @@ module WikiActions
   include RedisTracking
   extend ActiveSupport::Concern
 
+  RESCUE_GIT_TIMEOUTS_IN = %w[show edit history diff pages].freeze
+
   included do
     before_action { respond_to :html }
 
@@ -38,6 +40,12 @@ module WikiActions
       feature: :track_unique_wiki_page_views, feature_default_enabled: true
 
     helper_method :view_file_button, :diff_file_html_data
+
+    rescue_from ::Gitlab::Git::CommandTimedOut do |exc|
+      raise exc unless RESCUE_GIT_TIMEOUTS_IN.include?(action_name)
+
+      render 'shared/wikis/git_error'
+    end
   end
 
   def new
@@ -46,11 +54,7 @@ module WikiActions
 
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def pages
-    @wiki_pages = Kaminari.paginate_array(
-      wiki.list_pages(sort: params[:sort], direction: params[:direction])
-    ).page(params[:page])
-
-    @wiki_entries = WikiDirectory.group_pages(@wiki_pages)
+    @wiki_entries = WikiDirectory.group_pages(wiki_pages)
 
     render 'shared/wikis/pages'
   end
@@ -225,9 +229,19 @@ module WikiActions
     unless @sidebar_page # Fallback to default sidebar
       @sidebar_wiki_entries, @sidebar_limited = wiki.sidebar_entries
     end
+  rescue ::Gitlab::Git::CommandTimedOut => e
+    @sidebar_error = e
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
+  def wiki_pages
+    strong_memoize(:wiki_pages) do
+      Kaminari.paginate_array(
+        wiki.list_pages(sort: params[:sort], direction: params[:direction])
+      ).page(params[:page])
+    end
+  end
+
   def wiki_params
     params.require(:wiki).permit(:title, :content, :format, :message, :last_commit_sha)
   end

app/views/shared/_alert_info.html.haml

+.gl-alert.gl-alert-info
+  = sprite_icon('information-o', css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
+  %button.js-close.gl-alert-dismiss{ type: 'button', 'aria-label' => _('Dismiss') }
+    = sprite_icon('close', css_class: 'gl-icon')
+  .gl-alert-body
+    = body

app/views/shared/wikis/_sidebar.html.haml

@@ -10,11 +10,14 @@
           = sprite_icon('download', css_class: 'gl-mr-2')
           %span= _("Clone repository")
 
+    - if @sidebar_error.present?
+      = render 'shared/alert_info', body: s_('Wiki|The sidebar failed to load. You can reload the page to try again.')
+
     .blocks-container
       .block.block-first.w-100
         - if @sidebar_page
           = render_wiki_content(@sidebar_page)
-        - else
+        - elsif @sidebar_wiki_entries
           %ul.wiki-pages
             = render @sidebar_wiki_entries, context: 'sidebar'
       .block.w-100

app/views/shared/wikis/git_error.html.haml

+- if @page
+  - wiki_page_title @page
+
+- add_page_specific_style 'page_bundles/wiki'
+
+- git_access_url = wiki_path(@wiki, action: :git_access)
+
+.wiki-page-header.top-area.gl-flex-direction-column.gl-lg-flex-direction-row
+  .gl-mt-5.gl-mb-3
+    .gl-display-flex.gl-justify-content-space-between
+      %h2.gl-mt-0.gl-mb-5{ data: { qa_selector: 'wiki_page_title', testid: 'wiki_page_title' } }= @page ? @page.human_title : _('Failed to retrieve page')
+    .js-wiki-page-content.md.gl-pt-2{ data: { qa_selector: 'wiki_page_content', testid: 'wiki_page_content' } }
+      = _('The page could not be displayed because it timed out.')
+      = html_escape(_('You can view the source or %{linkStart}%{cloneIcon} clone the repository%{linkEnd}')) % { linkStart: "<a href=\"#{git_access_url}\">".html_safe, linkEnd: '</a>'.html_safe, cloneIcon: sprite_icon('download', css_class: 'gl-mr-2').html_safe }

changelogs/unreleased/ajk-catch-wiki-timeouts.yml

+---
+title: Catch wiki timeouts when rendering pages
+merge_request: 46627
+author:
+type: fixed

lib/gitlab/git.rb

@@ -17,6 +17,7 @@ module Gitlab
     CommitError = Class.new(BaseError)
     OSError = Class.new(BaseError)
     UnknownRef = Class.new(BaseError)
+    CommandTimedOut = Class.new(CommandError)
 
     class << self
       include Gitlab::EncodingHelper

lib/gitlab/git/wraps_gitaly_errors.rb

@@ -9,6 +9,8 @@ module Gitlab
         raise Gitlab::Git::Repository::NoRepository.new(e)
       rescue GRPC::InvalidArgument => e
         raise ArgumentError.new(e)
+      rescue GRPC::DeadlineExceeded => e
+        raise Gitlab::Git::CommandTimedOut.new(e)
       rescue GRPC::BadStatus => e
         raise Gitlab::Git::CommandError.new(e)
       end

locale/gitlab.pot

@@ -11450,6 +11450,9 @@ msgstr ""
 msgid "Failed to reset key. Please try again."
 msgstr ""
 
+msgid "Failed to retrieve page"
+msgstr ""
+
 msgid "Failed to save merge conflicts resolutions. Please try again!"
 msgstr ""
 
@@ -27034,6 +27037,9 @@ msgstr ""
 msgid "The number of times an upload record could not find its file"
 msgstr ""
 
+msgid "The page could not be displayed because it timed out."
+msgstr ""
+
 msgid "The parent epic is confidential and can only contain confidential epics and issues"
 msgstr ""
 
@@ -30713,6 +30719,9 @@ msgstr ""
 msgid "Wiki|Pages"
 msgstr ""
 
+msgid "Wiki|The sidebar failed to load. You can reload the page to try again."
+msgstr ""
+
 msgid "Wiki|Title"
 msgstr ""
 
@@ -31013,6 +31022,9 @@ msgstr ""
 msgid "You can test your .gitlab-ci.yml in %{linkStart}CI Lint%{linkEnd}."
 msgstr ""
 
+msgid "You can view the source or %{linkStart}%{cloneIcon} clone the repository%{linkEnd}"
+msgstr ""
+
 msgid "You cannot access the raw file. Please wait a minute."
 msgstr ""
 

spec/support/shared_examples/controllers/wiki_actions_shared_examples.rb

@@ -14,6 +14,22 @@ RSpec.shared_examples 'wiki controller actions' do
     sign_in(user)
   end
 
+  shared_examples 'recovers from git timeout' do
+    let(:method_name) { :page }
+
+    context 'when we encounter git command errors' do
+      it 'renders the appropriate template', :aggregate_failures do
+        expect(controller).to receive(method_name) do
+          raise ::Gitlab::Git::CommandTimedOut, 'Deadline Exceeded'
+        end
+
+        request
+
+        expect(response).to render_template('shared/wikis/git_error')
+      end
+    end
+  end
+
   describe 'GET #new' do
     subject(:request) { get :new, params: routing_params }
 
@@ -48,6 +64,12 @@ RSpec.shared_examples 'wiki controller actions' do
       get :pages, params: routing_params.merge(id: wiki_title)
     end
 
+    it_behaves_like 'recovers from git timeout' do
+      subject(:request) { get :pages, params: routing_params.merge(id: wiki_title) }
+
+      let(:method_name) { :wiki_pages }
+    end
+
     it 'assigns the page collections' do
       expect(assigns(:wiki_pages)).to contain_exactly(an_instance_of(WikiPage))
       expect(assigns(:wiki_entries)).to contain_exactly(an_instance_of(WikiPage))
@@ -99,6 +121,12 @@ RSpec.shared_examples 'wiki controller actions' do
       end
     end
 
+    it_behaves_like 'recovers from git timeout' do
+      subject(:request) { get :history, params: routing_params.merge(id: wiki_title) }
+
+      let(:allow_read_wiki)   { true }
+    end
+
     it_behaves_like 'fetching history', :ok do
       let(:allow_read_wiki)   { true }
 
@@ -139,6 +167,10 @@ RSpec.shared_examples 'wiki controller actions' do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    it_behaves_like 'recovers from git timeout' do
+      subject(:request) { get :diff, params: routing_params.merge(id: wiki_title, version_id: wiki.repository.commit.id) }
+    end
   end
 
   describe 'GET #show' do
@@ -151,6 +183,8 @@ RSpec.shared_examples 'wiki controller actions' do
     context 'when page exists' do
       let(:id) { wiki_title }
 
+      it_behaves_like 'recovers from git timeout'
+
       it 'renders the page' do
         request
 
@@ -161,6 +195,28 @@ RSpec.shared_examples 'wiki controller actions' do
         expect(assigns(:sidebar_limited)).to be(false)
       end
 
+      context 'the sidebar fails to load' do
+        before do
+          allow(Wiki).to receive(:for_container).and_return(wiki)
+          wiki.wiki
+          expect(wiki).to receive(:find_sidebar) do
+            raise ::Gitlab::Git::CommandTimedOut, 'Deadline Exceeded'
+          end
+        end
+
+        it 'renders the page, and marks the sidebar as failed' do
+          request
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template('shared/wikis/_sidebar')
+          expect(assigns(:page).title).to eq(wiki_title)
+          expect(assigns(:sidebar_page)).to be_nil
+          expect(assigns(:sidebar_wiki_entries)).to be_nil
+          expect(assigns(:sidebar_limited)).to be_nil
+          expect(assigns(:sidebar_error)).to be_a_kind_of(::Gitlab::Git::CommandError)
+        end
+      end
+
       context 'page view tracking' do
         it_behaves_like 'tracking unique hll events', :track_unique_wiki_page_views do
           let(:target_id) { 'wiki_action' }
@@ -308,6 +364,7 @@ RSpec.shared_examples 'wiki controller actions' do
     subject(:request) { get(:edit, params: routing_params.merge(id: id_param)) }
 
     it_behaves_like 'edit action'
+    it_behaves_like 'recovers from git timeout'
 
     context 'when page content encoding is valid' do
       render_views

spec/views/shared/wikis/_sidebar.html.haml_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe 'shared/wikis/_sidebar.html.haml' do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:wiki) { Wiki.for_container(project, project.default_owner) }
+
+  before do
+    assign(:wiki, wiki)
+    assign(:project, project)
+  end
+
+  it 'includes a link to clone the repository' do
+    render
+
+    expect(rendered).to have_link('Clone repository')
+  end
+
+  context 'the wiki is not a project wiki' do
+    it 'does not include the clone repository link' do
+      allow(wiki).to receive(:container).and_return(create(:group))
+
+      render
+
+      expect(rendered).not_to have_link('Clone repository')
+    end
+  end
+
+  context 'the sidebar failed to load' do
+    before do
+      assign(:sidebar_error, Object.new)
+    end
+
+    it 'reports this to the user' do
+      render
+
+      expect(rendered).to include('The sidebar failed to load')
+      expect(rendered).to have_css('.gl-alert.gl-alert-info')
+    end
+  end
+
+  context 'The sidebar comes from a custom page' do
+    before do
+      assign(:sidebar_page, double('WikiPage', path: 'sidebar.md', slug: 'sidebar', content: 'Some sidebar content'))
+    end
+
+    it 'does not show an alert' do
+      render
+
+      expect(rendered).not_to include('The sidebar failed to load')
+      expect(rendered).not_to have_css('.gl-alert.gl-alert-info')
+    end
+
+    it 'renders the wiki content' do
+      render
+
+      expect(rendered).to include('Some sidebar content')
+    end
+  end
+
+  context 'The sidebar comes a list of wiki pages' do
+    before do
+      assign(:sidebar_wiki_entries, create_list(:wiki_page, 3, wiki: wiki))
+      assign(:sidebar_limited, true)
+      stub_template "../shared/wikis/_wiki_pages.html.erb" => "Entries: <%= @sidebar_wiki_entries.size %>"
+      stub_template "../shared/wikis/_wiki_page.html.erb" => 'A WIKI PAGE'
+    end
+
+    it 'does not show an alert' do
+      render
+
+      expect(rendered).not_to include('The sidebar failed to load')
+      expect(rendered).not_to have_css('.gl-alert.gl-alert-info')
+    end
+
+    it 'renders the wiki content' do
+      render
+
+      expect(rendered).to include('A WIKI PAGE' * 3)
+      expect(rendered).to have_link('View All Pages')
+    end
+
+    context 'there is no more to see' do
+      it 'does not invite the user to view more' do
+        assign(:sidebar_limited, false)
+
+        render
+
+        expect(rendered).not_to have_link('View All Pages')
+      end
+    end
+  end
+end