phoenix_live_view icon indicating copy to clipboard operation
phoenix_live_view copied to clipboard

Phoenix.LiveViewTest.render_upload/3 crashes when upload process redirects

Open mogest opened this issue 2 years ago • 2 comments

Environment

  • Elixir version (elixir -v): 1.13.3
  • Phoenix version (mix deps): 1.6.7
  • Phoenix LiveView version (mix deps): 0.17.9
  • Operating system: MacOS
  • Browsers you attempted to reproduce this bug on (the more the merrier): n/a
  • Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: n/a

Actual behavior

Using render_upload when the upload function returns {:noreply, push_redirect(socket, to: some_url)} causes the following error:

     ** (FunctionClauseError) no function clause matching in Floki.RawHTML.build_raw_html/4

     The following arguments were given to Floki.RawHTML.build_raw_html/4:
     
         # 1
         [error: {:live_redirect, %{kind: :push, to: "/url"}}]
     
         # 2
         []
     
         # 3
         &HtmlEntities.encode/1
     
         # 4
         :noop

It's because this line in Phoenix.LiveViewTest.render_chunk/3 calls render/1 and that doesn't work with a redirect.

Makes sense—the function is called render_upload after all—but there's no way to to perform an upload otherwise as private functions are used.

Expected behavior

I don't think we should change the functionality of render_upload, but I propose a new function perform_upload that does the same without rendering; instead it can return :ok | {:error, atom()}.

Then render_upload can be refactored to use the new perform_upload function and render the view in the :ok case.

I'm happy to code this up as a PR but wanted to check in that that was a sensible path to take first.

mogest avatar Jun 01 '22 20:06 mogest

Hi @mogest – I think this issue is a duplicate of #1620. Are you consuming the entry in the progress callback?

mcrumm avatar Jun 01 '22 22:06 mcrumm

Yes:

  defp handle_progress(_name, entry, socket) do
    if entry.done? do
      consume_uploads(socket)
    else
      {:noreply, socket}
    end
  end

Is that OK to do that? I found the same workaround as in #1620, but get intermittent failures on test runs so swapped that out with a reimplementation that doesn't render.

mogest avatar Jun 01 '22 23:06 mogest

Closed via eeafb2a5834645d812264a3032192639c4473777

chrismccord avatar Jan 19 '24 18:01 chrismccord