Return errors when an event cannot be emitted

[benwilson512]

Hey @LostKobrakai, shamefully I'm only now upgrading our main app to take advantage of some of the changes you made last year. In the transition from Repo.serial to Events.emit I'm finding it difficult to return errors to the user, and I'm curious what you're doing.

For example today I have code like this:

def edit_comment(shipment, comment_data) do
  Repo.serial(shipment, fn shipment ->
    if comment = find_comment(comment_data) do
       Events.emit(shipment, %CommentEdited{...}) # etc
    else
      {:error, "comment not found"}
    end
  end)
end

It isn't clear how I can get the same functionality anymore. Specifically I need two things:
A) Blocking serialization around the shipment for the entire duration of the logic flow so that I don't need to worry about serialization errors on commit
B) The ability to return non event types

The api around Events.emit is slick, but as far as I can tell, the return type of the function you pass to Events.emit/3 is event.t | [event.t]. so it isn't a drop in replacement for Repo.serial. I could do:

def edit_comment(shipment, comment_data) do
  if comment = find_comment(comment_data) do
     Events.emit(shipment, fn shipment -> %CommentEdited{...} end) # etc
  else
    {:error, "comment not found"}
  end
end

However if I do this I've lost serialization guarantees.

Obviously I can add a Repo.serial back to my own Repo module, but I'm struggling to see how Fable can be used properly without it.

Notably, this isn't mitigated by the use of Multi, the same issues persist.

[LostKobrakai]

def edit_comment(shipment, comment_data) do
  shipment
  |> Events.emit(fn shipment, repo, _ -> 
    if comment = find_comment(comment_data) do
       %CommentEdited{...} 
   else
      repo.rollback("comment not found")
    end
  end)
  |> Repo.transaction()
end

Sorry for the briefness. Will comment again later today.

[LostKobrakai]

To explain it: Events.emit does wrap the function you're providing into a function executing the fable specific stuff as well. This wrapping function is then returned and executed by Repo.transaction. So everything within the provided function is executed within a transaction and you have all the error handling capabilities of Repo.transaction(fun).

[benwilson512]

Gotcha, that makes sense. I wonder if we could support a return type from the fun that is event | [event] | {:error, reason} and then repo.rollback(reason) within the def emit function.

Our code style uses with quite a lot eg:

with {:ok, comment} <- fetch_comment(shipment, args),
:authorized <- validate_rights(user, comment, :edit) do
  %Event{}
end

and adding a boilerplate else \n {:error, reason} -> Repo.rollback(reason) to all of these is going to be annoying. Thoughts?

[LostKobrakai]

It could be done (added here). I don't really like mixing tagged and untagged return values, which is why I didn't do so.

[benwilson512]

Agreed, I generally don't either. I just don't like repo.rollback(reason) very much because it's a non local return, and our error handling is all functional to date except for explicit ! functions that happy path or raise.

Let me take some time and use the approach you've outlined today and see how it goes.

[LostKobrakai]

I don't mind it much here, as it's likely to not be the last function to be called, but I see your argument. Maybe event | [event] | {:error, reason} is the best of both worlds, because I would hate to have {:ok, event | [event]} | {:error, reason}

[benwilson512]

@LostKobrakai agreed, it's not like there's any ambiguity between whether the return value is an event or error, a wrapping {:ok} tuple wouldn't really add anything. It's even clear enough for dialyzer to tell the difference since the options structs are always the success case, and an error tuple is always the error case, and everything else is a fail.