Merge pull request #5128 from bogdan/deprecate_rescuable

AS::Callbacks: deprecate rescuable option

actionpack/lib/action_dispatch/middleware/callbacks.rb

@@ -5,7 +5,7 @@ module ActionDispatch
   class Callbacks
     include ActiveSupport::Callbacks
 
-    define_callbacks :call, :rescuable => true
+    define_callbacks :call
 
     class << self
       delegate :to_prepare, :to_cleanup, :to => "ActionDispatch::Reloader"
@@ -24,9 +24,15 @@ module ActionDispatch
     end
 
     def call(env)
-      run_callbacks :call do
+      error = nil
-        @app.call(env)
+      result = run_callbacks :call do
+        begin
+          @app.call(env)
+        rescue => error
+        end
       end
+      raise error if error
+      result
     end
   end
 end

activesupport/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*    AS::Callbacks: deprecate `:rescuable` option. *Bogdan Gusiev*
+
 *    Adds Integer#ordinal to get the ordinal suffix string of an integer. *Tim Gildea*
 
 *    AS::Callbacks: `:per_key` option is no longer supported

activesupport/lib/active_support/callbacks.rb

@@ -307,7 +307,6 @@ module ActiveSupport
         @name = name
         @config = {
           :terminator => "false",
-          :rescuable => false,
           :scope => [ :kind ]
         }.merge(config)
       end
@@ -317,35 +316,16 @@ module ActiveSupport
         method << "value = nil"
         method << "halted = false"
 
-        callbacks = yielding
+        callbacks = "value = yield if block_given? && !halted"
         reverse_each do |callback|
           callbacks = callback.apply(callbacks)
         end
         method << callbacks
 
-        method << "raise rescued_error if rescued_error" if config[:rescuable]
         method << "halted ? false : (block_given? ? value : true)"
         method.flatten.compact.join("\n")
       end
 
-      # Returns part of method that evaluates the callback block
-      def yielding
-        method = []
-        if config[:rescuable]
-          method << "rescued_error = nil"
-          method << "begin"
-        end
-
-        method << "value = yield if block_given? && !halted"
-
-        if config[:rescuable]
-          method << "rescue Exception => e"
-          method << "rescued_error = e"
-          method << "end"
-        end
-        method.join("\n")
-      end
-
     end
 
     module ClassMethods
@@ -508,11 +488,6 @@ module ActiveSupport
       #   if callback chain was terminated or not.
       #   Option makes sence only when <tt>:terminator</tt> option is specified.
       #
-      # * <tt>:rescuable</tt> - By default, after filters are not executed if
-      #   the given block or a before filter raises an error. By setting this option
-      #   to <tt>true</tt> exception raised by given block is stored and after
-      #   executing all the after callbacks the stored exception is raised.
-      #
       # * <tt>:scope</tt> - Indicates which methods should be executed when an object
       #   is used as a callback.
       #

activesupport/test/callbacks_test.rb

@@ -3,7 +3,7 @@ require 'abstract_unit'
 module CallbacksTest
   class Phone
     include ActiveSupport::Callbacks
-    define_callbacks :save, :rescuable => true
+    define_callbacks :save
 
     set_callback :save, :before, :before_save1
     set_callback :save, :after, :after_save1
@@ -439,13 +439,6 @@ module CallbacksTest
   end
 
   class CallbacksTest < ActiveSupport::TestCase
-    def test_save_phone
-      phone = Phone.new
-      assert_raise RuntimeError do
-        phone.save
-      end
-      assert_equal [:before, :after], phone.history
-    end
 
     def test_save_person
       person = Person.new