From 750bb5c865ac9234da91ec451eec7d9de55b8f9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 1 Dec 2011 20:46:18 +0100 Subject: Split ShowExceptions responsibilities in two middlewares. --- actionpack/lib/action_dispatch.rb | 1 + .../action_dispatch/middleware/debug_exceptions.rb | 77 +++++++++++++++ .../middleware/exception_wrapper.rb | 1 + .../action_dispatch/middleware/show_exceptions.rb | 110 ++++++--------------- actionpack/test/abstract_unit.rb | 22 +++-- actionpack/test/controller/show_exceptions_test.rb | 1 + actionpack/test/dispatch/show_exceptions_test.rb | 4 +- 7 files changed, 123 insertions(+), 93 deletions(-) create mode 100644 actionpack/lib/action_dispatch/middleware/debug_exceptions.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 1f403b5e1a..89f515ee3d 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -51,6 +51,7 @@ module ActionDispatch autoload :BestStandardsSupport autoload :Callbacks autoload :Cookies + autoload :DebugExceptions autoload :ExceptionWrapper autoload :Flash autoload :Head diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb new file mode 100644 index 0000000000..5da21ddd3d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -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 diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 96598b529d..c0532c80c4 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -1,3 +1,4 @@ +require 'action_controller/metal/exceptions' require 'active_support/core_ext/exception' module ActionDispatch diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index aa7a5e4d64..346770acb7 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -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 - end + def render_exception(env, exception) + wrapper = ExceptionWrapper.new(env, exception) - # 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 + status = wrapper.status_code + locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale + path = "#{public_path}/#{status}.html" - # Attempts to render a static error page based on the - # status_code 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 public/500.da.html - # then attempt to render public/500.html. 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" - - 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 + 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 - 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 render_failsafe(failsafe_error) + $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" + FAILSAFE_RESPONSE + end - def logger(env) - env['action_dispatch.logger'] || stderr_logger - end + def render(status, body) + [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] + 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 diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index d191a203dd..c95b8221a1 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -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 diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index 74067cb895..5eff1eb09d 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -3,6 +3,7 @@ require 'abstract_unit' module ShowExceptions class ShowExceptionsController < ActionController::Base use ActionDispatch::ShowExceptions + use ActionDispatch::DebugExceptions def boom raise 'boom!' diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 90f13a3bb9..4f1140b5d3 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -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 -- cgit v1.2.3