Skip to content

fix (LiveViewTest): Support configurable timeouts on :async_pids GenServer calls to reduce test flakiness - #4425

Open
s3cur3 wants to merge 1 commit into
phoenixframework:mainfrom
s3cur3:ty/async-pids
Open

fix (LiveViewTest): Support configurable timeouts on :async_pids GenServer calls to reduce test flakiness#4425
s3cur3 wants to merge 1 commit into
phoenixframework:mainfrom
s3cur3:ty/async-pids

Conversation

@s3cur3

@s3cur3 s3cur3 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

A common source of CI test flakiness for our LiveView app at work (which heavily uses async assigns) looks like this:

    1) test the page loads (MyAppWeb.AccountSettingsLiveTest)
       lib/my_app_web/live/account_settings_live_test.exs:123
       ** (EXIT from #PID<0.12267.0>) exited in: GenServer.call(#PID<0.24640.0>, {:phoenix, :async_pids}, 5000)
           ** (EXIT) time out

That error is the result of a render_async/1 call that timed out (though since it doesn't include a stacktrace, it takes a little effort to figure that out). Given that CI machines tend to be under-powered compared to developer machines, my understanding is this occurs when the CPU is overloaded and takes longer than 5 seconds to fetch all the async processes attached to the LiveView.

The changes here are designed to both
a) allow clients to increase that timeout, and
b) present a clear error when a timeout does occur here.

The biggest part I'm not sure of: should the timeout provided to render_async/2 be split between the :async_pids lookup and the actual message receive (as written), or should it only apply to the message receive portion? Prior to this change, it was the latter, which means the whole function call could have taken up to 5 seconds + the indicated timeout. If the timeout is not shared between the two steps, though, it feels quite awkward; either there's no way to configure the :async_pids timeout on a per-call basis (you would only be able to do so using the application config variable), or the function would need to accept two timeouts.

…nServer calls to reduce test flakiness

A common source of test flakiness for our LiveView app at work (which heavily uses async assigns) looks like this:

```
    1) test the page loads (MyAppWeb.AccountSettingsLiveTest)
       lib/my_app_web/live/account_settings_live_test.exs:123
       ** (EXIT from #PID<0.12267.0>) exited in: GenServer.call(#PID<0.24640.0>, {:phoenix, :async_pids}, 5000)
           ** (EXIT) time out
```

That error is the result of a `render_async/1` call that timed out (though since it doesn't include a stacktrace, it takes a little effort to figure that out). Given that CI machines tend to be under-powered compared to developer machines, my understanding is that this occurs when the CPU is overloaded and takes longer than 5 seconds to fetch all the async processes attached to the LiveView.

The changes here are designed to both
a) allow clients to increase that timeout, and
b) present a clear error when a timeout does occur here.

The biggest part I'm not sure of: should the timeout provided to `render_async/2` be _split_ between the `:async_pids` lookup and the actual message receive (as written), or should it only apply to the message receive portion? Prior to this change, it was the latter, which means the whole function call could have taken up to 5 seconds + the indicated timeout. If the timeout is _not_ shared between the two steps, though, it feels quite awkward; either there's no way to configure the `:async_pids` timeout on a per-call basis (you would only be able to do so using the application config variable), or the function would need to accept *two* timeouts.
Comment on lines +1068 to +1069
timeout_ref = make_ref()
Process.send_after(self(), {timeout_ref, :timeout}, timeout)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to starting the timeout before the GenServer call, so that the single timeout is shared between both the process lookup and the actual async work. (See PR description for a question on the tradeoffs here.)

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

Successfully merging this pull request may close these issues.

1 participant