Repository: phoenixframework/phoenix
Pull request #4217 (2021-02-24 19:02:36)

emit [:phoenix, :router_dispatch, :stop] on halted Plug.Conn
Ensures the `[:phoenix, :router_dispatch, :stop]` event is emitted when either a plug in the router or a controller halts a conn using `Plug.Conn.halt/2`.

Fixes [#4216][1]

[1]: https://github.com/phoenixframework/phoenix/issues/4216

fivetanley: Should I port this to a pull request for the `1.5` branch as well?
mcrumm: @fivetanley this PR is fine -- we'll take care of the backport, thanks!

Patch:
diff --git a/lib/phoenix/endpoint.ex b/lib/phoenix/endpoint.ex
index ec1158f63..49ee33ffc 100644
--- a/lib/phoenix/endpoint.ex
+++ b/lib/phoenix/endpoint.ex
@@ -218,7 +218,7 @@ defmodule Phoenix.Endpoint do
       `c:broadcast_from/4`, `c:broadcast_from!/4`, `c:local_broadcast/3`,
       and `c:local_broadcast_from/4`
 
-    * for configuration: `c:start_link/0`, `c:config/2`, and `c:config_change/2`
+    * for configuration: `c:start_link/1`, `c:config/2`, and `c:config_change/2`
 
     * as required by the `Plug` behaviour: `c:Plug.init/1` and `c:Plug.call/2`
 
diff --git a/lib/phoenix/router.ex b/lib/phoenix/router.ex
index 2ec096cfb..b5e880690 100644
--- a/lib/phoenix/router.ex
+++ b/lib/phoenix/router.ex
@@ -346,6 +346,9 @@ defmodule Phoenix.Router do
 
     case pipeline.(conn) do
       %Plug.Conn{halted: true} = halted_conn ->
+        measurements = %{duration: System.monotonic_time() - start}
+        metadata = %{metadata | conn: halted_conn}
+        :telemetry.execute([:phoenix, :router_dispatch, :stop], measurements, metadata)
         halted_conn
       %Plug.Conn{} = piped_conn ->
         try do


Test patch:
diff --git a/test/phoenix/router/routing_test.exs b/test/phoenix/router/routing_test.exs
index 888a50571..8dc01c095 100644
--- a/test/phoenix/router/routing_test.exs
+++ b/test/phoenix/router/routing_test.exs
@@ -23,6 +23,11 @@ defmodule Phoenix.Router.RoutingTest do
     def any(conn, _params), do: text(conn, "users any")
     def raise(_conn, _params), do: raise("boom")
     def exit(_conn, _params), do: exit(:boom)
+    def halt(conn, _params) do
+      conn
+      |> send_resp(401, "Unauthorized")
+      |> halt()
+    end
   end
 
   defmodule LogLevel do
@@ -44,6 +49,7 @@ defmodule Phoenix.Router.RoutingTest do
     get "/backups/*path", UserController, :image
     get "/static/images/icons/*image", UserController, :image
     get "/exit", UserController, :exit
+    get "/halt-controller", UserController, :halt
 
     trace("/trace", UserController, :trace)
     options "/options", UserController, :options
@@ -60,9 +66,18 @@ defmodule Phoenix.Router.RoutingTest do
     get "/no_log", SomePlug, [], log: false
     get "/fun_log", SomePlug, [], log: {LogLevel, :log_level, []}
     get "/users/:user_id/files/:id", UserController, :image
+
+    scope "/halt-plug" do
+      pipe_through :halt
+      get "/", UserController, :raise
+    end
+
     get "/*path", UserController, :not_found
 
     defp noop(conn, _), do: conn
+    defp halt(conn, _) do
+      conn |> Plug.Conn.send_resp(401, "Unauthorized") |> halt()
+    end
   end
 
   setup do
@@ -285,6 +300,38 @@ defmodule Phoenix.Router.RoutingTest do
                        {_, %{route: "/users/:id"}, _}}
     end
 
+    test "phoenix.router_dispatch.start and .stop are emitted when conn halted in router" do
+      conn = call(Router, :get, "/halt-plug")
+
+      assert conn.halted
+      assert conn.status == 401
+
+      assert_received {:telemetry_event, @router_start_event,
+                       {_, %{route: "/halt-plug"}, _}}
+
+      assert_received {:telemetry_event, @router_stop_event,
+                       {_, %{route: "/halt-plug"}, _}}
+
+      refute_received {:telemetry_event, @router_exception_event,
+                       {_, %{route: "/halt-plug"}, _}}
+    end
+
+    test "phoenix.router_dispatch.start and .stop are emitted when conn is halted in controller" do
+      conn = call(Router, :get, "/halt-controller")
+
+      assert conn.halted
+      assert conn.status == 401
+
+      assert_received {:telemetry_event, @router_start_event,
+                       {_, %{route: "/halt-controller"}, _}}
+
+      assert_received {:telemetry_event, @router_stop_event,
+                       {_, %{route: "/halt-controller"}, _}}
+
+      refute_received {:telemetry_event, @router_exception_event,
+                       {_, %{route: "/halt-controller"}, _}}
+    end
+
     test "phoenix.router_dispatch.start and .exception are emitted on crash" do
       assert_raise Plug.Conn.WrapperError, ~r/UndefinedFunctionError/, fn ->
         call(Router, :get, "route_that_crashes")