Nebo15 / ecto_mnesia

Ecto adapter for Mnesia Erlang term database.
https://hex.pm/packages/ecto_mnesia
MIT License
243 stars 41 forks source link

Ecto.Multi errors result in `CaseClauseError {:error, :rollback}` rather than the error that was raised in the Ecto.Multi function. #73

Open Qqwy opened 5 years ago

Qqwy commented 5 years ago

Whenever creating an Ecto.Multi that returns an error. (Such as by using Ecto.Multi.error or by returning {:error, val} rather than {:ok, val} from an Ecto.Multi.run), this is not properly passed up through the Repo.transaction call.

Furthermore, rather than passing a value up, a CaseClauseError is raised.

Example:

iex> Ecto.Multi.new() |> Ecto.Multi.error(:example, "Foo") |> YourApp.Repo.transaction

Expected result:

{:error, {:example, "Foo", %Ecto.Multi{}}}

Actual result:

** (CaseClauseError) no case clause matching: {:error, :rollback}
    (ecto) lib/ecto/repo/queryable.ex:21: Ecto.Repo.Queryable.transaction/4
AndrewDryga commented 5 years ago

Can you please add Ecto version that you are using?

Qqwy commented 5 years ago

This is on the newest Ecto that EctoMnesia supports, which is 2.1 IIRC.

I was able to find the cause of this problem, btw. It's here. The rollback function is expected to take the reason as second argument, but EctoMnesia uses an unused _tid argument and hard-codes Mnesia.abort with the reason :rollback.

This causes Ecto.Multi's transaction handling logic (see here to crash because it expects the error result contain a four-element tuple rather than :rollback.