Extract ActionController rescue templates into Rescue and ShowExceptions middleware.

This commit breaks all exception catching plugins like ExceptionNotifier. These plugins should be rewritten as middleware instead overriding Controller#rescue_action_in_public.

8 files changed, 61 insertions, 326 deletions

diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb
index 60957a2f0e..484d3c5ce7 100644
--- a/actionpack/test/controller/action_pack_assertions_test.rb
+++ b/actionpack/test/controller/action_pack_assertions_test.rb
@@ -489,7 +489,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase
     get :index
     assert_response :success
     flunk 'Expected non-success response'
-  rescue ActiveSupport::TestCase::Assertion => e
+  rescue RuntimeError => e
     assert e.message.include?('FAIL')
   end
 
diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb
index f4517d06c4..0c54d61426 100644
--- a/actionpack/test/controller/base_test.rb
+++ b/actionpack/test/controller/base_test.rb
@@ -27,6 +27,7 @@ class EmptyController < ActionController::Base
 end
 class NonEmptyController < ActionController::Base
   def public_action
+    render :nothing => true
   end
   
   hide_action :hidden_action
@@ -51,6 +52,7 @@ end
 
 class DefaultUrlOptionsController < ActionController::Base
   def default_url_options_action
+    render :nothing => true
   end
 
   def default_url_options(options = nil)
@@ -151,11 +153,8 @@ class PerformActionTest < ActionController::TestCase
   
   def test_get_on_hidden_should_fail
     use_controller NonEmptyController
-    get :hidden_action
-    assert_response 404
-    
-    get :another_hidden_action
-    assert_response 404
+    assert_raise(ActionController::UnknownAction) { get :hidden_action }
+    assert_raise(ActionController::UnknownAction) { get :another_hidden_action }
   end
 
   def test_namespaced_action_should_log_module_name
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 6b9d1056a3..560c09509b 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -149,6 +149,9 @@ class PageCachingTest < ActionController::TestCase
 end
 
 class ActionCachingTestController < ActionController::Base
+  rescue_from(Exception) { head 500 }
+  rescue_from(ActiveRecord::RecordNotFound) { head :not_found }
+
   caches_action :index, :redirected, :forbidden, :if => Proc.new { |c| !c.request.format.json? }, :expires_in => 1.hour
   caches_action :show, :cache_path => 'http://test.host/custom/show'
   caches_action :edit, :cache_path => Proc.new { |c| c.params[:id] ? "http://test.host/#{c.params[:id]};edit" : "http://test.host/edit" }
diff --git a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb
index dd69a63020..0c02afea36 100644
--- a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb
+++ b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb
@@ -15,12 +15,6 @@ class DeprecatedBaseMethodsTest < ActionController::TestCase
 
   tests Target
 
-  def test_log_error_silences_deprecation_warnings
-    get :raises_name_error
-  rescue => e
-    assert_not_deprecated { @controller.send :log_error, e }
-  end
-
   if defined? Test::Unit::Error
     def test_assertion_failed_error_silences_deprecation_warnings
       get :raises_name_error
diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb
index c3bd113b86..b315232a7b 100644
--- a/actionpack/test/controller/dispatcher_test.rb
+++ b/actionpack/test/controller/dispatcher_test.rb
@@ -45,19 +45,6 @@ class DispatcherTest < Test::Unit::TestCase
     def log_failsafe_exception(status, exception); end
   end
 
-  def test_failsafe_response
-    Dispatcher.any_instance.expects(:_call).raises('b00m')
-    ActionDispatch::Failsafe.any_instance.expects(:log_failsafe_exception)
-
-    assert_nothing_raised do
-      assert_equal [
-        500,
-        {"Content-Type" => "text/html"},
-        ["<html><body><h1>500 Internal Server Error</h1></body></html>"]
-      ], dispatch
-    end
-  end
-
   def test_prepare_callbacks
     a = b = c = nil
     Dispatcher.to_prepare { |*args| a = b = c = 1 }
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index 5876f55420..5fd3eb0446 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -169,6 +169,7 @@ class FilterTest < Test::Unit::TestCase
     end
 
     def public
+      render :text => 'ok'
     end
   end
 
@@ -177,6 +178,10 @@ class FilterTest < Test::Unit::TestCase
     before_filter :find_record
     before_filter :ensure_login
 
+    def index
+      render :text => 'ok'
+    end
+
     private
       def find_record
         @ran_filter ||= []
diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb
index da3f7b0cb8..5b7d40d16d 100644
--- a/actionpack/test/controller/layout_test.rb
+++ b/actionpack/test/controller/layout_test.rb
@@ -205,8 +205,7 @@ end
 class LayoutExceptionRaised < ActionController::TestCase
   def test_exception_raised_when_layout_file_not_found
     @controller = SetsNonExistentLayoutFile.new
-    get :hello
-    assert_kind_of ActionView::MissingTemplate, @controller.template.instance_eval { @exception }
+    assert_raise(ActionView::MissingTemplate) { get :hello }
   end
 end
 
diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb
index 894420a910..f745926b20 100644
--- a/actionpack/test/controller/rescue_test.rb
+++ b/actionpack/test/controller/rescue_test.rb
@@ -138,213 +138,88 @@ class RescueController < ActionController::Base
     end
 end
 
-class RescueControllerTest < ActionController::TestCase
-  FIXTURE_PUBLIC = "#{File.dirname(__FILE__)}/../fixtures".freeze
-
-  def setup
-    super
-    set_all_requests_local
-    populate_exception_object
-  end
-
-  def set_all_requests_local
-    RescueController.consider_all_requests_local = true
-    @request.remote_addr = '1.2.3.4'
-    @request.host = 'example.com'
-  end
-
-  def populate_exception_object
-    begin
-      raise 'foo'
-    rescue => @exception
-    end
-  end
-
-  def test_rescue_exceptions_raised_by_filters
-    with_rails_root FIXTURE_PUBLIC do
-      with_all_requests_local false do
-        get :before_filter_raises
-      end
-    end
+class ExceptionInheritanceRescueController < ActionController::Base
 
-    assert_response :internal_server_error
+  class ParentException < StandardError
   end
 
-  def test_rescue_action_locally_if_all_requests_local
-    @controller.expects(:local_request?).never
-    @controller.expects(:rescue_action_locally).with(@exception)
-    @controller.expects(:rescue_action_in_public).never
-
-    with_all_requests_local do
-      @controller.send :rescue_action, @exception
-    end
+  class ChildException < ParentException
   end
 
-  def test_rescue_action_locally_if_remote_addr_is_localhost
-    @controller.expects(:local_request?).returns(true)
-    @controller.expects(:rescue_action_locally).with(@exception)
-    @controller.expects(:rescue_action_in_public).never
-
-    with_all_requests_local false do
-      @controller.send :rescue_action, @exception
-    end
+  class GrandchildException < ChildException
   end
 
-  def test_rescue_action_in_public_otherwise
-    @controller.expects(:local_request?).returns(false)
-    @controller.expects(:rescue_action_locally).never
-    @controller.expects(:rescue_action_in_public).with(@exception)
+  rescue_from ChildException,      :with => lambda { head :ok }
+  rescue_from ParentException,     :with => lambda { head :created }
+  rescue_from GrandchildException, :with => lambda { head :no_content }
 
-    with_all_requests_local false do
-      @controller.send :rescue_action, @exception
-    end
+  def raise_parent_exception
+    raise ParentException
   end
 
-  def test_rescue_action_in_public_with_localized_error_file
-    # Change locale
-    old_locale = I18n.locale
-    I18n.locale = :da
-
-    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.da.html")
-    assert_equal body, @response.body
-  ensure
-    I18n.locale = old_locale
+  def raise_child_exception
+    raise ChildException
   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
+  def raise_grandchild_exception
+    raise GrandchildException
   end
+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
+class ExceptionInheritanceRescueControllerTest < ActionController::TestCase
+  def test_bottom_first
+    get :raise_grandchild_exception
+    assert_response :no_content
   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
+  def test_inheritance_works
+    get :raise_child_exception
+    assert_response :created
   end
+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
+class ControllerInheritanceRescueController < ExceptionInheritanceRescueController
+  class FirstExceptionInChildController < StandardError
   end
 
-  def test_rescue_missing_template_in_public
-    with_rails_root FIXTURE_PUBLIC do
-      with_all_requests_local true do
-        get :missing_template
-      end
-    end
-
-    assert_response :internal_server_error
-    assert @response.body.include?('missing_template'), "Response should include the template name."
+  class SecondExceptionInChildController < StandardError
   end
 
-  def test_rescue_action_locally
-    get :raises
-    assert_response :internal_server_error
-    assert_template 'diagnostics.erb'
-    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
+  rescue_from FirstExceptionInChildController, 'SecondExceptionInChildController', :with => lambda { head :gone }
 
-  def test_local_request_when_remote_addr_is_localhost
-    @controller.expects(:request).returns(@request).at_least_once
-    with_remote_addr '127.0.0.1' do
-      assert @controller.send(:local_request?)
-    end
+  def raise_first_exception_in_child_controller
+    raise FirstExceptionInChildController
   end
 
-  def test_local_request_when_remote_addr_isnt_locahost
-    @controller.expects(:request).returns(@request)
-    with_remote_addr '1.2.3.4' do
-      assert !@controller.send(:local_request?)
-    end
+  def raise_second_exception_in_child_controller
+    raise SecondExceptionInChildController
   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]
-    assert_equal :not_found, responses['ActiveRecord::RecordNotFound']
-    assert_equal :conflict, responses['ActiveRecord::StaleObjectError']
-    assert_equal :unprocessable_entity, responses['ActiveRecord::RecordInvalid']
-    assert_equal :unprocessable_entity, responses['ActiveRecord::RecordNotSaved']
-    assert_equal :method_not_allowed, responses['ActionController::MethodNotAllowed']
-    assert_equal :not_implemented, responses['ActionController::NotImplemented']
+class ControllerInheritanceRescueControllerTest < ActionController::TestCase
+  def test_first_exception_in_child_controller
+    get :raise_first_exception_in_child_controller
+    assert_response :gone
   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[ActionView::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]
+  def test_second_exception_in_child_controller
+    get :raise_second_exception_in_child_controller
+    assert_response :gone
   end
 
-  def test_not_implemented
-    with_all_requests_local false do
-      with_rails_public_path(".") do
-        head :not_implemented
-      end
-    end
-    assert_response :not_implemented
-    assert_equal "GET, PUT", @response.headers['Allow']
+  def test_exception_in_parent_controller
+    get :raise_parent_exception
+    assert_response :created
   end
+end
 
-  def test_method_not_allowed
-    with_all_requests_local false do
-      with_rails_public_path(".") do
-        get :method_not_allowed
-      end
-    end
-    assert_response :method_not_allowed
-    assert_equal "GET, HEAD, PUT", @response.headers['Allow']
+class ApplicationController < ActionController::Base
+  rescue_from ActionController::RoutingError do
+    render :text => 'no way'
   end
+end
 
+class RescueControllerTest < ActionController::TestCase
   def test_rescue_handler
     get :not_authorized
     assert_response :forbidden
@@ -399,133 +274,6 @@ class RescueControllerTest < ActionController::TestCase
     get :resource_unavailable_raise_as_string
     assert_equal "RescueController::ResourceUnavailableToRescueAsString", @response.body
   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_public_path(rails_root)
-      old_rails = Object.const_get(:Rails) rescue nil
-      mod = Object.const_set(:Rails, Module.new)
-      (class << mod; self; end).instance_eval do
-        define_method(:public_path) { "#{rails_root}/public" }
-      end
-      yield
-    ensure
-      Object.module_eval { remove_const(:Rails) } if defined?(Rails)
-      Object.const_set(:Rails, old_rails) if old_rails
-    end
-
-    def with_rails_root(path = nil,&block)
-      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
-
-      with_rails_public_path(path, &block)
-
-    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
-
-class ExceptionInheritanceRescueController < ActionController::Base
-
-  class ParentException < StandardError
-  end
-
-  class ChildException < ParentException
-  end
-
-  class GrandchildException < ChildException
-  end
-
-  rescue_from ChildException,      :with => lambda { head :ok }
-  rescue_from ParentException,     :with => lambda { head :created }
-  rescue_from GrandchildException, :with => lambda { head :no_content }
-
-  def raise_parent_exception
-    raise ParentException
-  end
-
-  def raise_child_exception
-    raise ChildException
-  end
-
-  def raise_grandchild_exception
-    raise GrandchildException
-  end
-end
-
-class ExceptionInheritanceRescueControllerTest < ActionController::TestCase
-  def test_bottom_first
-    get :raise_grandchild_exception
-    assert_response :no_content
-  end
-
-  def test_inheritance_works
-    get :raise_child_exception
-    assert_response :created
-  end
-end
-
-class ControllerInheritanceRescueController < ExceptionInheritanceRescueController
-  class FirstExceptionInChildController < StandardError
-  end
-
-  class SecondExceptionInChildController < StandardError
-  end
-
-  rescue_from FirstExceptionInChildController, 'SecondExceptionInChildController', :with => lambda { head :gone }
-
-  def raise_first_exception_in_child_controller
-    raise FirstExceptionInChildController
-  end
-
-  def raise_second_exception_in_child_controller
-    raise SecondExceptionInChildController
-  end
-end
-
-class ControllerInheritanceRescueControllerTest < ActionController::TestCase
-  def test_first_exception_in_child_controller
-    get :raise_first_exception_in_child_controller
-    assert_response :gone
-  end
-
-  def test_second_exception_in_child_controller
-    get :raise_second_exception_in_child_controller
-    assert_response :gone
-  end
-
-  def test_exception_in_parent_controller
-    get :raise_parent_exception
-    assert_response :created
-  end
-end
-
-class ApplicationController < ActionController::Base
-  rescue_from ActionController::RoutingError do
-    render :text => 'no way'
-  end
 end
 
 class RescueTest < ActionController::IntegrationTest