we can deprecate Rescue with :instance_method and MyHandler.

Author: apotonick

lib/trailblazer/macro/rescue.rb

@@ -1,6 +1,6 @@
 module Trailblazer
   module Macro
-    NoopHandler = lambda { |*| }
+    NoopHandler = lambda { |ctx, flow_options, circuit_options, exception:| }
 
     def self.Rescue(*exceptions, handler: NoopHandler, id: Macro.id_for(nil, macro: :Rescue), &block)
       exceptions = [StandardError] unless exceptions.any?
@@ -13,14 +13,23 @@ def self.Rescue(*exceptions, handler: NoopHandler, id: Macro.id_for(nil, macro:
       handler = Activity::Option.build(handler) do |instance_method|
         # this is for :instance_methods
         is_instance_method = true
-        Rescue::InstanceMethodWithCircuitInterfaceAndKwargs.new(instance_method)
+        callable = Rescue::InstanceMethodWithCircuitInterfaceAndKwargs.new(instance_method)
+
+        # assuming we have to deprecate everything.
+        Rescue::Deprecate::InstanceMethodAtRuntime.new(callable, instance_method) # TODO: remove in 2.3.
       end
 
       if is_instance_method
-        # raise handler.inspect
         # deprecate at runtime :D
       else
         # deprecate now!
+        if handler.method(:call).arity == -3
+          # raise handler.inspect
+          puts "@@@@@ xxx #{handler.inspect}"
+          Rescue::Deprecate.warning_for_deprecated_callable
+# raise "why are we not hitting this from line 291?"
+          handler = Rescue::Deprecate::Callable.new(handler)
+        end
       end
 
 
@@ -35,11 +44,11 @@ def self.Rescue(*exceptions, handler: NoopHandler, id: Macro.id_for(nil, macro:
       # This block is evaluated by {Wrap}.
       rescue_block = ->(ctx, flow_options, circuit_options, &nested_activity) do
         begin
-          nested_activity.call # FIXME: use yield.
+          nested_activity.call(ctx, flow_options, circuit_options) # FIXME: use yield.
         rescue *exceptions => exception
           # DISCUSS: should we deprecate this signature and rather apply the Task API here?
           # handler.call(exception, ctx, flow_options, circuit_options) # FIXME: when there's an error here, it shows the wrong exception!
-          handler.call(exception, ctx, flow_options, circuit_options) # FIXME: when there's an error here, it shows the wrong exception!
+          handler.call(ctx, flow_options, circuit_options, exception: exception) # FIXME: when there's an error here, it shows the wrong exception!
 
           return ctx, flow_options, Trailblazer::Activity::Left
         end
@@ -49,12 +58,52 @@ def self.Rescue(*exceptions, handler: NoopHandler, id: Macro.id_for(nil, macro:
     end
 
     module Rescue
+      module Deprecate
+        class InstanceMethodAtRuntime < Struct.new(:filter, :instance_method_option)
+          def call(ctx, flow_options, circuit_options, exception:)
+            exec_context = circuit_options.fetch(:exec_context)
+            # hacky, but hey, it's deprecation code!
+            instance_method = exec_context.method(instance_method_option.instance_variable_get(:@filter))
+
+            if instance_method.arity == -3 # this doesn't catch all, but we ignore that.
+              # Activity::Deprecate.warn(
+                # Activity::DSL::Linear::Deprecate.dsl_caller_location(after: /forwardable.+Rescue/),
+              warning =  "#{exec_context.class}##{instance_method.name} " + Deprecate.message_for_deprecation_warning
+
+              Kernel.warn %([Trailblazer] #{warning}\n) # TODO: allow that in Activity::Deprecate.
+            end
+
+            filter.(ctx, flow_options, circuit_options, exception: exception)
+          end
+        end
+
+        class Callable < Struct.new(:filter)
+          def call(ctx, flow_options, circuit_options, exception:)
+            # Call a user handler with the deprecated interface.
+            filter.(exception, [ctx, flow_options], **circuit_options)
+          end
+        end
+
+        def self.message_for_deprecation_warning
+          %(The Rescue() handler has a new interface, the old `(exception, (ctx), ..), **` signature is deprecated.
+Please use (ctx, flow_options, circuit_options, exception:, **) , check ### FIXME _____---------------)
+        end
+
+        def self.warning_for_deprecated_callable
+          Activity::Deprecate.warn(
+            Activity::DSL::Linear::Deprecate.dsl_caller_location(after: /forwardable.+Rescue/),
+            message_for_deprecation_warning
+          )
+        end
+      end
       # This is kind of specific to Rescue, but might be useful elsewhere. We want to run a circuit interface
       # *instance method* with keyword arguments. in a normal CI-conform callable, this doesn't need any
       # wrapping, but instance_methods are different.
       class InstanceMethodWithCircuitInterfaceAndKwargs < Struct.new(:instance_method_option) # DISCUSS: move somewhere else?
-        def call(ctx, flow_options, circuit_options, **kwargs)
-          instance_method_option.(ctx, flow_options, circuit_options, keyword_arguments: kwargs, **circuit_options)
+        def call(ctx, flow_options, circuit_options, exception:, **kwargs)
+          # Call the method the old, weird style with exception as a positional argument
+          # followed by the composite circuit interface.
+          instance_method_option.(exception, [ctx, flow_options], keyword_arguments: kwargs, **circuit_options)
         end
       end
     end

test/docs/rescue_test.rb

@@ -251,3 +251,106 @@ def validate(ctx, validate: false, seq:, **)
     end
   end
 end
+
+class RescueDeprecateHandlerWithPositionalExceptionArgumentTest < Minitest::Spec
+  Song = Class.new
+  module Song::Operation; end
+
+  #:rescue-handler-2-1
+  class MyHandler
+    def self.call(exception, (ctx), *)
+      ctx[:seq] << :MyHandler
+      ctx[:exception_class] = exception.class
+    end
+  end
+  #:rescue-handler-2-1 end
+
+  class Song::Operation::Create < Trailblazer::Activity::Railway
+    step :create_model
+    step :notify
+    left :log_error
+    #~methods
+    include T.def_steps(:create_model, :upload, :notify, :log_error)
+    include Rehash
+    #~methods end
+  end
+
+  it do
+    activity = nil
+    _, warnings = capture_io do
+      activity = Class.new(Trailblazer::Activity::Railway) do
+        step Rescue(RuntimeError, handler: MyHandler) {
+          step :rehash
+        }
+        include Rehash
+      end
+    end
+    line_number_for_rescue = __LINE__ - 6
+
+    # Deprecation warning at compile time.
+    assert_equal warnings, %([Trailblazer] #{File.realpath(__FILE__)}:#{line_number_for_rescue} The Rescue() handler has a new interface, the old `(exception, (ctx), ..), **` signature is deprecated.
+Please use (ctx, flow_options, circuit_options, exception:, **) , check ### FIXME _____---------------
+)
+
+    _, warnings = capture_io do
+      assert_invoke activity, seq: "[:rehash]"
+    end
+    assert_equal warnings, ""
+
+    # _, warnings = capture_io do
+      assert_invoke activity, rehash_raise: RuntimeError, terminus: :failure, seq: "[:rehash, :MyHandler]", expected_ctx_variables: {exception_class: RuntimeError}
+    # end
+    assert_equal warnings, ""
+  end
+end
+
+class RescueDeprecateInstanceMethodHandlerWithPositionalExceptionArgumentTest < Minitest::Spec
+  Song = Class.new
+  module Song::Operation; end
+
+
+  class Song::Operation::Create < Trailblazer::Activity::Railway
+    step Rescue(RuntimeError, handler: :my_handler) {
+      step :upload
+      step :rehash
+    }
+
+    #:rescue-handler-instance-method-2-1
+    def my_handler(exception, (ctx), *)
+    # def my_handler(ctx, flow_options, *, **)
+      ctx[:seq] << :my_handler
+      ctx[:exception_class] = exception.class
+    end
+    #:rescue-handler-instance-method-2-1 end
+    #~methods
+    include T.def_steps(:upload)
+    include Rehash
+    #~methods end
+  end
+
+  it {
+    _, warnings = capture_io do
+      assert_invoke Song::Operation::Create, seq: "[:upload, :rehash]"
+    end
+
+    assert_equal warnings, ""
+  }
+
+  it {
+    _, warnings = capture_io do
+      assert_invoke Song::Operation::Create, rehash_raise: RuntimeError, terminus: :failure, seq: "[:upload, :rehash, :my_handler]", expected_ctx_variables: {exception_class: RuntimeError}
+    end
+
+    assert_equal warnings, %([Trailblazer] RescueDeprecateInstanceMethodHandlerWithPositionalExceptionArgumentTest::Song::Operation::Create#my_handler The Rescue() handler has a new interface, the old `(exception, (ctx), ..), **` signature is deprecated.
+Please use (ctx, flow_options, circuit_options, exception:, **) , check ### FIXME _____---------------\n)
+  }
+end
+
+=begin
+Option#call implies we need an :exec_context kwarg, since we want to separate the circuit_options from the Options-specific parameters. E.g. what if the filter
+calls another activity and needs the original circuit_options?
+
+that means pure circuit_options filter are called differently from Option ones (the first don't receive kwargs)
+=end
+
+