diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index bfd494a02..daee00722 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -112,11 +112,11 @@ class ApplicationController < ActionController::Base
when ActiveRecord::QueryCanceled
render_error_page(500, exception, template: "static/search_timeout", message: "The database timed out running your query.")
when ActionController::BadRequest
- render_error_page(400, exception)
+ render_error_page(400, exception, message: exception.message)
when SessionLoader::AuthenticationFailure
- render_error_page(401, exception, template: "sessions/new")
+ render_error_page(401, exception, message: exception.message, template: "sessions/new")
when ActionController::InvalidAuthenticityToken, ActionController::UnpermittedParameters, ActionController::InvalidCrossOriginRequest, ActionController::Redirecting::UnsafeRedirectError
- render_error_page(403, exception)
+ render_error_page(403, exception, message: exception.message)
when ActiveSupport::MessageVerifier::InvalidSignature, # raised by `find_signed!`
User::PrivilegeError,
Pundit::NotAuthorizedError
@@ -124,7 +124,7 @@ class ApplicationController < ActionController::Base
when ActiveRecord::RecordNotFound
render_error_page(404, exception, message: "That record was not found.")
when ActionController::RoutingError
- render_error_page(405, exception)
+ render_error_page(405, exception, message: exception.message)
when ActionController::UnknownFormat, ActionView::MissingTemplate
render_error_page(406, exception, message: "#{request.format} is not a supported format for this page")
when PaginationExtension::PaginationError
@@ -132,7 +132,7 @@ class ApplicationController < ActionController::Base
when PostQueryBuilder::TagLimitError
render_error_page(422, exception, template: "static/tag_limit_error", message: "You cannot search for more than #{CurrentUser.tag_query_limit} tags at a time.")
when RateLimiter::RateLimitError
- render_error_page(429, exception)
+ render_error_page(429, exception, message: "Rate limit exceeded. You're doing that too fast")
when Rack::Timeout::RequestTimeoutException
render_error_page(500, exception, message: "Your request took too long to complete and was canceled.")
when NotImplementedError
@@ -140,18 +140,20 @@ class ApplicationController < ActionController::Base
when PG::ConnectionBad
render_error_page(503, exception, message: "The database is unavailable. Try again later.")
else
- raise exception if !Rails.env.production? || Danbooru.config.debug_mode
+ raise exception if Rails.env.development? || Danbooru.config.debug_mode
render_error_page(500, exception)
end
end
- def render_error_page(status, exception = nil, message: exception.message, template: "static/error", format: request.format.symbol)
+ def render_error_page(status, exception = nil, message: "", template: "static/error", format: request.format.symbol)
@exception = exception
@expected = status < 500
- @message = message.encode("utf-8", invalid: :replace, undef: :replace)
+ @message = message.to_s.encode("utf-8", invalid: :replace, undef: :replace)
@backtrace = Rails.backtrace_cleaner.clean(@exception.backtrace) if @exception
format = :html unless format.in?(%i[html json xml js atom])
+ @api_response = { success: false, error: @exception.class.to_s, message: @message, backtrace: @backtrace }
+
# if InvalidAuthenticityToken was raised, CurrentUser isn't set so we have to use the blank layout.
layout = CurrentUser.user.present? ? "default" : "blank"
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index be32d8303..7a5b118bf 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -22,7 +22,7 @@ class SessionsController < ApplicationController
respond_with(user, location: url)
else
flash.now[:notice] = "Password was incorrect"
- raise SessionLoader::AuthenticationFailure
+ raise SessionLoader::AuthenticationFailure, "Username or password incorrect"
end
end
diff --git a/app/logical/session_loader.rb b/app/logical/session_loader.rb
index 74db0c2c7..3cdbcb724 100644
--- a/app/logical/session_loader.rb
+++ b/app/logical/session_loader.rb
@@ -109,7 +109,7 @@ class SessionLoader
elsif params[:login].present? && params[:api_key].present?
authenticate_api_key(params[:login], params[:api_key])
else
- raise AuthenticationFailure
+ raise AuthenticationFailure, "Missing `login` or `api_key`"
end
end
@@ -129,7 +129,7 @@ class SessionLoader
# permissions for this endpoint
def authenticate_api_key(name, key)
user, api_key = User.find_by_name(name)&.authenticate_api_key(key)
- raise AuthenticationFailure if user.blank?
+ raise AuthenticationFailure, "Invalid API key" if user.blank?
update_api_key(api_key)
raise User::PrivilegeError if !api_key.has_permission?(request.remote_ip, request.params[:controller], request.params[:action])
CurrentUser.user = user
diff --git a/app/views/static/_backtrace.html.erb b/app/views/static/_backtrace.html.erb
deleted file mode 100644
index 149fcc852..000000000
--- a/app/views/static/_backtrace.html.erb
+++ /dev/null
@@ -1,11 +0,0 @@
-<%# backtrace %>
-
-
- <% if defined?(exception) %>
- - <%= exception.class.to_s %> exception raised
- <% end %>
-
- <% Rails.backtrace_cleaner.clean(backtrace).each do |b| %>
- - <%= b %>
- <% end %>
-
diff --git a/app/views/static/error.atom.erb b/app/views/static/error.atom.erb
index e69de29bb..70da56c18 100644
--- a/app/views/static/error.atom.erb
+++ b/app/views/static/error.atom.erb
@@ -0,0 +1 @@
+<%= raw @api_response.to_xml(root: "result") %>
diff --git a/app/views/static/error.html.erb b/app/views/static/error.html.erb
index 8aa90dc1f..9a2fd3835 100644
--- a/app/views/static/error.html.erb
+++ b/app/views/static/error.html.erb
@@ -1,9 +1,19 @@
-<% page_title "Error: #{@message}" %>
+<% page_title "Error: #{@exception.class}" %>
Error
-<%= @message %>
-<% unless @expected %>
+<% if @message.present? %>
+ <%= @message %>
+<% else %>
+ Unexpected error: <%= @exception.class %>.
+
Details
- <%= render "static/backtrace", exception: @exception, backtrace: @backtrace %>
+
+
+ - <%= @exception.class.to_s %> exception raised
+
+ <% Rails.backtrace_cleaner.clean(@backtrace).each do |b| %>
+ - <%= b %>
+ <% end %>
+
<% end %>
diff --git a/app/views/static/error.js.erb b/app/views/static/error.js.erb
index a9acdfc58..22fa986fa 100644
--- a/app/views/static/error.js.erb
+++ b/app/views/static/error.js.erb
@@ -1,10 +1,7 @@
-var klass = <%= raw @exception.class.to_s.to_json %>;
-var message = <%= raw @message.to_json %>;
-var backtrace = <%= raw @backtrace.to_json %>;
+console.error(<%= raw @api_response.to_json %>);
-<% if @expected %>
- Danbooru.Utility.error(message);
+<% if @message.present? %>
+ Danbooru.Utility.error("<%= j @message %>");
<% else %>
- console.error(klass, message, backtrace);
- Danbooru.Utility.error(message);
+ Danbooru.Utility.error("<%= j "Unexpected error: #{@exception.class}" %>");
<% end %>
diff --git a/app/views/static/error.json.erb b/app/views/static/error.json.erb
index 3d730d214..9dbf8ab9f 100644
--- a/app/views/static/error.json.erb
+++ b/app/views/static/error.json.erb
@@ -1,5 +1 @@
-{
- "success": false,
- "message": <%= raw @message.to_json %>,
- "backtrace": <%= raw @backtrace.to_json %>
-}
+<%= raw @api_response.to_json %>
diff --git a/app/views/static/error.xml.erb b/app/views/static/error.xml.erb
index 4bb805938..70da56c18 100644
--- a/app/views/static/error.xml.erb
+++ b/app/views/static/error.xml.erb
@@ -1,2 +1 @@
-
-<%= @message %>
+<%= raw @api_response.to_xml(root: "result") %>
diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb
index e5179a36c..53ff6a86e 100644
--- a/test/functional/application_controller_test.rb
+++ b/test/functional/application_controller_test.rb
@@ -36,6 +36,45 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
end
end
+ context "on an unexpected error" do
+ setup do
+ User.stubs(:find).raises(NoMethodError.new("pwned"))
+ @user = create(:user)
+ end
+
+ should "not return the error message in the HTML response" do
+ get user_path(@user)
+
+ assert_response 500
+ assert_match(/NoMethodError/, response.body.to_s)
+ assert_no_match(/pwned/, response.body.to_s)
+ end
+
+ should "not return the error message in the JSON response" do
+ get user_path(@user, format: :json)
+
+ assert_response 500
+ assert_match(/NoMethodError/, response.body.to_s)
+ assert_no_match(/pwned/, response.body.to_s)
+ end
+
+ should "not return the error message in the XML response" do
+ get user_path(@user, format: :xml)
+
+ assert_response 500
+ assert_match(/NoMethodError/, response.body.to_s)
+ assert_no_match(/pwned/, response.body.to_s)
+ end
+
+ should "not return the error message in the JS response" do
+ get user_path(@user, format: :js)
+
+ assert_response 500
+ assert_match(/NoMethodError/, response.body.to_s)
+ assert_no_match(/pwned/, response.body.to_s)
+ end
+ end
+
context "on api authentication" do
setup do
@user = create(:user, password: "password")
diff --git a/test/functional/static_controller_test.rb b/test/functional/static_controller_test.rb
index 13b99b53c..77f1332a4 100644
--- a/test/functional/static_controller_test.rb
+++ b/test/functional/static_controller_test.rb
@@ -68,7 +68,7 @@ class StaticControllerTest < ActionDispatch::IntegrationTest
get "/qwoiqogieqg", as: :xml
assert_response 404
- assert_equal("Page not found", response.parsed_body.at("result").text)
+ assert_equal("Page not found", response.parsed_body.xpath("result/message").text)
end
should "render the 404 page when page_not_found_pool_id is configured" do