.success? still reporting true when interactor invoked via .call is calling internally another interactor with .call! and latter fails

[thaiden]

Context

I've noticed in our code base that we had cases where Interactor was encapsulating more interactors and calling them via .call!. One of the interactors was raising Interactor::Failure exception, but because parent interactor was invoked via .call context .success? was still reporting true.

I believe that is wrong behavior and since .call is not raising exception it at least should return false when .success? is invoked.

Steps to reproduce

  class ParentInteractor
    include Interactor

    def call
      InteractorWithError.call!
    end
  end

  class InteractorWithError
    include Interactor

    def call
      raise Interactor::Failure.new('test')
    end
  end

  test 'interactor still reports success after it failed' do
    context = ParentInteractor.call
    assert context.failure?
    assert_not context.success?
  end

[tiagofsilva]

If I understood correctly, this example is trying to fail context in an abnormal way. You're supposed to use the Context API, like context.fail! (which in its turn will raise the Failure error). This example bypasses the Context#fail! method, trying to raise the error by its own. The error class is supposed to be used internally. Notice, though, that you're not even failing the context object of the nested interactor. Try:

class InteractorWithError
  include Interactor

  def call
    raise Interactor::Failure.new('test')
  end
end

ctx = InteractorWithError.call
ctx.success? 
#=> true

The second issue in this example is that the context object of the parent interactor is not being shared with the nested interactor. Therefore, the context of the parent interactor remains untouched after the call of the nested one (which also doesn't fail context as mentioned in the above example).

A more correct version of what you're trying to do would be:

class ParentInteractor
  include Interactor

  def call
    InteractorWithError.call!(context)
  end
end

class InteractorWithError
  include Interactor

  def call
    context.fail!
  end
end

ctx = ParentInteractor.call
ctx.success?
#=> false

I'd recommend reading the Context class documentation just to clarify what I tried to explain here.
Hope it helps! =)

[sunny]

I think the current way breaks the idea that call! should raise an exception on failures. Calling InteractorWithError.call!(context) may be more "correct" but it isn't very handy when the children interactors need to accept different arguments or be applied on a collection.

We ended up adding the following to our interactors:

    def handle_failures
      yield
    rescue Interactor::Failure => e
      context.fail!(error: e.context.error)
    end

Sadly we still have to think to wrap every call! inside interactors with handle_failures do … end.

I think it would make more sense to either have the Interactor::Failure exception get rethrown or, if it is rescued, at least fail the parent interactor.

[jqr]

I appreciate what @tiagofsilva was getting at about correct usage, but sometimes nesting of Interactors is dynamic, indirect, or generally requires the caller to be aware of the callee's implementation.

In #192 I think we have a fix for this and generally unexpected behavior of nested Interactors.

[Kukunin]

This behavior is actually dangerous because it's pretty expected that internal Interactor.call! exceptions will be bubbled up to the top, and won't be swallowed. Exceptions might remain unnoticed. +1 for changing the current behavior