Split ShowExceptions responsibilities in two middlewares.

This commit is contained in:
José Valim 2011-12-01 20:46:18 +01:00
parent 956ecff833
commit 750bb5c865
10 changed files with 132 additions and 97 deletions

View File

@ -51,6 +51,7 @@ module ActionDispatch
autoload :BestStandardsSupport
autoload :Callbacks
autoload :Cookies
autoload :DebugExceptions
autoload :ExceptionWrapper
autoload :Flash
autoload :Head

View File

@ -0,0 +1,77 @@
require 'action_dispatch/http/request'
require 'action_dispatch/middleware/exception_wrapper'
module ActionDispatch
# This middleware is responsible for logging exceptions and
# showing a debugging page in case the request is local.
class DebugExceptions
RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates')
def initialize(app)
@app = app
end
def call(env)
begin
response = @app.call(env)
if response[1]['X-Cascade'] == 'pass'
raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}"
end
rescue Exception => exception
raise exception if env['action_dispatch.show_exceptions'] == false
end
response ? response : render_exception(env, exception)
end
private
def render_exception(env, exception)
wrapper = ExceptionWrapper.new(env, exception)
log_error(env, wrapper)
if env['action_dispatch.show_detailed_exceptions']
template = ActionView::Base.new([RESCUES_TEMPLATE_PATH],
:request => Request.new(env),
:exception => wrapper.exception,
:application_trace => wrapper.application_trace,
:framework_trace => wrapper.framework_trace,
:full_trace => wrapper.full_trace
)
file = "rescues/#{wrapper.rescue_template}"
body = template.render(:template => file, :layout => 'rescues/layout')
render(wrapper.status_code, body)
else
raise exception
end
end
def render(status, body)
[status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]]
end
def log_error(env, wrapper)
logger = logger(env)
return unless logger
exception = wrapper.exception
ActiveSupport::Deprecation.silence do
message = "\n#{exception.class} (#{exception.message}):\n"
message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
message << " " << wrapper.application_trace.join("\n ")
logger.fatal("#{message}\n\n")
end
end
def logger(env)
env['action_dispatch.logger'] || stderr_logger
end
def stderr_logger
@stderr_logger ||= Logger.new($stderr)
end
end
end

View File

@ -1,3 +1,4 @@
require 'action_controller/metal/exceptions'
require 'active_support/core_ext/exception'
module ActionDispatch

View File

@ -1,4 +1,3 @@
require 'action_controller/metal/exceptions'
require 'action_dispatch/http/request'
require 'action_dispatch/middleware/exception_wrapper'
require 'active_support/deprecation'
@ -36,100 +35,47 @@ module ActionDispatch
def call(env)
begin
status, headers, body = @app.call(env)
exception = nil
# Only this middleware cares about RoutingError. So, let's just raise
# it here.
if headers['X-Cascade'] == 'pass'
raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}"
end
response = @app.call(env)
rescue Exception => exception
raise exception if env['action_dispatch.show_exceptions'] == false
end
exception ? render_exception(env, exception) : [status, headers, body]
response ? response : render_exception(env, exception)
end
private
def render_exception(env, exception)
wrapper = ExceptionWrapper.new(env, exception)
log_error(env, wrapper)
if env['action_dispatch.show_detailed_exceptions'] == true
rescue_action_diagnostics(wrapper)
else
rescue_action_error_page(wrapper)
end
rescue Exception => failsafe_error
$stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}"
FAILSAFE_RESPONSE
def render_exception(env, exception)
wrapper = ExceptionWrapper.new(env, exception)
status = wrapper.status_code
locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale
path = "#{public_path}/#{status}.html"
if locale_path && File.exist?(locale_path)
render(status, File.read(locale_path))
elsif File.exist?(path)
render(status, File.read(path))
else
render(status, '')
end
rescue Exception => failsafe_error
render_failsafe(failsafe_error)
end
# Render detailed diagnostics for unhandled exceptions rescued from
# a controller action.
def rescue_action_diagnostics(wrapper)
template = ActionView::Base.new([RESCUES_TEMPLATE_PATH],
:request => Request.new(wrapper.env),
:exception => wrapper.exception,
:application_trace => wrapper.application_trace,
:framework_trace => wrapper.framework_trace,
:full_trace => wrapper.full_trace
)
file = "rescues/#{wrapper.rescue_template}"
body = template.render(:template => file, :layout => 'rescues/layout')
render(wrapper.status_code, body)
end
def render_failsafe(failsafe_error)
$stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}"
FAILSAFE_RESPONSE
end
# Attempts to render a static error page based on the
# <tt>status_code</tt> thrown, or just return headers if no such file
# exists. At first, it will try to render a localized static page.
# For example, if a 500 error is being handled Rails and locale is :da,
# it will first attempt to render the file at <tt>public/500.da.html</tt>
# then attempt to render <tt>public/500.html</tt>. If none of them exist,
# the body of the response will be left empty.
def rescue_action_error_page(wrapper)
status = wrapper.status_code
locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale
path = "#{public_path}/#{status}.html"
def render(status, body)
[status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]]
end
if locale_path && File.exist?(locale_path)
render(status, File.read(locale_path))
elsif File.exist?(path)
render(status, File.read(path))
else
render(status, '')
end
end
def render(status, body)
[status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]]
end
def public_path
defined?(Rails.public_path) ? Rails.public_path : 'public_path'
end
def log_error(env, wrapper)
logger = logger(env)
return unless logger
exception = wrapper.exception
ActiveSupport::Deprecation.silence do
message = "\n#{exception.class} (#{exception.message}):\n"
message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
message << " " << wrapper.application_trace.join("\n ")
logger.fatal("#{message}\n\n")
end
end
def logger(env)
env['action_dispatch.logger'] || stderr_logger
end
def stderr_logger
@stderr_logger ||= Logger.new($stderr)
end
# TODO: Make this a middleware initialization parameter once
# we deprecated the second option
def public_path
defined?(Rails.public_path) ? Rails.public_path : 'public_path'
end
end
end

View File

@ -175,6 +175,7 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase
def self.build_app(routes = nil)
RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware|
middleware.use "ActionDispatch::ShowExceptions"
middleware.use "ActionDispatch::DebugExceptions"
middleware.use "ActionDispatch::Callbacks"
middleware.use "ActionDispatch::ParamsParser"
middleware.use "ActionDispatch::Cookies"
@ -338,16 +339,19 @@ end
module ActionDispatch
class ShowExceptions
private
remove_method :public_path
def public_path
"#{FIXTURE_LOAD_PATH}/public"
end
remove_method :public_path
def public_path
"#{FIXTURE_LOAD_PATH}/public"
end
end
remove_method :stderr_logger
# Silence logger
def stderr_logger
nil
end
class DebugExceptions
private
remove_method :stderr_logger
# Silence logger
def stderr_logger
nil
end
end
end

View File

@ -3,6 +3,7 @@ require 'abstract_unit'
module ShowExceptions
class ShowExceptionsController < ActionController::Base
use ActionDispatch::ShowExceptions
use ActionDispatch::DebugExceptions
def boom
raise 'boom!'

View File

@ -29,8 +29,8 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
end
end
ProductionApp = ActionDispatch::ShowExceptions.new(Boomer.new(false))
DevelopmentApp = ActionDispatch::ShowExceptions.new(Boomer.new(true))
ProductionApp = ActionDispatch::ShowExceptions.new(ActionDispatch::DebugExceptions.new(Boomer.new(false)))
DevelopmentApp = ActionDispatch::ShowExceptions.new(ActionDispatch::DebugExceptions.new(Boomer.new(true)))
test "rescue with error page when show_exceptions is false" do
@app = ProductionApp

View File

@ -195,10 +195,13 @@ module Rails
middleware.use ::ActionDispatch::RequestId
middleware.use ::Rails::Rack::Logger, config.log_tags # must come after Rack::MethodOverride to properly log overridden methods
middleware.use ::ActionDispatch::ShowExceptions
middleware.use ::ActionDispatch::DebugExceptions
middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies
if config.action_dispatch.x_sendfile_header.present?
middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header
end
middleware.use ::ActionDispatch::Reloader unless config.cache_classes
middleware.use ::ActionDispatch::Callbacks
middleware.use ::ActionDispatch::Cookies

View File

@ -3,7 +3,7 @@ require 'isolation/abstract_unit'
require 'rack/test'
module ApplicationTests
class ShowExceptionsTest < Test::Unit::TestCase
class MiddlewareExceptionsTest < Test::Unit::TestCase
include ActiveSupport::Testing::Isolation
include Rack::Test::Methods

View File

@ -33,6 +33,7 @@ module ApplicationTests
"ActionDispatch::RequestId",
"Rails::Rack::Logger", # must come after Rack::MethodOverride to properly log overridden methods
"ActionDispatch::ShowExceptions",
"ActionDispatch::DebugExceptions",
"ActionDispatch::RemoteIp",
"Rack::Sendfile",
"ActionDispatch::Reloader",
@ -104,10 +105,11 @@ module ApplicationTests
assert !middleware.include?("ActionDispatch::Static")
end
test "includes show exceptions even if action_dispatch.show_exceptions is disabled" do
test "includes exceptions middlewares even if action_dispatch.show_exceptions is disabled" do
add_to_config "config.action_dispatch.show_exceptions = false"
boot!
assert middleware.include?("ActionDispatch::ShowExceptions")
assert middleware.include?("ActionDispatch::DebugExceptions")
end
test "removes ActionDispatch::Reloader if cache_classes is true" do