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 <link rel="next"> / <link rel="prev"> 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 <body> tag could fail with an
ActionView::Template::Error because the call to `current_item.present?`
forced evaluation of the search.
This commit is contained in:
evazion
2020-07-03 12:42:04 -05:00
parent e822d5f16e
commit f97c62c71d
6 changed files with 49 additions and 19 deletions

View File

@@ -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

View File

@@ -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)

View File

@@ -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

View File

@@ -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

View File

@@ -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 %>

View File

@@ -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