From f0753992ab8cc9bbbf9b047fdc56f8899df5635e Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 13 Nov 2006 02:03:50 +0000 Subject: test controller rescues git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5504 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/lib/action_controller/rescue.rb | 146 ++++++++------ .../templates/rescues/_request_and_response.rhtml | 16 +- actionpack/test/controller/rescue_test.rb | 212 +++++++++++++++++++++ actionpack/test/fixtures/public/404.html | 1 + actionpack/test/fixtures/public/500.html | 1 + 5 files changed, 306 insertions(+), 70 deletions(-) create mode 100644 actionpack/test/controller/rescue_test.rb create mode 100644 actionpack/test/fixtures/public/404.html create mode 100644 actionpack/test/fixtures/public/500.html diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 363eba2931..08ee699f7f 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -1,12 +1,36 @@ module ActionController #:nodoc: - # Actions that fail to perform as expected throw exceptions. These exceptions can either be rescued for the public view + # Actions that fail to perform as expected throw exceptions. These exceptions can either be rescued for the public view # (with a nice user-friendly explanation) or for the developers view (with tons of debugging information). The developers view # is already implemented by the Action Controller, but the public view should be tailored to your specific application. So too # could the decision on whether something is a public or a developer request. # # You can tailor the rescuing behavior and appearance by overwriting the following two stub methods. module Rescue + LOCALHOST = '127.0.0.1'.freeze + + DEFAULT_RESCUE_RESPONSE = :internal_server_error + DEFAULT_RESCUE_RESPONSES = { + 'ActionController::RoutingError' => :not_found, + 'ActionController::UnknownAction' => :not_found + } + + DEFAULT_RESCUE_TEMPLATE = 'diagnostics' + DEFAULT_RESCUE_TEMPLATES = { + 'ActionController::MissingTemplate' => 'missing_template', + 'ActionController::RoutingError' => 'routing_error', + 'ActionController::UnknownAction' => 'unknown_action', + 'ActionView::TemplateError' => 'template_error' + } + def self.included(base) #:nodoc: + base.cattr_accessor :rescue_responses + base.rescue_responses = Hash.new(DEFAULT_RESCUE_RESPONSE) + base.rescue_responses.update DEFAULT_RESCUE_RESPONSES + + base.cattr_accessor :rescue_templates + base.rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE) + base.rescue_templates.update DEFAULT_RESCUE_TEMPLATES + base.extend(ClassMethods) base.class_eval do alias_method_chain :perform_action, :rescue @@ -38,103 +62,101 @@ module ActionController #:nodoc: logger.fatal(exception.to_s) else logger.fatal( - "\n\n#{exception.class} (#{exception.message}):\n " + - clean_backtrace(exception).join("\n ") + + "\n\n#{exception.class} (#{exception.message}):\n " + + clean_backtrace(exception).join("\n ") + "\n\n" ) end end + # Overwrite to implement public exception handling (for requests answering false to local_request?). def rescue_action_in_public(exception) #:doc: - case exception - when RoutingError, UnknownAction - render_text(IO.read(File.join(RAILS_ROOT, 'public', '404.html')), "404 Not Found") - else - render_text(IO.read(File.join(RAILS_ROOT, 'public', '500.html')), "500 Internal Error") + render_optional_error_file response_code_for_rescue(exception) + end + + def render_optional_error_file(status_code) #:nodoc: + status = interpret_status(status_code) + path = "#{RAILS_ROOT}/public/#{status[0,3]}.html" + if File.exists?(path) + render :file => path, :status => status + else + head status end end - # Overwrite to expand the meaning of a local request in order to show local rescues on other occurrences than - # the remote IP being 127.0.0.1. For example, this could include the IP of the developer machine when debugging - # remotely. + # True if the request came from localhost, 127.0.0.1. Override this + # method if you wish to redefine the meaning of a local request to + # include remote IP addresses or other criteria. def local_request? #:doc: - [request.remote_addr, request.remote_ip] == ["127.0.0.1"] * 2 + request.remote_addr == LOCALHOST and request.remote_ip == LOCALHOST end - # Renders a detailed diagnostics screen on action exceptions. + # Render detailed diagnostics for unhandled exceptions rescued from + # a controller action. def rescue_action_locally(exception) add_variables_to_assigns @template.instance_variable_set("@exception", exception) - @template.instance_variable_set("@rescues_path", File.dirname(rescues_path("stub"))) + @template.instance_variable_set("@rescues_path", File.dirname(rescues_path("stub"))) @template.send(:assign_variables_from_controller) @template.instance_variable_set("@contents", @template.render_file(template_path_for_local_rescue(exception), false)) - + response.content_type = Mime::HTML render_file(rescues_path("layout"), response_code_for_rescue(exception)) end - + private def perform_action_with_rescue #:nodoc: - begin - perform_action_without_rescue - rescue Exception => exception # errors from action performed - if defined?(Breakpoint) && params["BP-RETRY"] - msg = exception.backtrace.first - if md = /^(.+?):(\d+)(?::in `(.+)')?$/.match(msg) then - origin_file, origin_line = md[1], md[2].to_i - - set_trace_func(lambda do |type, file, line, method, context, klass| - if file == origin_file and line == origin_line then - set_trace_func(nil) - params["BP-RETRY"] = false - - callstack = caller - callstack.slice!(0) if callstack.first["rescue.rb"] - file, line, method = *callstack.first.match(/^(.+?):(\d+)(?::in `(.*?)')?/).captures - - message = "Exception at #{file}:#{line}#{" in `#{method}'" if method}." # `´ ( for ruby-mode) - - Breakpoint.handle_breakpoint(context, message, file, line) - end - end) - - retry - end - end + perform_action_without_rescue + rescue Exception => exception # errors from action performed + if defined?(Breakpoint) && params["BP-RETRY"] + msg = exception.backtrace.first + if md = /^(.+?):(\d+)(?::in `(.+)')?$/.match(msg) then + origin_file, origin_line = md[1], md[2].to_i + + set_trace_func(lambda do |type, file, line, method, context, klass| + if file == origin_file and line == origin_line then + set_trace_func(nil) + params["BP-RETRY"] = false + + callstack = caller + callstack.slice!(0) if callstack.first["rescue.rb"] + file, line, method = *callstack.first.match(/^(.+?):(\d+)(?::in `(.*?)')?/).captures - rescue_action(exception) + message = "Exception at #{file}:#{line}#{" in `#{method}'" if method}." # `´ ( for ruby-mode) + + Breakpoint.handle_breakpoint(context, message, file, line) + end + end) + + retry + end end + + rescue_action(exception) end def rescues_path(template_name) - File.dirname(__FILE__) + "/templates/rescues/#{template_name}.rhtml" + "#{File.dirname(__FILE__)}/templates/rescues/#{template_name}.rhtml" end def template_path_for_local_rescue(exception) - rescues_path( - case exception - when MissingTemplate then "missing_template" - when RoutingError then "routing_error" - when UnknownAction then "unknown_action" - when ActionView::TemplateError then "template_error" - else "diagnostics" - end - ) + rescues_path(rescue_templates[exception.class.name]) end - + def response_code_for_rescue(exception) - case exception - when UnknownAction, RoutingError - "404 Page Not Found" - else - "500 Internal Error" - end + rescue_responses[exception.class.name] end - + def clean_backtrace(exception) - exception.backtrace.collect { |line| Object.const_defined?(:RAILS_ROOT) ? line.gsub(RAILS_ROOT, "") : line } + if backtrace = exception.backtrace + if defined?(RAILS_ROOT) + backtrace.map { |line| line.sub RAILS_ROOT, '' } + else + backtrace + end + end end end end diff --git a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml index f2f5732ecd..fe60bfdd40 100644 --- a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml +++ b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml @@ -10,7 +10,7 @@ <% begin %> <%= form_tag(request.request_uri, "method" => request.method) %> - + <% for key, values in params %> <% next if key == "BP-RETRY" %> <% for value in Array(values) %> @@ -26,19 +26,19 @@ <% end %> <% - request_parameters_without_action = request.parameters.clone - request_parameters_without_action.delete("action") - request_parameters_without_action.delete("controller") - - request_dump = request_parameters_without_action.inspect.gsub(/,/, ",\n") + clean_params = request.parameters.clone + clean_params.delete("action") + clean_params.delete("controller") + + request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n") %>

Request

-

Parameters: <%=h request_dump == "{}" ? "None" : request_dump %>

+

Parameters: 

<%=h request_dump %>

Show session dump

Response

-Headers: <%=h response ? response.headers.inspect.gsub(/,/, ",\n") : "None" %>
+

Headers: 

<%=h response ? response.headers.inspect.gsub(',', ",\n") : 'None' %>

diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb new file mode 100644 index 0000000000..c49575cad3 --- /dev/null +++ b/actionpack/test/controller/rescue_test.rb @@ -0,0 +1,212 @@ +require File.dirname(__FILE__) + '/../abstract_unit' +require 'flexmock' + +class RescueController < ActionController::Base + def raises + render :text => 'already rendered' + raise "don't panic!" + end +end + + +class RescueTest < Test::Unit::TestCase + include FlexMock::TestCase + FIXTURE_PUBLIC = "#{File.dirname(__FILE__)}/../fixtures".freeze + + def setup + @controller = RescueController.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + + RescueController.consider_all_requests_local = true + @request.remote_addr = '1.2.3.4' + @request.host = 'example.com' + + begin + raise 'foo' + rescue => @exception + end + end + + + def test_rescue_action_locally_if_all_requests_local + stub = flexstub(@controller) + stub.should_receive(:local_request?).and_return(true) + stub.should_receive(:rescue_action_locally).with(@exception).once + stub.should_receive(:rescue_action_in_public).never + + with_all_requests_local do + @controller.send :rescue_action, @exception + end + end + + def test_rescue_action_locally_if_remote_addr_is_localhost + stub = flexstub(@controller) + stub.should_receive(:local_request?).and_return(true) + stub.should_receive(:rescue_action_locally).with(@exception).once + stub.should_receive(:rescue_action_in_public).never + + with_all_requests_local false do + @controller.send :rescue_action, @exception + end + end + + def test_rescue_action_in_public_otherwise + stub = flexstub(@controller) + stub.should_receive(:local_request?).and_return(false) + stub.should_receive(:rescue_action_in_public).with(@exception).once + stub.should_receive(:rescue_action_locally).never + + with_all_requests_local false do + @controller.send :rescue_action, @exception + end + end + + + def test_rescue_action_in_public_with_error_file + with_rails_root FIXTURE_PUBLIC do + with_all_requests_local false do + get :raises + end + end + + assert_response :internal_server_error + body = File.read("#{FIXTURE_PUBLIC}/public/500.html") + assert_equal body, @response.body + end + + def test_rescue_action_in_public_without_error_file + with_rails_root '/tmp' do + with_all_requests_local false do + get :raises + end + end + + assert_response :internal_server_error + assert_equal ' ', @response.body + end + + + def test_rescue_unknown_action_in_public_with_error_file + with_rails_root FIXTURE_PUBLIC do + with_all_requests_local false do + get :foobar_doesnt_exist + end + end + + assert_response :not_found + body = File.read("#{FIXTURE_PUBLIC}/public/404.html") + assert_equal body, @response.body + end + + def test_rescue_unknown_action_in_public_without_error_file + with_rails_root '/tmp' do + with_all_requests_local false do + get :foobar_doesnt_exist + end + end + + assert_response :not_found + assert_equal ' ', @response.body + end + + + def test_rescue_action_locally + get :raises + assert_response :internal_server_error + assert_template 'diagnostics.rhtml' + assert @response.body.include?('RescueController#raises'), "Response should include controller and action." + assert @response.body.include?("don't panic"), "Response should include exception message." + end + + + def test_local_request_when_remote_addr_is_localhost + flexstub(@controller).should_receive(:request).and_return(@request) + with_remote_addr '127.0.0.1' do + assert @controller.send(:local_request?) + end + end + + def test_local_request_when_remote_addr_isnt_locahost + flexstub(@controller).should_receive(:request).and_return(@request) + with_remote_addr '1.2.3.4' do + assert !@controller.send(:local_request?) + end + end + + + def test_rescue_responses + responses = ActionController::Base.rescue_responses + + assert_equal ActionController::Rescue::DEFAULT_RESCUE_RESPONSE, responses.default + assert_equal ActionController::Rescue::DEFAULT_RESCUE_RESPONSE, responses[Exception.new] + + assert_equal :not_found, responses[ActionController::RoutingError.name] + assert_equal :not_found, responses[ActionController::UnknownAction.name] + end + + def test_rescue_templates + templates = ActionController::Base.rescue_templates + + assert_equal ActionController::Rescue::DEFAULT_RESCUE_TEMPLATE, templates.default + assert_equal ActionController::Rescue::DEFAULT_RESCUE_TEMPLATE, templates[Exception.new] + + assert_equal 'missing_template', templates[ActionController::MissingTemplate.name] + assert_equal 'routing_error', templates[ActionController::RoutingError.name] + assert_equal 'unknown_action', templates[ActionController::UnknownAction.name] + assert_equal 'template_error', templates[ActionView::TemplateError.name] + end + + + def test_clean_backtrace + with_rails_root nil do + # No action if RAILS_ROOT isn't set. + cleaned = @controller.send(:clean_backtrace, @exception) + assert_equal @exception.backtrace, cleaned + end + + with_rails_root Dir.pwd do + # RAILS_ROOT is removed from backtrace. + cleaned = @controller.send(:clean_backtrace, @exception) + expected = @exception.backtrace.map { |line| line.sub(RAILS_ROOT, '') } + assert_equal expected, cleaned + + # No action if backtrace is nil. + assert_nil @controller.send(:clean_backtrace, Exception.new) + end + end + + protected + def with_all_requests_local(local = true) + old_local, ActionController::Base.consider_all_requests_local = + ActionController::Base.consider_all_requests_local, local + yield + ensure + ActionController::Base.consider_all_requests_local = old_local + end + + def with_remote_addr(addr) + old_remote_addr, @request.remote_addr = @request.remote_addr, addr + yield + ensure + @request.remote_addr = old_remote_addr + end + + def with_rails_root(path = nil) + old_rails_root = RAILS_ROOT if defined?(RAILS_ROOT) + if path + silence_warnings { Object.const_set(:RAILS_ROOT, path) } + else + Object.remove_const(:RAILS_ROOT) rescue nil + end + + yield + + ensure + if old_rails_root + silence_warnings { Object.const_set(:RAILS_ROOT, old_rails_root) } + else + Object.remove_const(:RAILS_ROOT) rescue nil + end + end +end diff --git a/actionpack/test/fixtures/public/404.html b/actionpack/test/fixtures/public/404.html new file mode 100644 index 0000000000..497397ccea --- /dev/null +++ b/actionpack/test/fixtures/public/404.html @@ -0,0 +1 @@ +404 error fixture diff --git a/actionpack/test/fixtures/public/500.html b/actionpack/test/fixtures/public/500.html new file mode 100644 index 0000000000..7c66c7a943 --- /dev/null +++ b/actionpack/test/fixtures/public/500.html @@ -0,0 +1 @@ +500 error fixture -- cgit v1.2.3