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

Add Plug.Test.sent_chunks/2 #1160

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wojtekmach
Copy link
Contributor

Before this patch there was no way to reconstruct the invidual chunks that were sent. All we got was the full resulting body in conn.resp_body.

For completeness, I believe instead of messaging we could store chunks in a list in the test adapter state and simply append:

- def send_chunked(state, _status, _headers), do: {:ok, "", %{state | chunks: ""}}
+ def send_chunked(state, _status, _headers), do: {:ok, "", %{state | chunks: []}}

- def chunk(%{owner: owner, ref: ref, chunks: chunks} = state, chunk) do
-   send(owner, {ref, :chunk, chunk})
-   body = chunks <> IO.iodata_to_binary(chunk)
-   {:ok, body, %{state | chunks: body}}
- end
+ def chunk(%{owner: owner, ref: ref, chunks: chunks} = state, chunk) do
+   chunk = IO.iodata_to_binary(chunk)
+   body = IO.iodata_to_binary([chunks, chunk])
+   {:ok, body, %{state | chunks: chunks ++ [chunk]}}
+ end

but I was following the existing functions sent_informs and sent_upgrades.

My use case is to be able to test response streaming using Req :plug adapter:

req =
  Req.new(
    plug: fn conn ->
      conn = Plug.Conn.send_chunked(conn, 200)
      {:ok, conn} = Plug.Conn.chunk(conn, "foo")
      {:ok, conn} = Plug.Conn.chunk(conn, "bar")
      conn
    end,
    into: []
  )

resp = Req.request!(req)
assert resp.body == ["foo", "bar"]

@wojtekmach
Copy link
Contributor Author

Oh no, I just realised there's a Plug.Conn.register_before_chunk/2 on main which I can totally use instead. Sorry about that!

@wojtekmach wojtekmach closed this Sep 3, 2023
@wojtekmach
Copy link
Contributor Author

Given register_before_chunk got reverted, can I pursue this?

@josevalim
Copy link
Member

Sounds good to me!

Before this patch there was no way to reconstruct the invidual chunks
that were sent. All we got was the full resulting body in
`conn.resp_body`.

For completeness, I believe instead of messaging we could store chunks in a list
in the test adapter state and simply append:

    - def send_chunked(state, _status, _headers), do: {:ok, "", %{state | chunks: ""}}
    + def send_chunked(state, _status, _headers), do: {:ok, "", %{state | chunks: []}}

    - def chunk(%{owner: owner, ref: ref, chunks: chunks} = state, chunk) do
    -   send(owner, {ref, :chunk, chunk})
    -   body = chunks <> IO.iodata_to_binary(chunk)
    -   {:ok, body, %{state | chunks: body}}
    - end
    + def chunk(%{owner: owner, ref: ref, chunks: chunks} = state, chunk) do
    +   chunk = IO.iodata_to_binary(chunk)
    +   body = IO.iodata_to_binary([chunks, chunk])
    +   {:ok, body, %{state | chunks: chunks ++ [chunk]}}
    + end

but I was following the existing functions `sent_informs` and `sent_upgrades`.

My use case is to be able to test response streaming using Req :plug
adapter:

    req =
      Req.new(
        plug: fn conn ->
          conn = Plug.Conn.send_chunked(conn, 200)
          {:ok, conn} = Plug.Conn.chunk(conn, "foo")
          {:ok, conn} = Plug.Conn.chunk(conn, "bar")
          conn
        end,
        into: []
      )

    resp = Req.request!(req)
    assert resp.body == ["foo", "bar"]
Comment on lines +127 to +133
test "sent chunks on send_resp" do
conn = conn(:get, "/")
conn = Plug.Conn.send_resp(conn, 200, "foobar")

assert conn.resp_body == "foobar"
assert Plug.Test.sent_chunks(conn) == []
end
Copy link
Contributor Author

@wojtekmach wojtekmach Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When doing Req.get(plug: plug, into: stream), I'm using this new sent_chunks/1 function however as seen in this test it doesn't behave correctly (for Req) when the plug is not sending chunks. Maybe the function should raise?

@wojtekmach
Copy link
Contributor Author

wojtekmach commented Mar 5, 2024

Turns out process messaging isn't a great fit for my main use case for this which is Req :plug adapter. The reason is if someone ends up with something like this:

test "foo" do
  defmodule Foo do
    use GenServer

    def start_link(arg) do
      GenServer.start_link(__MODULE__, arg)
    end

    @impl true
    def init(_) do
      Req.get!(
        plug: fn conn ->
          Plug.Conn.send_resp(conn, 200, "ok")
        end
      )

      {:ok, %{}}
    end
  end

  start_supervised!(Foo)
  Process.sleep(100)
end

The GenServer ends up receiving these Plug.Test messages which is unexpected. This was reported in wojtekmach/req#316 and I fixed it in Req by consuming these.

In any case, I'd be still keen on a Plug.Test.sent_chunks function that receives accumulated chunks stored in the adapter state as mentioned in the PR description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants