Chris McCord's Twitter clone demo, handle_info issue for :post_deleted

Phoenix Forum Questions / Help
,
1

Hi, I coded up the @chrismccord Twitter clone demo here and the CRUD works fine for CRU but not for D: The tutorial does not cover broadcast of the :post_deleted message.

So, I added to timeline.ex this:

  def delete_post(%Post{} = post) do
    Repo.delete(post)
    |> broadcast(:post_deleted)
  end

Which broadcasts the :post_deleted message to both browser tabs; but because there is no handle_info handler, it crashes the process. Nevertheless, it reloads and both browser tabs look fine, with the post having been deleted.

So then I tried to add a proper handler to avoid the crash. In index.ex, I added:

  def handle_info({_, post}, socket) do
    {:noreply,  assign(socket, posts: fetch_posts())}
  end

But this locks up the process; it looks like a race condition or something.

Am I overthinking: Is letting it crash part of the design; i.e., just re-mount to get the new list?

What am I missing?
Thanks.

1 Like
2

Does this do the job? I’m not sure if Repo.delete is atomic, but Repo.transaction definitely is:

def delete_post(%Post{} = post) do
  {:ok, %{post: deleted_post}} = Ecto.Multi.new() |> Ecto.Multi.delete(:post, post) |> Repo.transaction()
  broadcast(:post_deleted, deleted_post)
end

It’d also be nice to see your error message :smile_cat:

1 Like
3

Thanks. I added your suggestion (note: a sprout is a post from the original tutorial), a bit modified to fit:

  def delete_sprout(%Sprout{} = sprout) do
    {:ok, %{sprout: deleted_sprout}} = Ecto.Multi.new() |> Ecto.Multi.delete(:sprout, sprout) |> Repo.transaction()
    broadcast({:ok, deleted_sprout}, :sprout_deleted)
  end

and the handlers are:

  def handle_event("delete", %{"id" => id}, socket) do
    IO.puts "HANDLE EVENT delete"
    sprout = Timeline.get_sprout!(id)
    {:ok, _} = Timeline.delete_sprout(sprout)

    {:noreply, assign(socket, :sprouts, fetch_sprouts())}
  end

 # and

  def handle_info({:sprout_deleted, sprout}, socket) do
    IO.puts "HANDLE INFO :sprout_deleted"
    {:noreply, assign(socket, :sprouts, fetch_sprouts())}
  end

And the output is:

HANDLE EVENT delete
[debug] QUERY OK source="sprouts" db=5.4ms idle=1592.2ms
SELECT s0."id", s0."body", s0."likes_count", s0."reposts_count", s0."username", s0."inserted_at", s0."updated_at" FROM "sprouts" AS s0 WHERE (s0."id" = $1) [71]
[debug] QUERY OK db=0.7ms queue=0.5ms idle=1597.1ms
begin []
HANDLE INFO :sprout_deleted
[debug] QUERY OK db=0.7ms
DELETE FROM "sprouts" WHERE "id" = $1 [71]
[debug] QUERY OK db=0.8ms
commit []
HANDLE INFO :sprout_deleted
[debug] QUERY OK source="sprouts" db=0.6ms idle=1600.4ms
SELECT s0."id", s0."body", s0."likes_count", s0."reposts_count", s0."username", s0."inserted_at", s0."updated_at" FROM "sprouts" AS s0 ORDER BY s0."id" DESC []
[debug] QUERY OK source="sprouts" db=0.6ms idle=1600.7ms
SELECT s0."id", s0."body", s0."likes_count", s0."reposts_count", s0."username", s0."inserted_at", s0."updated_at" FROM "sprouts" AS s0 ORDER BY s0."id" DESC []
[debug] QUERY OK source="sprouts" db=0.4ms idle=1601.3ms
SELECT s0."id", s0."body", s0."likes_count", s0."reposts_count", s0."username", s0."inserted_at", s0."updated_at" FROM "sprouts" AS s0 ORDER BY s0."id" DESC []

As you can see, there is no error message – it just locks up. Based on the number of SELECT queries shown to reload the list; I’m now thinking it’s a runaway loop that gets killed.
NOTE that both handle_event and handle_info are calling fetch_sprouts() to reload the posts. I’m thinking that this is a problem. Maybe I need to use PubSub.broadcast_from/5 which takes a PID as a from argument; then, ignore the event if I’m the process that initiated the delete. I’ll experiment…

4

I tried broadcast_from/5, which does not send to self(), and even with just one browser tab open, it locks up if I have an info handler implemented for :sprout_deleted.

Weird.

5

Summary so far: If I have this implemented:

  def handle_event("delete", %{"id" => id}, socket) do
    sprout = Timeline.get_sprout!(id)
    {:ok, _} = Timeline.delete_sprout(sprout)

    {:noreply, assign(socket, :sprouts, fetch_sprouts())}
  end

Then I must not try to handle the corresponding broadcast event or it locks up. I.e., it works fine from the user’s perspective if I comment this out:

 # def handle_info({:sprout_deleted, sprout} = event, socket) do
  #   # {:noreply, assign(socket, :sprouts, fetch_sprouts())}
  #   {:noreply, socket}
  # end

However, the only reason it works is because the broadcast event does not match any function so the process crashes, and the list is reloaded (with the item deleted). Is this a feature or a bug?

6

As demoed in the video, you are adding the newly inserted post to the head of the list.

def handle_info({:post_inserted, post}, socket) do
  {:noreply, update(socket, :posts, fn posts -> [post | posts] end)}
end

The same goes to the deletion.

def handle_info({:post_deleted, post}, socket) do
  {:noreply, update(socket, :posts, fn posts -> List.delete(posts, post) end)}
end
1 Like
7

The demo uses phx-append=“prepend”, so reloading the list of tweets won’t remove the deleted one. I think you’d have to use “replace” (the default).

https://hexdocs.pm/phoenix_live_view/Phoenix.LiveView.html#module-dom-patching-and-temporary-assigns

“Let it crash” is about recovering from faults, caused by software bugs or hardware failure. Crashing should never be part of an app’s normal operation, otherwise the logs would be filled with crash logs, making it very difficult to find the real bugs.

2 Likes
8

@co0lsky , I tried that. It locks up just the same.

Also, the posts argument in the fn posts callback is an empty list []; I assume, because of the temporary_assigns: [posts: []] that was added to the return of mount.

9

@dom I guessed that “let it crash” was not appropriate here, but wasn’t sure.

So, if I understand the docs correctly: If I want to broadcast a delete of an item in a List, then I have to use phx-append="replace", which means I cannot use temporary_assigns: [sprouts: []] in mount.

In other words, there is a tradeoff between a) keeping server RAM & payloads lean by using temporary_assigns & append/prepend, vs. b) ability to broadcast deletes via replace but having to hold all in RAM?

It seems like what is needed is a separate message to the client with an ID to delete.

10

Just to confirm, when I use replace instead of prepend, and I do not use temporary_assigns, then the broadcast of :post_deleted is handled smoothly.

However, the handler has to be:

 def handle_info({:post_deleted, post} = event, socket) do
    {:noreply, update(socket, :posts, fn posts -> fetch_posts() end)}
  end

as this does not work:

  def handle_info({:post_deleted, post} = event, socket) do
    {:noreply, update(socket, :posts, fn posts -> List.delete(posts, post) end)}
  end

…I’ll see if I can figure out why.

Aside: It’s occurring to me that broadcasting a delete may be problematic from a UX standpoint. Unless it can be done surgically, without reloading everything.

11

Using replace fixes the broadcast of delete (see above), but it breaks the likes & repost logic. Seems to be a tradeoff.

12

I assume your handler for :post_updated prepends the updated post to the list of posts?

  def handle_info({:post_updated, post}, socket) do
    {:noreply, update(socket, :posts, fn posts -> [post | posts] end)}
  end

I think this works with phx-append=“prepend”, because from the doc:

When using phx-update , a unique DOM ID must always be set in the container. If using “append” or “prepend”, a DOM ID must also be set for each child. When appending or prepending elements containing an ID already present in the container, LiveView will replace the existing element with the new content instead appending or prepending a new element.

Since you’re no longer using that phx-append, you may have to tweak the handler to reload the posts from DB, or update the specific post without changing the ordering of the list (for instance using Enum.map and modifying the item only if the ID matches).

1 Like
13

I’m confident that I could get a phx-update="replace" approach to work; however, I want to focus on solving “prepend,” to be efficient.

My understanding so far: When using phx-update="prepend" coupled with the temporary_assigns: [posts: []] return value in mount, you are changing the logic to send only a posts diff list down the wire.

Hence, although the demo uses this for update:

  def handle_info({:post_updated, post}, socket) do
    {:noreply, update(socket, :posts, fn posts -> [post | posts] end)}
  end

…posts is always empty, [], so you can do the same thing with this:

  def handle_info({:post_updated, post}, socket) do
    {:noreply, update(socket, :posts, fn posts -> [post] end)}
  end

The problem is: Because you are assigning only a posts diff list to the assigns, how to you tell liveview to send a delete message? For this to happen, you would have to be adding some sort of flag to the assigns, which is not being done, so there is no way to send a delete message in the diff. Hope this makes sense.

14

You could use hooks, for instance assign a :deleted_posts list, render that into hidden divs, and use phx-hook to remove a tweet once it’s marked deleted. Or perhaps cleaner: set a “deleted” flag inside the tweet, and just render it as an empty invisible element if deleted is true?

15

Thanks @dom , those are both good ideas. I’ll start with he flag to hide the div. I’m confident enough now to start using this for my app. I’ll report back here in a week or so, let you know how it went.

16

:wave:

Ecto.Multi.new() |> Ecto.Multi.delete(:post, post) |> Repo.transaction()

and

Repo.delete(post)

are equivalent, I think.