Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition between assign_async and temporary_assigns #3537

Open
kelvinst opened this issue Nov 30, 2024 · 2 comments
Open

Race condition between assign_async and temporary_assigns #3537

kelvinst opened this issue Nov 30, 2024 · 2 comments

Comments

@kelvinst
Copy link
Contributor

kelvinst commented Nov 30, 2024

Environment

  • Elixir version (elixir -v):
Erlang/OTP 27 [erts-15.1.2] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]

Elixir 1.17.3 (compiled with Erlang/OTP 27)
  • Phoenix version (mix deps):
* phoenix 1.7.14 (Hex package) (mix)
  locked at 1.7.14 (phoenix) c7859bc5
  ok
  • Phoenix LiveView version (mix deps):
* phoenix_live_view 1.0.0-rc.7 (Hex package) (mix)
  locked at 1.0.0-rc.7 (phoenix_live_view) b82a4575
  ok
  • Operating system: MacOS Sonoma 14.4.1
  • Browsers you attempted to reproduce this bug on (the more the merrier): It's a problem with tests
  • Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: Yes (well, it should, didn't test, but you will get it)

Actual behavior

I have a LiveView like this:

defmodule TwelvexWeb.StocksLive do
  def mount(params, _session, socket) do
    {:ok, socket, temporary_assigns: [time_series: AsyncResult.loading()]}
  end

  # ...
  
  defp apply_action(socket, :time_series, params) do
    assign_async(socket, :time_series, fn ->
      {:ok, %{time_series: get_time_series(params["symbol"])}}
    end)
  end

  # ...
  
  def render(assigns) do
    ~H"""
    <%!-- ... --%>

    <.async_result :let={time_series} assign={@time_series}>
      <:loading> Loading time series </:loading>
      <:failed> Failed to load time series </:failed>
      <.chart
        id="time-series-chart"
        series={time_series}
      />
    </.async_result>


    <%!-- ... --%>
    """
  end
end

Then a test for it, like this:

test "loads the time series chart asynchronously", %{conn, conn} do
  {:ok, live, html} = live(conn, ~p"/time_series?symbol=AAPL")
  assert html =~ "Loading time series"
  html = render_async(live)
  refute html =~ "Loading time series"
  # ...
end

And this test gets flaky, failing randomly in the refute html =~ "Loading time series". During my investigations I figured the flakiness went away when I added a pid |> Process.info() |> dbg() inside the function I pass assign_async(:time_series, <here>). Then I realized the same assign was being set by temporary_assigns too.

My guess is that the error happens when the temporary_assigns logic runs right after assign_async task finishes but before the render. Not even sure if that's fixable on LiveView side, but at least worth mentioning in the docs.

Expected behavior

I would expect to not have problems with assign_async and temporary_assigns being combined.

@kelvinst
Copy link
Contributor Author

kelvinst commented Nov 30, 2024

I think I can put up a simple example project for it, let me know if you need that.

@drtheuns-enreach
Copy link

I think I've run into the same (or similar) issue, where I have a start_async that, if it finishes too soon, causes the loading state to be rendered even though the state has already updated. In my case this is without temporary assigns. Inside of a live component if that makes a difference. I don't have a small reproduction; I just added a Process.sleep for now, which is less than ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants