elixir-plug / plug

Compose web applications with functions
https://hex.pm/packages/plug
Other
2.88k stars 586 forks source link

Plug.Conn.chunk/2 never returns an error #1040

Closed daniel-jodlos closed 3 years ago

daniel-jodlos commented 3 years ago

I wanted to use Plug to implement chunked HTTP streaming in my application, but run into some unexpected issues detecting that client has disconnected. According to the documentation, it should be as straightforward as checking if chunk/2 returned {:error, reason}, but it always returns {:ok, conn}, even after the socket terminates cleanly.

After a short investigation, I came to the conclusion that implementation of Plug.Cowboy.Conn.chunk/2 inside the Cowboy adapter is to blame, as cowboy_req:stream_body always returns :ok.

Would it be possible to detect that client has disconnected in any other way?

josevalim commented 3 years ago

If Cowboy doesn’t surface this information, there is nothing we can do. We do support error returns though in case other adapters decide to implement it.

Gazler commented 3 years ago

chowboy_req:stream_body used to return {:error, reason} in the case of a disconnect https://github.com/ninenines/cowboy/blob/1.0.x/src/cowboy_req.erl#L952

However, this was changed in Cowboy 2 as this was not a reliable way to detect a disconnect. The recommended way is to use a monitor and monitor for a down event.

Something like:

defmodule HTTPStreamWatcher do
  use GenServer, restart: :transient

  def start_link(opts \\ []) do
    GenServer.start_link(__MODULE__, opts)
  end

  def init(opts) do
    ref = Process.monitor(opts.pid)
    {:ok, %{ref: ref}}
  end

  def handle_info({:DOWN, ref, :process, _, :normal}, %{ref: ref} = state) do
    # normal terminate
    {:stop, :normal, state}
  end

  def handle_info({:DOWN, ref, :process, _, reason}, %{ref: ref} = state)
      when reason != :normal do
    # This was a disconnect
    {:stop, :normal, state}
  end

  def handle_info(_, state) do
    {:noreply, state}
  end
end

You'll want to start this with a dynamic supervisor. https://hexdocs.pm/elixir/1.12/DynamicSupervisor.html