Skip to content

Commit

Permalink
Fix diagnostics page for routing errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Dec 15, 2011
1 parent 283a087 commit 26e7400
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 16 deletions.
4 changes: 2 additions & 2 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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*

Expand Down
9 changes: 2 additions & 7 deletions actionpack/lib/action_controller/metal/rescue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@ module Rescue
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) &&
Expand All @@ -18,14 +13,14 @@ def rescue_with_handler(exception)
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
Expand Down
2 changes: 0 additions & 2 deletions actionpack/lib/action_controller/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ class Railtie < Rails::Railtie
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
Expand Down
13 changes: 10 additions & 3 deletions actionpack/test/controller/show_exceptions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,17 @@ class ShowExceptionsController < ActionController::Base
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
Expand All @@ -27,9 +35,8 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
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)
Expand Down
1 change: 1 addition & 0 deletions railties/lib/rails/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ def env_config
"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
})
Expand Down
15 changes: 13 additions & 2 deletions railties/test/application/middleware/exceptions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,30 @@ def call(env)
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
get '/foo'
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

Expand Down

0 comments on commit 26e7400

Please sign in to comment.