test controller rescues

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5504 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 306 insertions, 70 deletions

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 <tt>local_request?</tt>).
       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) %>
       <input type="hidden" name="BP-RETRY" value="1" />
-  
+
       <% 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")
 %>
 
 <h2 style="margin-top: 30px">Request</h2>
-<p><b>Parameters</b>: <%=h request_dump == "{}" ? "None" : request_dump %></p>
+<p><b>Parameters</b>: <pre><%=h request_dump %></pre></p>
 
 <p><a href="#" onclick="document.getElementById('session_dump').style.display='block'; return false;">Show session dump</a></p>
 <div id="session_dump" style="display:none"><%= debug(request.session.instance_variable_get("@data")) %></div>
 
 
 <h2 style="margin-top: 30px">Response</h2>
-<b>Headers</b>: <%=h response ? response.headers.inspect.gsub(/,/, ",\n") : "None" %><br/>
+<p><b>Headers</b>: <pre><%=h response ? response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p>
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