From 7fa292870159c4cae87ab1952575aeff9400dd85 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Wed, 11 Jan 2023 12:54:32 -0800 Subject: [PATCH 1/6] Add GracefulTimeoutHandling and basic rescue --- app/controllers/concerns/issuable_collections_action.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/controllers/concerns/issuable_collections_action.rb b/app/controllers/concerns/issuable_collections_action.rb index b8249345a54819..f293ffd215c0c2 100644 --- a/app/controllers/concerns/issuable_collections_action.rb +++ b/app/controllers/concerns/issuable_collections_action.rb @@ -2,6 +2,7 @@ module IssuableCollectionsAction extend ActiveSupport::Concern + include GracefulTimeoutHandling include IssuableCollections include IssuesCalendar @@ -33,6 +34,11 @@ def merge_requests @merge_requests = issuables_collection.page(params[:page]) @issuable_meta_data = Gitlab::IssuableMetadata.new(current_user, @merge_requests).data + rescue ActiveRecord::QueryCanceled # rubocop:disable Database/RescueQueryCanceled + # Kerri, maybe you could return something different here so we know this isn't + # an empty result, but an overstuffed one? + # + @merge_requests = [] end # rubocop:enable Gitlab/ModuleWithInstanceVariables -- GitLab From ab27600f3c2af3e0d6ceab03e34d4ba6f576f5e5 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Thu, 12 Jan 2023 15:55:36 -0800 Subject: [PATCH 2/6] Set an ivar to indicate a query timeout occurred --- .../concerns/issuable_collections_action.rb | 9 ++++--- spec/controllers/dashboard_controller_spec.rb | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/app/controllers/concerns/issuable_collections_action.rb b/app/controllers/concerns/issuable_collections_action.rb index f293ffd215c0c2..13298b9065de84 100644 --- a/app/controllers/concerns/issuable_collections_action.rb +++ b/app/controllers/concerns/issuable_collections_action.rb @@ -34,11 +34,10 @@ def merge_requests @merge_requests = issuables_collection.page(params[:page]) @issuable_meta_data = Gitlab::IssuableMetadata.new(current_user, @merge_requests).data - rescue ActiveRecord::QueryCanceled # rubocop:disable Database/RescueQueryCanceled - # Kerri, maybe you could return something different here so we know this isn't - # an empty result, but an overstuffed one? - # - @merge_requests = [] + rescue ActiveRecord::QueryCanceled => exception # rubocop:disable Database/RescueQueryCanceled + log_exception(exception) + + @search_timeout_occurred = true end # rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index ea12b0c5ad795b..2158bfb006e25c 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -46,6 +46,30 @@ describe 'GET merge requests' do it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests it_behaves_like 'issuables requiring filter', :merge_requests + + context 'when an ActiveRecord::QueryCanceled is raised' do + before do + allow_next_instance_of(Gitlab::IssuableMetadata) do |instance| + allow(instance).to receive(:data).and_raise(ActiveRecord::QueryCanceled) + end + end + + it 'sets :search_timeout_occurred' do + get :merge_requests, params: { author_id: user.id } + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:search_timeout_occurred)).to eq(true) + end + + it 'logs the exception' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original + + get :merge_requests, params: { author_id: user.id } + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:search_timeout_occurred)).to eq(true) + end + end end end -- GitLab From 78528f0a833eae1c3a1e41168f76c66a6f48c45a Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Tue, 17 Jan 2023 15:17:15 -0800 Subject: [PATCH 3/6] Render search timeout partial on error --- app/views/dashboard/merge_requests.html.haml | 2 ++ .../dashboard/_search_timeout_occurred.html.haml | 8 ++++++++ spec/controllers/dashboard_controller_spec.rb | 10 ++++++++++ 3 files changed, 20 insertions(+) create mode 100644 app/views/shared/dashboard/_search_timeout_occurred.html.haml diff --git a/app/views/dashboard/merge_requests.html.haml b/app/views/dashboard/merge_requests.html.haml index c921375edd1b7b..97fb35b28ab0ca 100644 --- a/app/views/dashboard/merge_requests.html.haml +++ b/app/views/dashboard/merge_requests.html.haml @@ -19,5 +19,7 @@ - if current_user && @no_filters_set = render 'shared/dashboard/no_filter_selected' +- elsif @search_timeout_occurred + = render 'shared/dashboard/search_timeout_occurred' - else = render 'shared/merge_requests' diff --git a/app/views/shared/dashboard/_search_timeout_occurred.html.haml b/app/views/shared/dashboard/_search_timeout_occurred.html.haml new file mode 100644 index 00000000000000..61c1bd773e8aff --- /dev/null +++ b/app/views/shared/dashboard/_search_timeout_occurred.html.haml @@ -0,0 +1,8 @@ +.row.empty-state.text-center + .col-12 + .svg-130.gl-mt-3 + = image_tag 'illustrations/issue-dashboard_results-without-filter.svg' + .col-12 + .text-content + %h4 + = _("Too many results to display") diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 2158bfb006e25c..08fd9d6481fb1e 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -61,6 +61,16 @@ expect(assigns(:search_timeout_occurred)).to eq(true) end + context 'rendering views' do + render_views + + it 'shows error message' do + get :merge_requests, params: { author_id: user.id } + + expect(response.body).to have_content('Too many results to display') + end + end + it 'logs the exception' do expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original -- GitLab From 8f4c7d40c07803a7222a18a2a9542df53aa07adb Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Tue, 17 Jan 2023 15:38:49 -0800 Subject: [PATCH 4/6] Tightens up test by removing extraneous expects --- spec/controllers/dashboard_controller_spec.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 08fd9d6481fb1e..70b914c2e74981 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -75,9 +75,6 @@ expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original get :merge_requests, params: { author_id: user.id } - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:search_timeout_occurred)).to eq(true) end end end -- GitLab From 9d690e7a8fe6bfb2a6a4a56043736b21534e5634 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Wed, 18 Jan 2023 09:15:09 -0800 Subject: [PATCH 5/6] Update timeout error message --- app/views/shared/dashboard/_search_timeout_occurred.html.haml | 2 +- spec/controllers/dashboard_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/shared/dashboard/_search_timeout_occurred.html.haml b/app/views/shared/dashboard/_search_timeout_occurred.html.haml index 61c1bd773e8aff..4b6331ee986565 100644 --- a/app/views/shared/dashboard/_search_timeout_occurred.html.haml +++ b/app/views/shared/dashboard/_search_timeout_occurred.html.haml @@ -5,4 +5,4 @@ .col-12 .text-content %h4 - = _("Too many results to display") + = _("Too many results to display. Edit your search or add a filter.") diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 70b914c2e74981..d19145058e78d2 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -67,7 +67,7 @@ it 'shows error message' do get :merge_requests, params: { author_id: user.id } - expect(response.body).to have_content('Too many results to display') + expect(response.body).to have_content('Too many results to display. Edit your search or add a filter.') end end -- GitLab From b034f8a2fd70640567270fb3cda257b4d90054b7 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Wed, 18 Jan 2023 10:31:27 -0800 Subject: [PATCH 6/6] Add generated string --- locale/gitlab.pot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1ff9b6586f2656..5e9450effd43c9 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44211,6 +44211,9 @@ msgstr "" msgid "Too many references. Quick actions are limited to at most %{max_count} user references" msgstr "" +msgid "Too many results to display. Edit your search or add a filter." +msgstr "" + msgid "Too many users found. Quick actions are limited to at most %{max_count} users" msgstr "" -- GitLab