Store failure exception with cause on result

[branson-simplethread]

Hello! 👋

The goal of this change is to make raised exceptions available to the caller of Actor#result. Additionally, exceptions rescued with fail_on are captured as the cause of the configured failure class.

I'm not too attached to the implementation here, so I'd appreciate your thoughts! But I found this necessary for some work I'm doing and had to patch ServiceActor::Failable.

[sunny]

Looks good! Before we merge this, can you explain a bit how you are patching ServiceActor::Failable?

[viralpraxis]

This seems to be an incompatible change. For instance, it breaks actors with something like

begin
rescue ExceptionClass => e
  fail! exception: e
end

without the patch, the result would have {exception: e}; with the patch e would be ServiceActor::Failure

[branson-simplethread]

can you explain a bit how you are patching ServiceActor::Failable?

Sure! It's basically what is in this PR. Without this change the exception is lost.

module ServiceActor::Failable::PrependedMethods
  def _call
    super
  rescue *self.class.fail_ons => e
    fail!(error: e.message, exception: e)
  end
end

The exception is then available at result[:exception].

---

For context, here's a simplified Rails example demonstrating how this is useful.

class UpdatePost < ApplicationActor
  input :id
  input :attributes

  fail_on ActiveRecord::RecordInvalid

  def call
    Post.find(id).update!(attributes)
  end
end

class PostsController < ApplicationController
  def update
    result = UpdatePost.result(id: params[:id], attributes: post_params)
    if result.success?
      head :ok
    elsif result[:exception].is_a?(ActiveRecord::RecordInvalid)
      render json: { errors: result[:exception].record.errors.full_messages }, status: :unprocessable_content
    else
      render json: { error: 'Could not update post' }, status: :bad_request
    end
  end
end

[branson-simplethread]

This seems to be an incompatible change. For instance, it breaks actors with something like

begin
rescue ExceptionClass => e
  fail! exception: e
end

without the patch, the result would have {exception: e}; with the patch e would be ServiceActor::Failure

True! I guess the thing to decide is whether to:

• make this a breaking change, with an exception: passed manually to fail! now accessible at result.exception.cause rather than result[:exception]. That would make this a public feature that could be used in a more local manner than actor-wide fail_on.
• pass rescued fail_on exception from Failable to Result in a more private way

[viralpraxis]

This seems to be an incompatible change. For instance, it breaks actors with something like

begin
rescue ExceptionClass => e
  fail! exception: e
end

without the patch, the result would have {exception: e}; with the patch e would be ServiceActor::Failure

True! I guess the thing to decide is whether to:

* make this a breaking change, with an `exception:` passed manually to `fail!` now accessible at `result.exception.cause` rather than `result[:exception]`. That would make this a public feature that could be used in a more local manner than actor-wide `fail_on`.

* pass rescued `fail_on` exception from `Failable` to `Result` in a more private way

Perhaps we could just emit a warning first? we used to follow this way e.g. here: #138

[sunny]

This seems to be an incompatible change. For instance, it breaks actors with something like

begin
rescue ExceptionClass => e
  fail! exception: e
end

without the patch, the result would have {exception: e}; with the patch e would be ServiceActor::Failure

Another way to lower the surface of the incompatible change would be to only fill the exception when using fail_on. When using fail! it would stay nil.