Strange error in Ecto.Multi

I am trying to load a book, update it and update the associated record like this:

Multi.new()
|> Multi.run(:get_book, fn _ -> Mango.Books.get_book!(line.book_id) end)
|> Ecto.Multi.run(:update_book, fn %{book: book} ->
  Books.update_book(Books.get_book!(book.book_id), %{status: 1})
end)
|> Ecto.Multi.run(:update_associated_record, fn %{book: book} ->
  Records.update_record(Records.get_record!(book.record_id), %{
    returned_at: :os.system_time(:seconds)
  })
end)
|> Repo.transaction()
|> case do
  {:ok, result} -> result
  {:ok, %{book: book}} -> book
  {:ok, %{record: record}} -> record |> whitelist_record_fields
end

This produces this error message:

** (exit) an exception was raised:
    ** (CaseClauseError) no case clause matching: %Mango.Books.Book{__meta__: #Ecto.Schema.Metadata<:loaded, "books">, author: "Waseem", code: 1116, id: 107, inserted_at: ~N[2018-05-05 11:39:49.688053], institute: #Ecto.Association.NotLoaded<association :institute is not loaded>, institute_id: 1, isbn: "13-1234-78", record_id: 17, records: #Ecto.Association.NotLoaded<association :records is not loaded>, status: :out, title: "Hello World", updated_at: ~N[2018-05-25 16:01:41.752519], year: 1982}
        (ecto) lib/ecto/multi.ex:421: Ecto.Multi.apply_operation/5
        (elixir) lib/enum.ex:1899: Enum."-reduce/3-lists^foldl/2-0-"/3
        (ecto) lib/ecto/multi.ex:411: anonymous fn/5 in Ecto.Multi.apply_operations/5
        (ecto) lib/ecto/adapters/sql.ex:576: anonymous fn/3 in Ecto.Adapters.SQL.do_transaction/3
        (db_connection) lib/db_connection.ex:1283: DBConnection.transaction_run/4
        (db_connection) lib/db_connection.ex:1207: DBConnection.run_begin/3
        (db_connection) lib/db_connection.ex:798: DBConnection.transaction/3
        (ecto) lib/ecto/repo/queryable.ex:23: Ecto.Repo.Queryable.transaction/4
        (mango) lib/mango/records/records.ex:112: anonymous fn/1 in Mango.Records.create_record_in/2 # referring to this line: |> Repo.transaction()
..
..

Any idea what might be going on here? Thank you.

I think you just need to name your first Multi.run(:get_book... as Multi.run(:book... ?

2 Likes

I wil l try but I believe that is only a name, will not make any difference.

The name is how you reference that step in later steps though.

Ecto.Multi very much is a named monadic pipeline.

Elixir really needs some monad stuff built in better so umpteen libraries don’t have to keep remaking it and could just use a behaviour…

2 Likes

It could also be failing on your .get_zzz!s but if it’s that, when corrected it would still fail when you use fn %{book: book} as you need to use a key that refers to a previous multi “step” I guess - I just recently started using multi though

I have tried the suggestions above, like this:

  multi_result =
    Multi.new()
    |> Multi.run(:bookA, fn _ -> Mango.Books.get_book!(line.book_id) end)
    |> Multi.run(:bookB, fn %{bookA: bookA} ->
    IO.inspect bookA
      Books.update_book(Books.get_book!(bookA.id), %{status: 1})
    end)
    |> Multi.run(:update_associated_record, fn %{bookB: bookB} ->
      Records.update_record(Records.get_record!(bookB.record_id), %{
        returned_at: :os.system_time(:seconds)
      })
    end)
    |> Repo.transaction()
    |> case do
      {:ok, result} -> result
      {:ok, %{book: book}} -> book
      {:ok, %{record: record}} -> record |> whitelist_record_fields
      {:error, :book_to_be_returned, reason, _changes} -> reason
    end

But I am getting the same error message.

Solved…

I have to start with:

Multi.new()
|> Multi.update(:book, Book.changeset(Repo.get(Book, line.book_id), %{status: 1}))

A couple of things now that I have time to look:

  1. Books.update_book(Books.get_book!(book.book_id), %{status: 1})
    What is this returning? An {:ok, result} term?

  Records.update_record(Records.get_record!(book.record_id), %{
    returned_at: :os.system_time(:seconds)
  })

What is this returning? Also an {:ok, result} term?

      {:ok, %{book: book}} -> book
      {:ok, %{record: record}} -> record |> whitelist_record_fields

These will never ever be matched on because {:ok, result} -> result will grab it all first.

3 Likes

Update is a different kind of call, it is better to use though, but still, run requires {:ok, result}/{:error, reason} and I bet that one of those were not.

3 Likes

As @OvermindDL1 states, check the retuning value of the Multi.run functions:

https://hexdocs.pm/ecto/Ecto.Multi.html#module-run

1 Like

Or rather what you return ‘from’ that function. :slight_smile:

2 Likes

When I set the run methods to return {:ok, result} it did not work because in the next line I was getting error like: could not find key record_id in :ok.record_id or something like that, but when I set the run method to return result only it worked. Like this:

  def update_book_to_be_returned_multi(multi, book_id) do
    alias Ecto.Multi
    alias Mango.Books
    alias Mango.Books.Book

    Multi.run(multi, :book_to_be_returned, fn _changes ->
      case Repo.get_by(Book, %{id: book_id, status: :out}) do
        nil ->
          {:error, %{error: "Book not found or already available.."}}

        book ->
          case Books.update_book(book, %{status: :available}) do
            book = book -> book
            nil -> {:error, %{error: "Error occured while updating the book.."}}
          end
      end
    end)
      end

And:

  multi_result =
    Multi.new()
    |> Works.update_book_to_be_returned_multi(line.book_id)
    |> Multi.run(:update_associated_record, fn %{book_to_be_returned: book_to_be_returned} ->
      Records.update_record(Repo.get(Record, book_to_be_returned.record_id), %{
        returned_at: :os.system_time(:seconds)
      })
    end)
...

I’m not sure what ‘next line’ is being spoken of though? It doesn’t sound like exiting a run function?

I was referring to this:

Records.update_record(Repo.get(Record, book_to_be_returned.record_id), %{

If you can make a reproducible self-contained testcase, that sounds like a bug. ^.^;

Yes, it was looking for record_id inside {ok:..}

Which it shouldn’t do yeah…

I grepped for Multi.run's at work and although I have a number of them I don’t use any of the previous values of the things (simple log dispatches and such)… Hmm…

Try this refactor:

def update_book_to_be_returned_multi(book_id) do
    alias Mango.Books.Book

    case Repo.get_by(Book, %{id: book_id, status: :out}) do
        nil ->
          {:error, %{error: "Book not found or already available.."}}
        book ->
          case Books.update_book(book, %{status: :available}) do
            book -> {:ok, book}
            nil -> {:error, %{error: "Error occured while updating the book.."}}
          end
    end
end

And:

  multi_result =
    Multi.new()
    |> Multi.run(:book_to_be_returned, update_book_to_be_returned_multi(line.book_id)
    |> Multi.run(:update_associated_record, fn %{book_to_be_returned: book_to_be_returned} ->
      Records.update_record(Repo.get(Record, book_to_be_returned.record_id), %{
        returned_at: :os.system_time(:seconds)
      })
    end)
1 Like

Thanks for sharing, this is the working code:

  multi_result =
    Multi.new() 
    |> Multi.run(:book_to_be_returned, fn _ -> Works.update_book_to_be_returned_multi(line.book_id) end)

and:

def update_book_to_be_returned_multi(book_id) do
alias Mango.Books
alias Mango.Books.Book

case Repo.get_by(Book, %{id: book_id, status: :out}) do
  nil ->
    {:error, %{error: "Book not found or already available.."}}

  book ->
    case Books.update_book(book, %{status: :available}) do
      {:ok, book} -> {:ok, book}  # update_book returns %{:ok, book_object } so this is the correct match
      nil -> {:error, %{error: "Error occured while updating the book.."}}
    end
end

end

1 Like