From 26e7400cc5415dbce5e2c5d13da96ad8c25749e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 15 Dec 2011 19:43:49 +0100 Subject: Fix diagnostics page for routing errors. --- actionpack/CHANGELOG.md | 4 ++-- actionpack/lib/action_controller/metal/rescue.rb | 9 ++------- actionpack/lib/action_controller/railtie.rb | 2 -- actionpack/test/controller/show_exceptions_test.rb | 13 ++++++++++--- railties/lib/rails/application.rb | 1 + railties/test/application/middleware/exceptions_test.rb | 15 +++++++++++++-- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index b354f05960..da18060202 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -68,9 +68,9 @@ <%= f.text_field :version %> <% end %> -* Refactor ActionDispatch::ShowExceptions. Controller is responsible for choice to show exceptions. *Sergey Nartimov* +* Refactor ActionDispatch::ShowExceptions. Controller is responsible for choosing to show exceptions when `consider_all_requests_local` is false. *Sergey Nartimov* - It's possible to override +show_detailed_exceptions?+ in controllers to specify which requests should provide debugging information on errors. + It's possible to override `show_detailed_exceptions?` in controllers to specify which requests should provide debugging information on errors. * Responders now return 204 No Content for API requests without a response body (as in the new scaffold) *José Valim* diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb index 736ff5b31c..c4b056ebc0 100644 --- a/actionpack/lib/action_controller/metal/rescue.rb +++ b/actionpack/lib/action_controller/metal/rescue.rb @@ -3,11 +3,6 @@ module ActionController #:nodoc: extend ActiveSupport::Concern include ActiveSupport::Rescuable - included do - config_accessor :consider_all_requests_local - self.consider_all_requests_local = false if consider_all_requests_local.nil? - end - def rescue_with_handler(exception) if (exception.respond_to?(:original_exception) && (orig_exception = exception.original_exception) && @@ -18,14 +13,14 @@ module ActionController #:nodoc: end def show_detailed_exceptions? - consider_all_requests_local || request.local? + request.local? end private def process_action(*args) super rescue Exception => exception - request.env['action_dispatch.show_detailed_exceptions'] = show_detailed_exceptions? + request.env['action_dispatch.show_detailed_exceptions'] ||= show_detailed_exceptions? rescue_with_handler(exception) || raise(exception) end end diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index cb2e2b17aa..7af256fd99 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -21,8 +21,6 @@ module ActionController paths = app.config.paths options = app.config.action_controller - options.consider_all_requests_local ||= app.config.consider_all_requests_local - options.assets_dir ||= paths["public"].first options.javascripts_dir ||= paths["public/javascripts"].first options.stylesheets_dir ||= paths["public/stylesheets"].first diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index 5eff1eb09d..2f5c268330 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -5,9 +5,17 @@ module ShowExceptions use ActionDispatch::ShowExceptions use ActionDispatch::DebugExceptions + before_filter :only => :another_boom do + request.env["action_dispatch.show_detailed_exceptions"] = true + end + def boom raise 'boom!' end + + def another_boom + raise 'boom!' + end end class ShowExceptionsTest < ActionDispatch::IntegrationTest @@ -27,9 +35,8 @@ module ShowExceptions end end - test 'show diagnostics from a remote ip when consider_all_requests_local is true' do - ShowExceptionsController.any_instance.stubs(:consider_all_requests_local).returns(true) - @app = ShowExceptionsController.action(:boom) + test 'show diagnostics from a remote ip when env is already set' do + @app = ShowExceptionsController.action(:another_boom) self.remote_addr = '208.77.188.166' get '/' assert_match(/boom/, body) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 22689cc278..56d11d0b01 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -168,6 +168,7 @@ module Rails "action_dispatch.parameter_filter" => config.filter_parameters, "action_dispatch.secret_token" => config.secret_token, "action_dispatch.show_exceptions" => config.action_dispatch.show_exceptions, + "action_dispatch.show_detailed_exceptions" => config.consider_all_requests_local, "action_dispatch.logger" => Rails.logger, "action_dispatch.backtrace_cleaner" => Rails.backtrace_cleaner }) diff --git a/railties/test/application/middleware/exceptions_test.rb b/railties/test/application/middleware/exceptions_test.rb index 0174352900..d130d244c1 100644 --- a/railties/test/application/middleware/exceptions_test.rb +++ b/railties/test/application/middleware/exceptions_test.rb @@ -45,7 +45,7 @@ module ApplicationTests assert_equal 404, last_response.status end - test "unspecified route when set action_dispatch.show_exceptions to false" do + test "unspecified route when action_dispatch.show_exceptions is not set raises an exception" do app.config.action_dispatch.show_exceptions = false assert_raise(ActionController::RoutingError) do @@ -53,11 +53,22 @@ module ApplicationTests end end - test "unspecified route when set action_dispatch.show_exceptions to true" do + test "unspecified route when action_dispatch.show_exceptions is set shows 404" do app.config.action_dispatch.show_exceptions = true assert_nothing_raised(ActionController::RoutingError) do get '/foo' + assert_match "The page you were looking for doesn't exist.", last_response.body + end + end + + test "unspecified route when action_dispatch.show_exceptions and consider_all_requests_local are set shows diagnostics" do + app.config.action_dispatch.show_exceptions = true + app.config.consider_all_requests_local = true + + assert_nothing_raised(ActionController::RoutingError) do + get '/foo' + assert_match "No route matches", last_response.body end end -- cgit v1.2.3