Permalink
Browse files

Split ShowExceptions responsibilities in two middlewares.

  • Loading branch information...
1 parent 956ecff commit 750bb5c865ac9234da91ec451eec7d9de55b8f9b @josevalim josevalim committed Dec 1, 2011
View
1 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
View
77 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
View
1 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
View
110 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 @@ def initialize(app, consider_all_requests_local = nil)
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
- # <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"
-
- 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
View
22 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 @@ def to_s
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
1 actionpack/test/controller/show_exceptions_test.rb
@@ -3,6 +3,7 @@
module ShowExceptions
class ShowExceptionsController < ActionController::Base
use ActionDispatch::ShowExceptions
+ use ActionDispatch::DebugExceptions
def boom
raise 'boom!'
View
4 actionpack/test/dispatch/show_exceptions_test.rb
@@ -29,8 +29,8 @@ def call(env)
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
3 railties/lib/rails/application.rb
@@ -195,10 +195,13 @@ def default_middleware_stack
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
2 ...cation/middleware/show_exceptions_test.rb → ...application/middleware/exceptions_test.rb
@@ -3,7 +3,7 @@
require 'rack/test'
module ApplicationTests
- class ShowExceptionsTest < Test::Unit::TestCase
+ class MiddlewareExceptionsTest < Test::Unit::TestCase
include ActiveSupport::Testing::Isolation
include Rack::Test::Methods
View
4 railties/test/application/middleware_test.rb
@@ -33,6 +33,7 @@ def app
"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 @@ def app
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

0 comments on commit 750bb5c

Please sign in to comment.