fix: do not crash boot when routes are missing (#701)

lib/beacon/router.ex

@@ -145,6 +145,9 @@ defmodule Beacon.Router do
           get "/__beacon_assets__/css-:md5", Beacon.Web.AssetsController, :css, assigns: %{site: opts[:site]}
           get "/__beacon_assets__/js-:md5", Beacon.Web.AssetsController, :js, assigns: %{site: opts[:site]}
 
+          # simulate a beacon page inside site prefix so we can check this site is reachable?/2
+          get "/__beacon_check__", Beacon.Web.CheckController, :check, metadata: %{site: opts[:site]}
+
           live "/*path", Beacon.Web.PageLive, :path
         end
       end
@@ -250,27 +253,13 @@ defmodule Beacon.Router do
   @doc false
   # Tells if a `beacon_site` is reachable in the current environment.
   #
-  # Supposed the following router:
-  #
-  #   scope "/", MyAppWeb, host: ["beacon-site-a.fly.dev"] do
-  #     pipe_through [:browser]
-  #     beacon_site "/", site: :site_a
-  #   end
-  #
-  #   scope "/", MyAppWeb, host: ["beacon-site-b.fly.dev"] do
-  #     pipe_through [:browser]
-  #     beacon_site "/", site: :site_b
-  #   end
-  #
-  # On a node deployed to beacon-site-a.fly.dev, the second `beacon_site`
-  # will never match, so starting `:site_b` is a waste of resources
-  # and a common cause of problems on BeaconLiveAdmin since we can't resolve URLs properly.
-  #
-  # Similarly, if a `get "/"` is added _before_ either `beacon_site` that `get` would always
-  # match and invalidate the `beacon_site` mount.
+  # It's considered reachable if a dynamic page can be served on the site prefix.
   def reachable?(%Beacon.Config{} = config, opts \\ []) do
     %{site: site, endpoint: endpoint, router: router} = config
-    function_exported?(router, :__beacon_scoped_prefix_for_site__, 1) && reachable?(site, endpoint, router, opts)
+    reachable?(site, endpoint, router, opts)
+  rescue
+    # missing router or missing beacon macros in the router
+    _ -> false
   end
 
   defp reachable?(site, endpoint, router, opts) do
@@ -281,14 +270,10 @@ defmodule Beacon.Router do
         router.__beacon_scoped_prefix_for_site__(site)
       end)
 
-    case Phoenix.Router.route_info(router, "GET", prefix, host) do
-      # bypass and allow booting beacon sites even though there's a route conflict
-      # but only for root paths, for example:
-      #   live /
-      #   beacon_site /
-      # that's because even though they share the same prefix,
-      # beacon can still serve pages at /:page
-      %{route: "/"} ->
+    path = Beacon.Router.sanitize_path(prefix <> "/__beacon_check__")
+
+    case Phoenix.Router.route_info(router, "GET", path, host) do
+      %{site: ^site, plug: Beacon.Web.CheckController} ->
         true
 
       %{phoenix_live_view: {Beacon.Web.PageLive, _, _, %{extra: %{session: %{"beacon_site" => ^site}}}}} ->

lib/beacon/web/controllers/check_controller.ex

@@ -0,0 +1,16 @@
+defmodule Beacon.Web.CheckController do
+  @moduledoc false
+  # TODO: replace CheckController with a plug in https://github.com/BeaconCMS/beacon/pull/694
+
+  import Plug.Conn
+
+  def init(conn), do: conn
+
+  def call(conn, _) do
+    conn
+    |> put_resp_header("content-type", "text/plain")
+    |> put_resp_header("cache-control", "public, max-age=31536000, immutable")
+    |> send_resp(200, "")
+    |> halt()
+  end
+end

test/beacon/router_test.exs

@@ -63,32 +63,35 @@ defmodule Beacon.RouterTest do
   end
 
   describe "reachable?" do
-    setup do
-      site = :host_test
-      config = Beacon.Config.fetch!(site)
-      [config: config]
+    defp config(site, opts \\ []) do
+      Map.merge(
+        Beacon.Config.fetch!(site),
+        Enum.into(opts, %{router: Beacon.BeaconTest.ReachTestRouter})
+      )
     end
 
-    test "match existing host", %{config: config} do
-      valid_host = "host.com"
-      assert Router.reachable?(config, host: valid_host)
+    test "match existing host" do
+      config = config(:host_test)
+      assert Router.reachable?(config, host: "host.com", prefix: "/host_test")
     end
 
-    test "existing nested conflicting route", %{config: config} do
-      valid_host = "host.com"
-      refute Router.reachable?(config, host: valid_host, prefix: "/nested/some_page")
+    test "existing nested conflicting route" do
+      config = config(:not_booted)
+      refute Router.reachable?(config, host: nil, prefix: "/conflict")
     end
 
-    test "with no specific host", %{config: config} do
+    test "root path with no host" do
+      config = config(:my_site)
       assert Router.reachable?(config, host: nil)
     end
 
-    test "do not match any existing host/path", %{config: config} do
-      refute Router.reachable?(config, host: nil, prefix: "/nested/invalid")
+    test "not reachable when does not match any existing host/path" do
+      config = config(:my_site)
+      refute Router.reachable?(config, host: nil, prefix: "/other")
     end
 
     test "router without beacon routes" do
-      config = Beacon.Config.fetch!(:no_routes)
+      config = config(:my_site, router: Beacon.BeaconTest.NoRoutesRouter)
       refute Router.reachable?(config)
     end
   end

test/support/routers.ex

@@ -2,7 +2,7 @@ defmodule Beacon.BeaconTest.NoRoutesRouter do
   use Beacon.BeaconTest.Web, :router
 end
 
-defmodule Beacon.BeaconTest.Router do
+defmodule Beacon.BeaconTest.ReachTestRouter do
   use Beacon.BeaconTest.Web, :router
   use Beacon.Router
 
@@ -14,24 +14,43 @@ defmodule Beacon.BeaconTest.Router do
     plug :put_secure_browser_headers
   end
 
-  scope "/nested" do
+  scope path: "/host_test", host: "host.com" do
     pipe_through :browser
-    beacon_site "/site", site: :booted
-    beacon_site "/media", site: :s3_site
+    beacon_site "/", site: :host_test
   end
 
-  scope "/" do
+  scope path: "/conflict" do
     pipe_through :browser
+    live "/:page", Beacon.BeaconTest.LiveViews.FooBarLive
+    beacon_site "/", site: :not_booted
+  end
 
-    live_session :default do
-      live "/", Beacon.BeaconTest.LiveViews.FooBarLive
-      live "/nested/:page", Beacon.BeaconTest.LiveViews.FooBarLive
-    end
+  scope path: "/my_site" do
+    pipe_through :browser
+    beacon_site "/", site: :my_site
   end
 
-  scope path: "/", host: "host.com" do
+  scope path: "/other" do
     pipe_through :browser
-    beacon_site "/", site: :host_test
+  end
+end
+
+defmodule Beacon.BeaconTest.Router do
+  use Beacon.BeaconTest.Web, :router
+  use Beacon.Router
+
+  pipeline :browser do
+    plug :accepts, ["html"]
+    plug :fetch_session
+    plug :fetch_live_flash
+    plug :protect_from_forgery
+    plug :put_secure_browser_headers
+  end
+
+  scope "/nested" do
+    pipe_through :browser
+    beacon_site "/site", site: :booted
+    beacon_site "/media", site: :s3_site
   end
 
   # `alias` is not really used but is present here to verify that `beacon_site` has no conflicts with custom aliases