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 support for async requests #228

Merged
merged 34 commits into from
Jun 13, 2023
Merged
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9daf971
ci: fix elixir.yml indentation
zachallaun May 30, 2023
8adbf5d
ci: update elixir version and add otp 26
zachallaun May 30, 2023
8aee995
wip: overly simple first pass at `Finch.async_request/3`
zachallaun May 30, 2023
785616c
wip: `Finch.cancel_async_request/2` (test failing)
zachallaun May 31, 2023
bb3beaa
wip: push async api to the adapters
zachallaun Jun 2, 2023
186385e
test: more reliable tests & test async cancel with http1 pool
zachallaun Jun 3, 2023
fc97489
fix: exit async req process if caller exits
zachallaun Jun 3, 2023
5cbba4e
test: add tests for success and error async requests to http1 pool test
zachallaun Jun 3, 2023
4f7e31e
refactor: use request_ref in existing http2 pool messages
zachallaun Jun 3, 2023
00e1752
refactor: use request_ref to cancel http2 requests
zachallaun Jun 3, 2023
f29b841
wip: implement async_request & cancelation for http2
zachallaun Jun 4, 2023
69d37d6
refactor: unify sync and async http2 request flow
zachallaun Jun 4, 2023
0b65bc8
refactor: clean up http2 request handling
zachallaun Jun 4, 2023
e43a005
fix: return consistent errors in http2 pool
zachallaun Jun 4, 2023
6632115
test(http2): add async request tests for error conditions
zachallaun Jun 6, 2023
8968a2e
test: only start finch and server when needed
zachallaun Jun 6, 2023
4d63e0b
fix(http2): cancel async requests if calling pid exits
zachallaun Jun 6, 2023
a1f81d2
test(http1): improve flaky pool tests
zachallaun Jun 6, 2023
7af3595
docs: improve docs and typespecs for `async_request/3`
zachallaun Jun 6, 2023
42d3fbe
test: no doctests on main module
zachallaun Jun 7, 2023
6e75444
test: http2 telemetry + always detach telemetry after test
zachallaun Jun 7, 2023
3142972
fix: don't send `{ref, :ok}` to async requests callers
zachallaun Jun 7, 2023
cf3e822
refactor: clean up http2 pool request sending
zachallaun Jun 7, 2023
c12aeec
fix: cancel http2 request when in connected_read_only
zachallaun Jun 8, 2023
efdb914
refactor: don't store extra state in request stream struct
zachallaun Jun 8, 2023
1289f7d
docs: note which telemetry events are http1-only
zachallaun Jun 8, 2023
d4f86b5
refactor: rename internal helper
zachallaun Jun 8, 2023
8bae1ce
chore: split out request_opt type from request_opts
zachallaun Jun 11, 2023
9ae43ef
chore: spawn linked async req process in http1 pool
zachallaun Jun 11, 2023
f89106b
refactor: simplify request_ref structure
zachallaun Jun 11, 2023
83b538a
test: test async cancelation when caller exits normally or abnormally
zachallaun Jun 11, 2023
f53e8e4
docs: additional documentation for async requests
zachallaun Jun 11, 2023
4d259b6
test: less finicky http1 pool tests
zachallaun Jun 11, 2023
aa2ced7
remove otp 26 from test matrix due to unexpected changes in ssl optio…
sneako Jun 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
test: more reliable tests & test async cancel with http1 pool
zachallaun committed Jun 4, 2023
commit 186385e75ccd34d9e06d371a80ba03bef8f20eb4
4 changes: 3 additions & 1 deletion lib/finch/http1/pool.ex
Original file line number Diff line number Diff line change
@@ -103,7 +103,9 @@ defmodule Finch.HTTP1.Pool do
end

@impl Finch.Pool
def cancel_async_request(_request_ref) do
def cancel_async_request({_, _, _, pid}) do
Process.exit(pid, :shutdown)
:ok
end

@impl NimblePool
2 changes: 1 addition & 1 deletion test/finch/http1/integration_proxy_test.exs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ defmodule Finch.HTTP1.IntegrationProxyTest do
alias Finch.HTTP1Server

setup_all do
port = 4002
port = 4004

# Not quite a proper forward proxy server, but good enough
{:ok, _} = HTTP1Server.start(port)
30 changes: 30 additions & 0 deletions test/finch/http1/pool_test.exs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
defmodule Finch.HTTP1.PoolTest do
use FinchCase, async: true

alias Finch.HTTP1Server

@tag capture_log: true
test "should terminate pool after idle timeout", %{bypass: bypass, finch_name: finch_name} do
test_name = to_string(finch_name)
@@ -47,4 +49,32 @@ defmodule Finch.HTTP1.PoolTest do

:telemetry.detach(test_name)
end

describe "async requests" do
@describetag bypass: false

setup do
port = 4005
endpoint = "http://localhost:#{port}"

start_supervised!({HTTP1Server, port: port})

{:ok, endpoint: endpoint}
end

test "can be canceled with cancel_async_request/1", %{
finch_name: finch_name,
endpoint: endpoint
} do
start_supervised!({Finch, name: finch_name, pools: %{default: [protocol: :http1]}})

ref =
Finch.build(:get, endpoint <> "/stream/1/50")
|> Finch.async_request(finch_name)

assert_receive {^ref, {:status, 200}}
Finch.HTTP1.Pool.cancel_async_request(ref)
refute_receive {^ref, {:data, _}}
end
end
end
26 changes: 0 additions & 26 deletions test/finch_test.exs
Original file line number Diff line number Diff line change
@@ -767,32 +767,6 @@ defmodule FinchTest do
for _ <- 1..5, do: assert_receive({^request_ref, {:data, "chunk-data"}})
assert_receive {^request_ref, :done}
end

@tag :skip
test "can be canceled with cancel_async_request/1", %{bypass: bypass, finch_name: finch_name} do
start_supervised!({Finch, name: finch_name})

Bypass.expect(bypass, fn conn ->
conn = Plug.Conn.send_chunked(conn, 200)

:timer.sleep(10)

Enum.reduce(1..5, conn, fn _, conn ->
{:ok, conn} = Plug.Conn.chunk(conn, "chunk-data")
conn
end)
end)

request_ref =
Finch.build(:get, endpoint(bypass))
|> Finch.async_request(finch_name)

assert_receive {^request_ref, {:status, 200}}, 10

Finch.cancel_async_request(request_ref)

refute_receive {^request_ref, {:data, _}}
end
end

defp get_pools(name, shp) do
10 changes: 8 additions & 2 deletions test/support/finch_case.ex
Original file line number Diff line number Diff line change
@@ -8,8 +8,14 @@ defmodule FinchCase do
end
end

setup %{test: finch_name} do
{:ok, bypass: Bypass.open(), finch_name: finch_name}
setup context do
bypass =
case context do
%{bypass: false} -> []
%{} -> [bypass: Bypass.open()]
end

{:ok, bypass ++ [finch_name: context.test]}
end

@doc "Returns the url for a Bypass instance"
48 changes: 33 additions & 15 deletions test/support/http1_server.ex
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
defmodule Finch.HTTP1Server do
@moduledoc false

def start(port) do
children = [
Plug.Adapters.Cowboy.child_spec(
scheme: :http,
plug: Finch.HTTP1Server.PlugRouter,
options: [
port: port,
otp_app: :finch,
protocol_options: [
idle_timeout: 3_000,
request_timeout: 10_000
]
def child_spec(opts) do
Plug.Adapters.Cowboy.child_spec(
scheme: :http,
plug: Finch.HTTP1Server.PlugRouter,
options: [
port: Keyword.fetch!(opts, :port),
otp_app: :finch,
protocol_options: [
idle_timeout: 3_000,
request_timeout: 10_000
]
)
]
]
)
end

Supervisor.start_link(children, strategy: :one_for_one)
def start(port) do
Supervisor.start_link([child_spec(port: port)], strategy: :one_for_one)
end
end

@@ -37,4 +37,22 @@ defmodule Finch.HTTP1Server.PlugRouter do
|> send_resp(200, "Hello #{name}!")
|> halt()
end

get "/wait/:delay" do
delay = conn.params["delay"] |> String.to_integer()
Process.sleep(delay)
send_resp(conn, 200, "ok")
end

get "/stream/:num/:delay" do
num = conn.params["num"] |> String.to_integer()
delay = conn.params["delay"] |> String.to_integer()
conn = send_chunked(conn, 200)

Enum.reduce(1..num, conn, fn i, conn ->
Process.sleep(delay)
{:ok, conn} = chunk(conn, "chunk-#{i}\n")
conn
end)
end
end