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

fix: crash recovery #729

Merged
merged 3 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
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
13 changes: 9 additions & 4 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 All @@ -148,8 +153,8 @@ defmodule Mix.Tasks.Beacon.GenSiteTest do
|> apply_igniter!()
|> Igniter.compose_task("beacon.gen.site", @opts_other_site)
|> assert_has_patch("lib/test/application.ex", """
21 - | {Beacon, [sites: [Application.fetch_env!(:beacon, :my_site)]]}
21 + | {Beacon, [sites: [Application.fetch_env!(:beacon, :my_site), Application.fetch_env!(:beacon, :other)]]}
14 - | {Beacon, [sites: [Application.fetch_env!(:beacon, :my_site)]]},
14 + | {Beacon, [sites: [Application.fetch_env!(:beacon, :my_site), Application.fetch_env!(:beacon, :other)]]},
""")
end
end
Expand Down
Loading