application controller: clean up exception handling.

* Simplify code.
* Show backtraces for all users, not just builders.
* Show backtraces only for unexpected server errors (status 5xx), not
  for normal client errors (status 4xx).
* Log expected errors at info level (reduce noise in production logs).
This commit is contained in:
evazion
2019-08-13 21:30:20 -05:00
parent 84d311f366
commit e70cae457d
7 changed files with 32 additions and 40 deletions

View File

@@ -73,21 +73,19 @@ class ApplicationController < ActionController::Base
end end
def rescue_exception(exception) def rescue_exception(exception)
@exception = exception
case exception case exception
when ActiveRecord::QueryCanceled when ActiveRecord::QueryCanceled
render_error_page(500, exception, message: "The database timed out running your query.") render_error_page(500, exception, message: "The database timed out running your query.")
when ActionController::BadRequest when ActionController::BadRequest
render_error_page(400, exception, expected: true) render_error_page(400, exception)
when ActiveRecord::RecordNotFound when ActiveRecord::RecordNotFound
render_error_page(404, exception, message: "That record was not found", expected: true) render_error_page(404, exception, message: "That record was not found.")
when ActionController::RoutingError when ActionController::RoutingError
render_error_page(405, exception, expected: true) render_error_page(405, exception)
when ActionController::UnknownFormat, ActionView::MissingTemplate when ActionController::UnknownFormat, ActionView::MissingTemplate
render_error_page(406, exception, message: "#{request.format.to_s} is not a supported format for this page", html: true, expected: true) render_error_page(406, exception, message: "#{request.format.to_s} is not a supported format for this page", format: :html)
when Danbooru::Paginator::PaginationError when Danbooru::Paginator::PaginationError
render_error_page(410, exception, expected: true) render_error_page(410, exception)
when NotImplementedError when NotImplementedError
render_error_page(501, exception, message: "This feature isn't available: #{exception.message}") render_error_page(501, exception, message: "This feature isn't available: #{exception.message}")
when PG::ConnectionBad when PG::ConnectionBad
@@ -97,16 +95,15 @@ class ApplicationController < ActionController::Base
end end
end end
def render_error_page(status, exception, message: exception.message, html: false, expected: false) def render_error_page(status, exception, message: exception.message, format: request.format.symbol)
@error_message = message @exception = exception
@expected = status < 500
@message = message.encode("utf-8", { invalid: :replace, undef: :replace })
@backtrace = Rails.backtrace_cleaner.clean(@exception.backtrace)
format = :html unless format.in?(%i[html json xml js atom])
DanbooruLogger.log(exception, expected: expected) DanbooruLogger.log(@exception, expected: @expected)
render "static/error", status: status, formats: format
if html || !request.format.symbol.in?(%i[html json xml js atom])
render template: "static/error.html", status: status
else
render template: "static/error", status: status
end
end end
def authentication_failed def authentication_failed

View File

@@ -1,6 +1,8 @@
class DanbooruLogger class DanbooruLogger
def self.log(exception, expected: false, **params) def self.log(exception, expected: false, **params)
if !expected if expected
Rails.logger.info("#{exception.class}: #{exception.message}")
else
backtrace = Rails.backtrace_cleaner.clean(exception.backtrace).join("\n") backtrace = Rails.backtrace_cleaner.clean(exception.backtrace).join("\n")
Rails.logger.error("#{exception.class}: #{exception.message}\n#{backtrace}") Rails.logger.error("#{exception.class}: #{exception.message}\n#{backtrace}")
end end

View File

@@ -1,6 +1,10 @@
<%# backtrace %> <%# backtrace %>
<ul class="backtrace"> <ul class="backtrace">
<% if exception.present? %>
<li><%= exception.class.to_s %> exception raised</li>
<% end %>
<% Rails.backtrace_cleaner.clean(backtrace).each do |b| %> <% Rails.backtrace_cleaner.clean(backtrace).each do |b| %>
<li class="backtrace-line"><%= b %></li> <li class="backtrace-line"><%= b %></li>
<% end %> <% end %>

View File

@@ -1,10 +1,7 @@
<% if CurrentUser.user.try(:is_builder?) && @exception.present? %> <h1>Error</h1>
<h1><%= @exception.class.to_s %> exception raised</h1> <p><%= @message %></p>
<p><%= @exception.message.dup.force_encoding("utf-8") %></p> <% unless @expected %>
<%= render "static/backtrace", backtrace: @exception.backtrace %> <h6>Details</h6>
<% elsif @error_message %> <%= render "static/backtrace", exception: @exception, backtrace: @backtrace %>
<p><%= @error_message %></p>
<% else %>
<p><%= @exception.message.dup.force_encoding("utf-8") %></p>
<% end %> <% end %>

View File

@@ -1,2 +1,2 @@
var message = <%= raw @error_message.try(:to_json) || @exception.message.to_json %>; var message = <%= raw @message.to_json %>;
Danbooru.Utility.error(message); Danbooru.Utility.error(message);

View File

@@ -1,9 +1,5 @@
<% if @error_message %> {
{"success": false, "message": <%= raw @error_message.encode("utf-8", {:invalid => :replace, :undef => :replace, :replace => "?"}).to_json %>} "success": false,
<% else %> "message": <%= raw @message.to_json %>,
{ "backtrace": <%= raw @backtrace.to_json %>
"success": false, }
"message": <%= raw @exception.to_s.encode("utf-8", {:invalid => :replace, :undef => :replace, :replace => "?"}).to_json %>,
"backtrace": <%= raw Rails.backtrace_cleaner.clean(@exception.backtrace).to_json %>
}
<% end %>

View File

@@ -1,6 +1,2 @@
<?xml version="1.0" encoding="UTF-8"?> <?xml version="1.0" encoding="UTF-8"?>
<% if @error_message %> <result success="false"><%= @message %></result>
<result success="false"><%= @error_message %></result>
<% else %>
<result success="false"><%= @exception.to_s %></result>
<% end %>