Support returning {:ok, payload} from adapter.inform/3

[wojtekmach]

There is a minor Bandit bug where it looks like it does not support inform but it definitely does. Turns out using Plug.Conn.inform/3 worked but inform!/3 did not. The bug in Bandit was wrong return value which went unchecked in inform/3 but got incorrectly interpreted in inform!/3.

This is not a breaking change for inform!/3 as we were already raising but it could be considered a breaking change for inform/3 but in this particular case it seemed warranted to me.

cc @mtrudel

[mtrudel]

It'd actually be real nice if we could instead add a match on {:ok, adapter} and return that back to the caller. As it is this makes it impossible to track state (namely metrics) past an inform call. WDYT?

[wojtekmach]

@mtrudel to be a bit more precise, do you have something like this in mind?

  def inform(%Conn{} = conn, status, headers \\ []) do
    status_code = Plug.Conn.Status.code(status)
    adapter_inform(conn, status_code, headers)
-   conn
  end

  # simplified for brevity
  defp adapter_inform(%Conn{adapter: {adapter, payload}}, status, headers) do
-   :ok = adapter.inform(payload, status, headers)
+   {:ok, payload} = adapter.inform(payload, status, headers)
+   put_in(conn.adapter, {adapter, payload})
  end

With almost zero knowledge about this side of Plug, this looks sensible to me. 😅

[mtrudel]

Conceptually, yeah. Though in an additive sense (with a case statement) so as to be backwards compatible. Like here (and without the change to the private function further down)

[wojtekmach]

I believe this is ready. I renamed the PR to "Support returning {:ok, payload} from adapter.inform/3".

[wojtekmach]

I can add a catch-all clause for backwards compatibility, let me know. If so perhaps with a warning?

Suggested change

	{:error, :not_supported} ->
	conn
	{:error, :not_supported} ->
	conn
	_other ->
	conn

there's no need for a catch-all clause in inform!/3 as it already was rasing.

[josevalim]

💚 💙 💜 💛 ❤️