Skip to content

Commit

Permalink
fix: crash recovery
Browse files Browse the repository at this point in the history
Start Beacon before Endpoint so requests are blocked until the host app
is ready to accept them, otherwise we expose Beacon to race conditions
causing blank pages and boot errors.
  • Loading branch information
leandrocp committed Jan 16, 2025
1 parent 6e69859 commit fdb7c12
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 20 deletions.
4 changes: 2 additions & 2 deletions dev.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1220,8 +1220,8 @@ Task.start(fn ->
children = [
Demo.Repo,
{Phoenix.PubSub, [name: Demo.PubSub]},
DemoWeb.Endpoint,
{Beacon, sites: [dev_site, dy_site]}
{Beacon, sites: [dev_site, dy_site]},
DemoWeb.Endpoint
]

{:ok, _} = Supervisor.start_link(children, strategy: :one_for_one)
Expand Down
5 changes: 0 additions & 5 deletions guides/general/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,3 @@ Note that if you're using `:host` on the scope and running in `localhost`,
consider adding `"localhost"` to the list of allowed hosts.

Also check the [Beacon.Router](https://hexdocs.pm/beacon/Beacon.Router.html) for more information.

## RuntimeError - could not find persistent term for endpoint

`Beacon` should be started after your host's `Endpoint`, please review the application children
and make sure is declared after the endpoint.
19 changes: 19 additions & 0 deletions guides/upgrading/v0.4.0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Upgrading to v0.4.0

## Move `Beacon` before the endpoint in application.ex

Open the file `application.ex`, find the Beacon tuple in the list of `children`,
and move it to before the endpoint(s) declarations:

```elixir
@impl true
def start(_type, _args) do
children = [
# ...
{Beacon, [sites: [Application.fetch_env!(:beacon, :my_site)]]}, # <- moved to before `MyAppWeb.Endpoint`
MyAppWeb.Endpoint
]
```

In v0.3.0 the order was inverted which caused a regression on crash recoveries.

13 changes: 6 additions & 7 deletions lib/beacon/beacon.ex
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,12 @@ defmodule Beacon do
children = [
MyApp.Repo,
{Phoenix.PubSub, name: MyApp.PubSub},
MyAppWeb.Endpoint,
{Beacon,
[
sites: [
Application.fetch_env!(:beacon, :my_site)
]
]}
{Beacon, [
sites: [
Application.fetch_env!(:beacon, :my_site)
]
]},
MyAppWeb.Endpoint
]
opts = [strategy: :one_for_one, name: MyApp.Supervisor]
Expand Down
22 changes: 22 additions & 0 deletions lib/beacon/private.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@ defmodule Beacon.Private do

# Should be avoided as much as possible.

@doc """
Fetch the host app `:otp_app` from the Repo config.
"""
def otp_app!(%Beacon.Config{repo: repo}) do
repo.config()[:otp_app] || raise Beacon.RuntimeError, "failed to discover :otp_app"
rescue
_ -> reraise Beacon.RuntimeError, [message: "failed to discover :otp_app, make sure Repo is started before Beacon"], __STACKTRACE__
end

@phoenix_live_view_version to_string(Application.spec(:phoenix_live_view)[:vsn])

@doc """
Expand Down Expand Up @@ -48,4 +57,17 @@ defmodule Beacon.Private do
Phoenix.LiveView.Route.live_link_info(endpoint, router, url)
end
end

def endpoint_config(otp_app, endpoint) do
Phoenix.Endpoint.Supervisor.config(otp_app, endpoint)[:url]
end

def endpoint_host(otp_app, endpoint) do
url_config = endpoint_config(otp_app, endpoint)[:url]
host_to_binary(url_config[:host] || "localhost")
end

# https://github.com/phoenixframework/phoenix/blob/4ebefb9d1f710c576f08c517f5852498dd9b935c/lib/phoenix/endpoint/supervisor.ex#L301-L302
defp host_to_binary({:system, env_var}), do: host_to_binary(System.get_env(env_var))
defp host_to_binary(host), do: host
end
7 changes: 4 additions & 3 deletions lib/beacon/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,16 @@ defmodule Beacon.Router do
#
# It's considered reachable if a dynamic page can be served on the site prefix.
def reachable?(%Beacon.Config{} = config, opts \\ []) do
otp_app = Beacon.Private.otp_app!(config)
%{site: site, endpoint: endpoint, router: router} = config
reachable?(site, endpoint, router, opts)
reachable?(site, endpoint, router, otp_app, opts)
rescue
# missing router or missing beacon macros in the router
_ -> false
end

defp reachable?(site, endpoint, router, opts) do
host = Keyword.get_lazy(opts, :host, fn -> endpoint.host() end)
defp reachable?(site, endpoint, router, otp_app, opts) do
host = Keyword.get_lazy(opts, :host, fn -> Beacon.Private.endpoint_host(otp_app, endpoint) end)

prefix =
Keyword.get_lazy(opts, :prefix, fn ->
Expand Down
3 changes: 2 additions & 1 deletion lib/mix/tasks/beacon.gen.site.ex
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ defmodule Mix.Tasks.Beacon.Gen.Site do
quote do
[sites: [Application.fetch_env!(:beacon, unquote(site))]]
end}},
after: [repo, endpoint],
after: [repo],
before: [endpoint],
opts_updater: fn zipper ->
with {:ok, zipper} <-
Igniter.Code.Keyword.put_in_keyword(
Expand Down
9 changes: 7 additions & 2 deletions test/mix/tasks/gen_site_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,13 @@ defmodule Mix.Tasks.Beacon.GenSiteTest do
project
|> Igniter.compose_task("beacon.gen.site", @opts_my_site)
|> assert_has_patch("lib/test/application.ex", """
20 + | TestWeb.Endpoint,
21 + | {Beacon, [sites: [Application.fetch_env!(:beacon, :my_site)]]}
...|
12 12 | Test.Repo,
13 13 | {DNSCluster, query: Application.get_env(:test, :dns_cluster_query) || :ignore},
14 + | {Beacon, [sites: [Application.fetch_env!(:beacon, :my_site)]]},
14 15 | {Phoenix.PubSub, name: Test.PubSub},
15 16 | # Start the Finch HTTP client for sending emails
...|
""")
end

Expand Down

0 comments on commit fdb7c12

Please sign in to comment.