Nebo15 / sage

A dependency-free tool to run distributed transactions in Elixir, inspired by Sagas pattern.
MIT License
912 stars 40 forks source link

Noisy exceptions from run_async #68

Closed sparkertime closed 1 year ago

sparkertime commented 2 years ago

Environment

Current Behavior

I created a repo https://github.com/sparkertime/sage_exceptions_oddity where I can reproduce what seems like strange behavior to me. If a transaction raises an exception during run_async, it still dumps the exception to the terminal even though the process continues.

In that repo, I define two functions

  def bust_synchronously() do
    Sage.new()
    |> Sage.run(:foo, &this_errors/2)
    |> Sage.execute()
  end

  def bust_asynchronously() do
    Sage.new()
    |> Sage.run_async(:foo, &this_errors/2, :noop)
    |> Sage.execute()
  end

  defp this_errors(_, _), do: raise(RuntimeError, "boop")

Both of these tests pass, but the bust_asynchronously test produces this console output before passing

14:44:06.800 [error] Task #PID<0.216.0> started from #PID<0.215.0> terminating
** (RuntimeError) boop
    (sage_exceptions 0.1.0) lib/exceptional_saga.ex:14: ExceptionalSaga.this_errors/2
    (sage 0.6.1) lib/sage/executor.ex:212: Sage.Executor.apply_transaction_fun/4
    (elixir 1.13.1) lib/task/supervised.ex:89: Task.Supervised.invoke_mfa/2
    (elixir 1.13.1) lib/task/supervised.ex:34: Task.Supervised.reply/4
    (stdlib 3.17) proc_lib.erl:226: :proc_lib.init_p_do_apply/3
Function: #Function<1.58992025/0 in Sage.Executor.execute_transaction/4>
    Args: []
AndrewDryga commented 2 years ago

@sparkertime I think the only way to silence this is to wrap async executors into a try..catch block here, but I'm not 100% sure we need to do this. Raises are not a common way to return errors so maybe having them in the log is preferred for some users..

AndrewDryga commented 1 year ago

I'm closing this for now but I'll be happy to discuss it further.