From f97c62c71daf7d11dcfaaffefadd7519c04b183e Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 3 Jul 2020 12:42:04 -0500 Subject: [PATCH] search: fix search timeout error page not appearing. Bug: when a search timed out we got the generic failbooru page instead of the search timeout error page. Cause: when rendering the / tags in the header, we may need to evaluate the search to determine the next or previous page, but if the searches times out then this fails, which caused Rails to throw a ActionView::Template::Error because an exception was thrown while rendering the template. Likewise, rendering the attributes for the tag could fail with an ActionView::Template::Error because the call to `current_item.present?` forced evaluation of the search. --- app/controllers/application_controller.rb | 2 ++ app/helpers/application_helper.rb | 2 +- app/logical/pagination_extension.rb | 28 ++++++++++++++-------- app/logical/post_sets/post.rb | 2 +- app/views/application/_meta_links.html.erb | 12 ++++------ test/functional/posts_controller_test.rb | 22 +++++++++++++++++ 6 files changed, 49 insertions(+), 19 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a0825ee0d..9afec6b03 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -88,6 +88,8 @@ class ApplicationController < ActionController::Base def rescue_exception(exception) case exception + when ActionView::Template::Error + rescue_exception(exception.cause) when ActiveRecord::QueryCanceled render_error_page(500, exception, template: "static/search_timeout", message: "The database timed out running your query.") when ActionController::BadRequest diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4ad0202d8..26be0501f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -253,7 +253,7 @@ module ApplicationHelper def body_attributes(user, params, current_item = nil) current_user_data_attributes = data_attributes_for(user, "current-user", current_user_attributes) - if current_item.present? && current_item.respond_to?(:html_data_attributes) && current_item.respond_to?(:model_name) + if !current_item.nil? && current_item.respond_to?(:html_data_attributes) && current_item.respond_to?(:model_name) model_name = current_item.model_name.singular.dasherize model_attributes = current_item.html_data_attributes current_item_data_attributes = data_attributes_for(current_item, model_name, model_attributes) diff --git a/app/logical/pagination_extension.rb b/app/logical/pagination_extension.rb index dcb89ee5b..686cdcd27 100644 --- a/app/logical/pagination_extension.rb +++ b/app/logical/pagination_extension.rb @@ -61,27 +61,35 @@ module PaginationExtension end def prev_page - return nil if is_first_page? - - if paginator_mode == :numbered + if is_first_page? + nil + elsif paginator_mode == :numbered current_page - 1 - elsif paginator_mode == :sequential_before + elsif paginator_mode == :sequential_before && records.present? "a#{records.first.id}" - elsif paginator_mode == :sequential_after + elsif paginator_mode == :sequential_after && records.present? "b#{records.last.id}" + else + nil end + rescue ActiveRecord::QueryCanceled + nil end def next_page - return nil if is_last_page? - - if paginator_mode == :numbered + if is_last_page? + nil + elsif paginator_mode == :numbered current_page + 1 - elsif paginator_mode == :sequential_before + elsif paginator_mode == :sequential_before && records.present? "b#{records.last.id}" - elsif paginator_mode == :sequential_after + elsif paginator_mode == :sequential_after && records.present? "a#{records.first.id}" + else + nil end + rescue ActiveRecord::QueryCanceled + nil end # XXX Hack: in sequential pagination we fetch one more record than we diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index b6ed3c379..c13a1f5fb 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -105,7 +105,7 @@ module PostSets if is_random? get_random_posts else - normalized_query.build.paginate(page, count: post_count, search_count: !post_count.nil?, limit: per_page) + normalized_query.build.paginate(page, count: post_count, search_count: !post_count.nil?, limit: per_page).load end end end diff --git a/app/views/application/_meta_links.html.erb b/app/views/application/_meta_links.html.erb index d0ddc7cd4..7b398ed83 100644 --- a/app/views/application/_meta_links.html.erb +++ b/app/views/application/_meta_links.html.erb @@ -1,13 +1,11 @@ <%# collection %> <% content_for(:html_header) do %> - <% if collection.try(:records).present? %> - <% if collection.try(:prev_page) %> - <%= tag.link rel: "prev", href: url_for(nav_params_for(collection.prev_page)) %> - <% end %> + <% if collection.try(:prev_page) %> + <%= tag.link rel: "prev", href: url_for(nav_params_for(collection.prev_page)) %> + <% end %> - <% if collection.try(:next_page) %> - <%= tag.link rel: "next", href: url_for(nav_params_for(collection.next_page)) %> - <% end %> + <% if collection.try(:next_page) %> + <%= tag.link rel: "next", href: url_for(nav_params_for(collection.next_page)) %> <% end %> <% end %> diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index a1c7d3851..65cd5d335 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -380,6 +380,28 @@ class PostsControllerTest < ActionDispatch::IntegrationTest assert_select "title", text: "1girl Art | Safebooru" end end + + context "for a search that times out" do + context "during numbered pagination" do + should "show the search timeout error page" do + Post::const_get(:ActiveRecord_Relation).any_instance.stubs(:records).raises(ActiveRecord::QueryCanceled) + + get posts_path(page: "1") + assert_response 500 + assert_select "h1", text: "Search Timeout" + end + end + + context "during sequential pagination" do + should "show the search timeout error page" do + Post::const_get(:ActiveRecord_Relation).any_instance.stubs(:records).raises(ActiveRecord::QueryCanceled) + + get posts_path(page: "a0") + assert_response 500 + assert_select "h1", text: "Search Timeout" + end + end + end end context "show_seq action" do