diff --git a/guides/http2_guide.md b/guides/http2_guide.md index 878e0584..18c3dc44 100644 --- a/guides/http2_guide.md +++ b/guides/http2_guide.md @@ -375,6 +375,25 @@ The option applies to whole-body requests and to streamed bodies sent with `hack ## Error Handling +A request the server turns away before processing it is sent again, once. +That is a stream above the `last_stream_id` of a GOAWAY (RFC 9113 §6.8), or +one reset with `REFUSED_STREAM` (§8.7): the server guarantees it did nothing +with the request, so hackney repeats it whatever the method: on a fresh +connection, or on the same pooled connection when a `REFUSED_STREAM` left it +serving. The streams a GOAWAY accepted still complete on the draining +connection. + +The refusal still reaches the caller, as `{error, {goaway, ErrorCode}}` or +`{error, {stream_error, refused_stream}}`, when: + +- the second attempt is refused too; +- the body is a function (`fun/0` or `{fun/1, State}`), which the first + attempt may already have consumed; +- the refusal arrives after `request/5` has returned: a streamed request body + (`stream`) gets it from `send_body/2` or `start_response/1`, an `async` + request as a `{hackney_response, Ref, {error, _}}` message; +- the request was made on an explicit connection with `send_request/2`. + HTTP/2 specific errors: ```erlang @@ -382,7 +401,7 @@ case hackney:get(URL) of {ok, Status, Headers, Body} -> ok; {error, {goaway, ErrorCode}} -> - %% Peer sent GOAWAY + %% Refused twice, or refused after request/5 had returned io:format("HTTP/2 GOAWAY: ~p~n", [ErrorCode]); {error, {stream_error, ErrorCode}} -> %% Peer sent RST_STREAM for this request diff --git a/guides/http3_guide.md b/guides/http3_guide.md index 791c8ecc..b498c177 100644 --- a/guides/http3_guide.md +++ b/guides/http3_guide.md @@ -404,6 +404,15 @@ Both HTTP/2 and HTTP/3 use lowercase header names: ## Error Handling +A request the server turns away before processing it is sent again, once: a +stream at or above the identifier of a GOAWAY (RFC 9114 §5.2) goes again on a +fresh connection, one reset with `H3_REQUEST_REJECTED` (§4.1.1) on the same +one. The refusal still reaches the caller, as `{error, {goaway, no_error}}` +or `{error, {stream_reset, 16#10b}}`, in the same cases as for HTTP/2 (see +the [HTTP/2 guide](http2_guide.md#error-handling)): a second refusal, a body +function, a refusal after `request/5` has returned, or a request made with +`send_request/2`. + ```erlang case hackney:get(URL, [], <<>>, [{protocols, [http3]}]) of {ok, Status, Headers, Body} -> diff --git a/src/hackney.erl b/src/hackney.erl index 72488748..fbbac1af 100644 --- a/src/hackney.erl +++ b/src/hackney.erl @@ -79,6 +79,9 @@ -include("hackney_lib.hrl"). -include_lib("hackney_internal.hrl"). +%% HTTP/3 H3_REQUEST_REJECTED (RFC 9114 8.1): the server did not process the +%% request. +-define(H3_REQUEST_REJECTED, 16#10b). -type url() :: #hackney_url{} | binary(). -type conn() :: pid(). @@ -720,17 +723,66 @@ request_with_transport(Method, URL, Headers0, Body, Options0, _ -> <> end, - %% Check for proxy - case maybe_proxy(Transport, URL#hackney_url.scheme, Host, Port, Options) of + Dial = fun() -> maybe_proxy(Transport, URL#hackney_url.scheme, Host, Port, Options) end, + Send = fun(ConnPid, ReqPath, ReqHeaders) -> + do_request(ConnPid, Method, ReqPath, ReqHeaders, Body, Options, URL, Host) + end, + case dial_and_send(Dial, Send, FinalPath, Headers0) of + {{error, Reason} = Error, ConnPid} -> + %% A request the server turned away unprocessed can go again whatever + %% its method, as long as the body can be sent twice. Once only: a + %% second refusal is the caller's. + case refused(Reason) andalso replayable_body(Body) of + true -> + maybe_stop_unpooled(ConnPid, Options), + {Result, _} = dial_and_send(Dial, Send, FinalPath, Headers0), + Result; + false -> + Error + end; + {Result, _} -> + Result + end. + +%% @private The server guarantees it did not process the request: a GOAWAY +%% below its stream (RFC 9113 6.8, RFC 9114 5.2), RST_STREAM(REFUSED_STREAM) +%% (RFC 9113 8.7) or H3_REQUEST_REJECTED (RFC 9114 4.1.1). +refused({goaway, _}) -> true; +refused({stream_error, refused_stream}) -> true; +refused({stream_reset, ?H3_REQUEST_REJECTED}) -> true; +refused(_Reason) -> false. + +%% @private Dial (possibly through a proxy) and send, returning the connection +%% alongside the result so a refused request can be retried elsewhere. +dial_and_send(Dial, Send, FinalPath, Headers0) -> + case Dial() of {ok, ConnPid} -> - do_request(ConnPid, Method, FinalPath, Headers0, Body, Options, URL, Host); + {Send(ConnPid, FinalPath, Headers0), ConnPid}; {ok, ConnPid, {http_proxy, TargetScheme, TargetHost, TargetPort, ProxyAuth}} -> %% HTTP proxy mode - use absolute URLs AbsolutePath = build_absolute_url(TargetScheme, TargetHost, TargetPort, FinalPath), Headers1 = add_proxy_auth_header(Headers0, ProxyAuth), - do_request(ConnPid, Method, AbsolutePath, Headers1, Body, Options, URL, Host); + {Send(ConnPid, AbsolutePath, Headers1), ConnPid}; Error -> - Error + {Error, undefined} + end. + +%% @private A body produced by a fun may already have been consumed by the +%% first attempt. Everything else hackney accepts is re-sendable: binaries, +%% iolists, forms, multipart, files, and `stream', whose chunks only start +%% after the headers were accepted. +replayable_body(Body) when is_function(Body) -> false; +replayable_body({Fun, _State}) when is_function(Fun, 1) -> false; +replayable_body(_Body) -> true. + +%% @private A pooled connection stays the pool's: one a GOAWAY refused has +%% already left it and closes on its own, one that refused a single stream +%% still serves the others. An unpooled one belongs to this caller, who is +%% about to replace it. +maybe_stop_unpooled(ConnPid, Options) -> + case use_pool(Options) of + false -> stop_conn(ConnPid); + _ -> ok end. %% @doc Send a request on an existing connection. diff --git a/src/hackney_conn.erl b/src/hackney_conn.erl index 1684a0cc..46ba583f 100644 --- a/src/hackney_conn.erl +++ b/src/hackney_conn.erl @@ -4776,8 +4776,9 @@ h3_body_too_large(StreamId, From, Streams, #conn_data{h3_conn = ConnRef} = Data) %% @private Handle HTTP/3 stream reset handle_h3_stream_reset(StreamId, ErrorCode, Streams, Data) -> case maps:get(StreamId, Streams, undefined) of - {From, _State} when is_pid(From) -> - %% Sync stream - reply with error + {From, StreamState} when StreamState =:= waiting_headers; + element(1, StreamState) =:= waiting_headers_streaming -> + %% Sync request awaiting headers - reply via the state machine. UpdatedStreams = maps:remove(StreamId, Streams), {keep_state, Data#conn_data{h3_streams = UpdatedStreams, request_from = undefined}, [{reply, From, {error, {stream_reset, ErrorCode}}}]}; diff --git a/src/hackney_pool.erl b/src/hackney_pool.erl index 74011365..a7facc1a 100644 --- a/src/hackney_pool.erl +++ b/src/hackney_pool.erl @@ -680,7 +680,7 @@ handle_call({checkout_h2, Key}, _From, #state{h2_connections = H2Conns} = State) undefined -> {reply, none, State}; Pid -> - case h2_conn_usable(Pid) of + case shared_conn_usable(Pid) of true -> {reply, {ok, Pid}, State}; false -> @@ -690,17 +690,17 @@ handle_call({checkout_h2, Key}, _From, #state{h2_connections = H2Conns} = State) end; handle_call({checkout_h3, Key}, _From, #state{h3_connections = H3Conns} = State) -> - %% HTTP/3 checkout - return existing connection if available + %% HTTP/3 checkout - return existing connection if available. The same + %% check as HTTP/2: a connection draining after a GOAWAY leaves the pool + %% with a cast that can lose the race against this checkout. case maps:get(Key, H3Conns, undefined) of undefined -> {reply, none, State}; Pid -> - %% Verify connection is still alive - case erlang:is_process_alive(Pid) of + case shared_conn_usable(Pid) of true -> {reply, {ok, Pid}, State}; false -> - %% Connection died, remove from pool H3Conns2 = maps:remove(Key, H3Conns), {reply, none, State#state{h3_connections = H3Conns2}} end @@ -1360,9 +1360,10 @@ pool_has_idle_room(#state{available=Available, max_connections=MaxConn}) -> idle_count(Available) -> maps:fold(fun(_, Pids, Acc) -> Acc + length(Pids) end, 0, Available). -%% @private Check that a pooled HTTP/2 conn is alive and in `connected` state. -%% Short timeout so a stuck conn doesn't wedge the pool; any failure → unusable. -h2_conn_usable(Pid) -> +%% @private Check that a shared HTTP/2 or HTTP/3 conn is alive and in +%% `connected` state. Short timeout so a stuck conn doesn't wedge the pool; +%% any failure → unusable. +shared_conn_usable(Pid) -> case erlang:is_process_alive(Pid) of false -> false; true -> diff --git a/test/hackney_h2_goaway_server.erl b/test/hackney_h2_goaway_server.erl index e5c0de33..3028fbe0 100644 --- a/test/hackney_h2_goaway_server.erl +++ b/test/hackney_h2_goaway_server.erl @@ -6,12 +6,17 @@ %%% streamed body once that body is complete. Later connections answer every %%% request as soon as it is complete. Each response body is the stream id. %%% -%%% The GOAWAY is sent on a trigger: -%%% {second_stream, Pick} when the second stream opens, with +%%% The refusal is sent on a trigger: +%%% {second_stream, Pick} GOAWAY when the second stream opens, with %%% Pick(FirstStreamId, SecondStreamId) as last_stream_id -%%% first_data when a stream's first body chunk arrives, with that -%%% stream as last_stream_id +%%% first_data GOAWAY when a stream's first body chunk arrives, +%%% with that stream as last_stream_id +%%% first_stream GOAWAY when the first stream opens, refusing it +%%% refused_stream RST_STREAM(REFUSED_STREAM) on the first stream; the +%%% connection goes on answering the others %%% and shaped by options: +%%% trigger_on => first | all only the first connection refuses, or every +%%% connection does (default first) %%% goaway => direct | two_step two_step first sends GOAWAY(2^31-1), the %%% graceful shutdown of RFC 9113 6.8 (default direct) %%% answer => boolean() false answers nothing after the GOAWAY, not @@ -37,7 +42,7 @@ start(Trigger, Opts) -> {versions, ['tlsv1.2', 'tlsv1.3']}, {active, false}, {mode, binary}, {reuseaddr, true}]), {ok, {_, Port}} = ssl:sockname(LSock), - Options = maps:merge(#{goaway => direct, answer => true}, Opts), + Options = maps:merge(#{goaway => direct, answer => true, trigger_on => first}, Opts), Pid = spawn(fun() -> accept_loop(LSock, Trigger, Options) end), Url = iolist_to_binary([<<"https://localhost:">>, integer_to_list(Port), <<"/">>]), {Pid, Url}. @@ -45,11 +50,12 @@ start(Trigger, Opts) -> stop(Pid) -> exit(Pid, kill). -accept_loop(LSock, Trigger, Opts) -> +accept_loop(LSock, Trigger, #{trigger_on := TriggerOn} = Opts) -> case ssl:transport_accept(LSock, 2000) of {ok, TSock} -> spawn(fun() -> serve(TSock, Trigger, Opts) end), - accept_loop(LSock, none, maps:remove(notify, Opts)); + Next = case TriggerOn of all -> Trigger; first -> none end, + accept_loop(LSock, Next, maps:remove(notify, Opts)); {error, timeout} -> accept_loop(LSock, Trigger, Opts); {error, closed} -> ok end. @@ -131,19 +137,30 @@ on_request_frame(Sock, Kind, Sid, EndStream, #{seen := Seen, complete := Complet Seen2 = case lists:member(Sid, Seen) of true -> Seen; false -> Seen ++ [Sid] end, Complete2 = case EndStream of true -> [Sid | Complete]; false -> Complete end, St1 = St#{seen := Seen2, complete := Complete2}, - St2 = case goaway_for(Kind, Sid, EndStream, St1) of + St2 = case refusal_for(Kind, Sid, EndStream, St1) of none -> St1; - LastStreamId -> send_goaway(Sock, LastStreamId, St1) + {goaway, LastStreamId} -> send_goaway(Sock, LastStreamId, St1); + {rst_stream, Sid} -> send_rst_stream(Sock, Sid, St1) end, answer_ready(Sock, St2). -goaway_for(headers, _Sid, _EndStream, #{trigger := {second_stream, Pick}, seen := [First, Second]}) -> - Pick(First, Second); -goaway_for(data, Sid, false, #{trigger := first_data}) -> - Sid; -goaway_for(_Kind, _Sid, _EndStream, _St) -> +refusal_for(headers, _Sid, _EndStream, #{trigger := {second_stream, Pick}, seen := [First, Second]}) -> + {goaway, Pick(First, Second)}; +refusal_for(data, Sid, false, #{trigger := first_data}) -> + {goaway, Sid}; +refusal_for(headers, Sid, _EndStream, #{trigger := first_stream, seen := [Sid]}) -> + {goaway, max(Sid - 2, 0)}; +refusal_for(headers, Sid, _EndStream, #{trigger := refused_stream, seen := [Sid]}) -> + {rst_stream, Sid}; +refusal_for(_Kind, _Sid, _EndStream, _St) -> none. +%% RFC 9113 8.7: the stream was not processed; the connection stays up and +%% answers every other stream. +send_rst_stream(Sock, Sid, #{complete := Complete} = St) -> + send(Sock, h2_frame:rst_stream(Sid, refused_stream)), + St#{trigger := none, complete := Complete -- [Sid]}. + %% One GOAWAY per connection, then a short drain before anything is answered. send_goaway(Sock, LastStreamId, #{seen := Seen, goaway := Shape, answer := Answer} = St) -> case Shape of diff --git a/test/hackney_h3_test_server.erl b/test/hackney_h3_test_server.erl index 743c1d3b..7347a861 100644 --- a/test/hackney_h3_test_server.erl +++ b/test/hackney_h3_test_server.erl @@ -23,8 +23,9 @@ %%% GET|POST /hold holds the request: sends {held, Handler, Conn, %%% StreamId} to the process registered as %%% hackney_h3_test_hold, then answers 200 with the -%%% stream id as body on `release', or sends -%%% {held_reset, StreamId} there if the client +%%% stream id as body on `release', resets the +%%% stream with H3_REQUEST_REJECTED on `reject', or +%%% sends {held_reset, StreamId} there if the client %%% resets the stream %%% POST any path 200, echoes the request body %%% anything else 404 @@ -181,6 +182,8 @@ hold(Conn, StreamId) -> receive release -> respond(Conn, StreamId, 200, [], integer_to_binary(StreamId)); + reject -> + quic_h3:cancel(Conn, StreamId, 16#10b); {quic_h3, Conn, {stream_reset, StreamId, _Code}} -> hackney_h3_test_hold ! {held_reset, StreamId}; {quic_h3, Conn, _Other} -> diff --git a/test/hackney_http2_goaway_drain_tests.erl b/test/hackney_http2_goaway_drain_tests.erl index f805f062..29ea569d 100644 --- a/test/hackney_http2_goaway_drain_tests.erl +++ b/test/hackney_http2_goaway_drain_tests.erl @@ -1,10 +1,14 @@ -%%% GOAWAY must only fail the streams the peer did not accept. +%%% GOAWAY must only fail the streams the peer did not accept, and those go +%%% again on a fresh connection, as does a stream reset with REFUSED_STREAM. %%% %%% RFC 9113 6.8: streams up to and including the GOAWAY's last_stream_id may %%% still be processed, and the peer keeps the connection open to finish them. %%% hackney used to abort every in-flight stream with {error, {goaway, _}}, so a %%% request the server went on to complete came back as an error: for a payment -%%% API that is a charge that succeeded and was reported as failed. +%%% API that is a charge that succeeded and was reported as failed. A stream +%%% above last_stream_id, like one reset with REFUSED_STREAM (8.7), was never +%%% processed, so request/5 sends it once more instead of handing the caller +%%% the refusal. %%% %%% The server (hackney_h2_goaway_server) holds the first two streams of its %%% first connection, sends GOAWAY with a chosen last_stream_id, then answers @@ -18,7 +22,11 @@ goaway_drain_test_() -> {foreach, fun setup/0, fun cleanup/1, [{timeout, 30, fun accepted_streams_complete/0}, - {timeout, 30, fun unaccepted_stream_fails_fast/0}, + {timeout, 30, fun refused_stream_is_sent_again/0}, + {timeout, 30, fun refused_unpooled_request_is_sent_again/0}, + {timeout, 30, fun refusal_from_the_fresh_connection_is_returned/0}, + {timeout, 30, fun rst_stream_refused_request_is_sent_again/0}, + {timeout, 30, fun rst_stream_refusal_from_the_fresh_connection_is_returned/0}, {timeout, 30, fun two_step_shutdown/0}, {timeout, 30, fun stalled_drain_ends_with_the_stream/0}]}. @@ -53,31 +61,73 @@ accepted_streams_complete() -> hackney_h2_goaway_server:stop(Server) end. -%% GOAWAY(last_stream_id = 1) after streams 1 and 3: stream 3 was not accepted -%% and fails at once with the goaway reason, stream 1 still completes. Only -%% the refused stream is reset. -unaccepted_stream_fails_fast() -> +%% GOAWAY(last_stream_id = 1) after streams 1 and 3: stream 3 was not accepted, +%% so it is reset and sent again on a fresh connection, where it is that +%% connection's stream 1. Stream 1 on the first connection still completes. +refused_stream_is_sent_again() -> {Server, Url} = hackney_h2_goaway_server:start({second_stream, fun(First, _Second) -> First end}, #{notify => self()}), try [R1, R2] = concurrent_requests(Url, 2), ?assertEqual({ok, 200, <<"1">>}, R1), - ?assertEqual({error, {goaway, no_error}}, R2), + ?assertEqual({ok, 200, <<"1">>}, R2), ?assertEqual([3], hackney_h2_goaway_server:rst_streams()) after hackney_h2_goaway_server:stop(Server) end. +%% Without a pool the refused connection is the caller's own; it is stopped and +%% the request goes again on a new one. +refused_unpooled_request_is_sent_again() -> + {Server, Url} = hackney_h2_goaway_server:start(first_stream), + try + ?assertEqual({ok, 200, <<"1">>}, fetch(Url, [{pool, false}])) + after + hackney_h2_goaway_server:stop(Server) + end. + +%% The request is sent again once. When the fresh connection refuses it too, +%% the caller gets that refusal. +refusal_from_the_fresh_connection_is_returned() -> + {Server, Url} = hackney_h2_goaway_server:start(first_stream, #{trigger_on => all}), + try + ?assertEqual({error, {goaway, no_error}}, fetch(Url, [])) + after + hackney_h2_goaway_server:stop(Server) + end. + +%% RST_STREAM(REFUSED_STREAM) turns away one stream and leaves the connection +%% serving (RFC 9113 8.7), so the request goes again on the same pooled +%% connection, as its next stream. +rst_stream_refused_request_is_sent_again() -> + {Server, Url} = hackney_h2_goaway_server:start(refused_stream), + try + ?assertEqual({ok, 200, <<"3">>}, fetch(Url, [])) + after + hackney_h2_goaway_server:stop(Server) + end. + +%% Without a pool the refused connection is replaced. When the fresh one +%% refuses the stream too, the caller gets that refusal. +rst_stream_refusal_from_the_fresh_connection_is_returned() -> + {Server, Url} = hackney_h2_goaway_server:start(refused_stream, #{trigger_on => all}), + try + ?assertEqual({error, {stream_error, refused_stream}}, fetch(Url, [{pool, false}])) + after + hackney_h2_goaway_server:stop(Server) + end. + %% The graceful shutdown of RFC 9113 6.8: GOAWAY(2^31-1) stops new streams %% without refusing any, then a second GOAWAY gives the real last_stream_id. -%% The first frame alone must not fail anything. +%% The first frame alone must not fail anything, and the stream the second +%% one refuses goes again on a fresh connection. two_step_shutdown() -> {Server, Url} = hackney_h2_goaway_server:start({second_stream, fun(First, _Second) -> First end}, #{goaway => two_step, notify => self()}), try [R1, R2] = concurrent_requests(Url, 2), ?assertEqual({ok, 200, <<"1">>}, R1), - ?assertEqual({error, {goaway, no_error}}, R2), + ?assertEqual({ok, 200, <<"1">>}, R2), ?assertEqual([3], hackney_h2_goaway_server:rst_streams()) after hackney_h2_goaway_server:stop(Server) @@ -85,15 +135,16 @@ two_step_shutdown() -> %% A server that accepts a stream and then never answers it must not keep the %% draining connection around: the stream's own recv_timeout ends it, and with -%% it the connection, so the next request gets a fresh one. +%% it the connection. The refused request went again on a fresh connection, +%% which the request after then shares. stalled_drain_ends_with_the_stream() -> {Server, Url} = hackney_h2_goaway_server:start({second_stream, fun(First, _Second) -> First end}, #{answer => false}), try [R1, R2] = concurrent_requests(Url, 2, [{recv_timeout, 1000}]), ?assertEqual({error, timeout}, R1), - ?assertEqual({error, {goaway, no_error}}, R2), - ?assertEqual({ok, 200, <<"1">>}, fetch(Url, [])) + ?assertEqual({ok, 200, <<"1">>}, R2), + ?assertMatch({ok, 200, _}, fetch(Url, [])) after hackney_h2_goaway_server:stop(Server) end. diff --git a/test/hackney_http2_goaway_streaming_tests.erl b/test/hackney_http2_goaway_streaming_tests.erl index 457e84f7..bb47f4fa 100644 --- a/test/hackney_http2_goaway_streaming_tests.erl +++ b/test/hackney_http2_goaway_streaming_tests.erl @@ -96,7 +96,8 @@ accepted_streamed_response_completes() -> Self ! {self(), R} end), timer:sleep(300), - ?assertEqual({error, {goaway, no_error}}, fetch(Url)), + %% The plain request is refused and goes again on a fresh connection. + ?assertEqual({ok, 200, <<"1">>}, fetch(Url)), ?assertEqual({ok, <<"1">>}, await(P1)) after hackney_h2_goaway_server:stop(Server) diff --git a/test/hackney_http3_goaway_tests.erl b/test/hackney_http3_goaway_tests.erl index afa22e08..c7d41e7d 100644 --- a/test/hackney_http3_goaway_tests.erl +++ b/test/hackney_http3_goaway_tests.erl @@ -5,6 +5,10 @@ %%% pool, refused streams fail with {error, {goaway, no_error}} and are reset, %%% accepted ones complete, and the connection closes once they have. %%% +%%% A refused request made with request/5 goes again on a fresh connection, +%%% and one the server rejects with H3_REQUEST_REJECTED (RFC 9114 4.1.1), the +%%% other way to turn a request away unprocessed, goes again on the same one. +%%% %%% The quic_h3 server always sends GOAWAY past the last stream it has seen, so %%% it never refuses an open stream. The tests that need a refusal deliver the %%% GOAWAY event to the connection as hackney_h3 would forward it. @@ -18,7 +22,10 @@ h3_goaway_test_() -> {foreach, fun setup/0, fun cleanup/1, [fun(Ctx) -> {timeout, 60, fun() -> held(fun accepted_stream_completes/1, Ctx) end} end, fun(Ctx) -> {timeout, 60, fun() -> held(fun refused_stream_fails_and_is_reset/1, Ctx) end} end, - fun(Ctx) -> {timeout, 60, fun() -> held(fun refused_upload_fails_on_next_call/1, Ctx) end} end]}. + fun(Ctx) -> {timeout, 60, fun() -> held(fun refused_upload_fails_on_next_call/1, Ctx) end} end, + fun(Ctx) -> {timeout, 60, fun() -> held(fun refused_request_is_sent_again/1, Ctx) end} end, + fun(Ctx) -> {timeout, 60, fun() -> held(fun refusal_from_the_fresh_connection_is_returned/1, Ctx) end} end, + fun(Ctx) -> {timeout, 60, fun() -> held(fun rejected_request_is_sent_again/1, Ctx) end} end]}. %% The test process gets the server's held-request notifications. held(Test, Ctx) -> @@ -83,6 +90,41 @@ refused_upload_fails_on_next_call(Ctx) -> ?assertEqual({ok, 200, <<"0">>}, await(P1)), ok = await_state(Conn, closed). +%% A request made with request/5 is held as stream 0, then GOAWAY(0) refuses +%% it: it is reset and goes again on a fresh connection, where the server +%% holds it a second time. +refused_request_is_sent_again(Ctx) -> + P = spawn_request(Ctx), + {_Handler0, ServerConn0, 0} = await_held(), + ok = goaway(pooled_conn(), 0), + ?assertEqual(ok, receive {held_reset, 0} -> ok after 10000 -> no_reset end), + {Handler, ServerConn1, 0} = await_held(), + ?assertNotEqual(ServerConn0, ServerConn1), + Handler ! release, + ?assertEqual({ok, 200, <<"0">>}, await(P)). + +%% The request is sent again once. When the fresh connection refuses it too, +%% the caller gets that refusal. +refusal_from_the_fresh_connection_is_returned(Ctx) -> + P = spawn_request(Ctx), + {_Handler0, _, 0} = await_held(), + ok = goaway(pooled_conn(), 0), + ?assertEqual(ok, receive {held_reset, 0} -> ok after 10000 -> no_reset end), + {_Handler1, _, 0} = await_held(), + ok = goaway(pooled_conn(), 0), + ?assertEqual(ok, receive {held_reset, 0} -> ok after 10000 -> no_reset end), + ?assertEqual({error, {goaway, no_error}}, await(P)). + +%% The server rejects the held request with H3_REQUEST_REJECTED: the +%% connection stays up, so the request goes again on it as the next stream. +rejected_request_is_sent_again(Ctx) -> + P = spawn_request(Ctx), + {Handler0, ServerConn, 0} = await_held(), + Handler0 ! reject, + {Handler, ServerConn, 4} = await_held(), + Handler ! release, + ?assertEqual({ok, 200, <<"4">>}, await(P)). + %%==================================================================== %% Helpers %%==================================================================== @@ -90,6 +132,26 @@ refused_upload_fails_on_next_call(Ctx) -> opts() -> [{pool, ?POOL} | hackney_h3_test_server:hackney_opts()]. +%% A /hold request through the pool, made with request/5. +spawn_request(Ctx) -> + Self = self(), + Url = hackney_h3_test_server:url(Ctx, <<"/hold">>), + spawn_link(fun() -> + R = case hackney:request(get, Url, [], <<>>, opts()) of + {ok, Status, _Headers, Body} -> {ok, Status, Body}; + {error, E} -> {error, E} + end, + Self ! {self(), R} + end). + +%% The connection the pool hands out: the one still connected, as a drained +%% one closes once its streams are gone. +pooled_conn() -> + [Conn] = [Owner || {_Ref, Pid} <- ets:tab2list(hackney_h3_conns), + Owner <- [h3_owner(Pid)], is_pid(Owner), + (catch hackney_conn:get_state(Owner, 1000)) =:= {ok, connected}], + Conn. + connect(Ctx) -> {ok, Conn} = hackney:connect(hackney_ssl, "127.0.0.1", hackney_h3_test_server:port(Ctx), opts()), @@ -125,10 +187,9 @@ goaway(Conn, GoawayId) -> %% The hackney_h3 process monitors its owner, the connection. h3_owner(Pid) -> - {monitors, Monitors} = erlang:process_info(Pid, monitors), - case [P || {process, P} <- Monitors] of - [Owner | _] -> Owner; - [] -> undefined + case erlang:process_info(Pid, monitors) of + {monitors, [{process, Owner} | _]} -> Owner; + _ -> undefined end. %% Poll the connection state: GOAWAY handling and the drain end are driven by diff --git a/test/hackney_pool_h3_tests.erl b/test/hackney_pool_h3_tests.erl index ac2d47e9..b2ebc17b 100644 --- a/test/hackney_pool_h3_tests.erl +++ b/test/hackney_pool_h3_tests.erl @@ -53,7 +53,7 @@ test_h3_checkout_none() -> test_h3_register_checkout() -> %% Start a dummy process to act as a connection - DummyConn = spawn(fun() -> receive stop -> ok end end), + DummyConn = dummy_conn(), %% Register it as an H3 connection ok = hackney_pool:register_h3("h3test.example.com", 443, hackney_ssl, DummyConn, []), @@ -70,7 +70,7 @@ test_h3_register_checkout() -> test_h3_unregister() -> %% Start a dummy process - DummyConn = spawn(fun() -> receive stop -> ok end end), + DummyConn = dummy_conn(), %% Register it ok = hackney_pool:register_h3("h3unreg.example.com", 443, hackney_ssl, DummyConn, []), @@ -91,7 +91,7 @@ test_h3_unregister() -> test_h3_connection_death() -> %% Start a dummy process - DummyConn = spawn(fun() -> receive stop -> ok end end), + DummyConn = dummy_conn(), %% Register it ok = hackney_pool:register_h3("h3death.example.com", 443, hackney_ssl, DummyConn, []), @@ -110,7 +110,7 @@ test_h3_connection_death() -> test_h3_tls_key_isolation() -> %% A connection registered under one trust key must not be handed out %% for a request carrying a different trust key. - DummyConn = spawn(fun() -> receive stop -> ok end end), + DummyConn = dummy_conn(), K1 = hackney_ssl:h3_options_key([{insecure, true}], []), K2 = hackney_ssl:h3_options_key([], []), ok = hackney_pool:register_h3("h3key.example.com", 443, hackney_ssl, DummyConn, @@ -129,7 +129,7 @@ test_h3_tls_key_isolation() -> test_h3_tls_key_default_bucket() -> %% Callers without an h3_tls_key land in the default bucket, isolated %% from explicitly keyed connections. - DummyConn = spawn(fun() -> receive stop -> ok end end), + DummyConn = dummy_conn(), K1 = hackney_ssl:h3_options_key([{insecure, true}], []), ok = hackney_pool:register_h3("h3default.example.com", 443, hackney_ssl, DummyConn, []), timer:sleep(50), @@ -161,7 +161,7 @@ h3_multiplexing_test_() -> test_h3_multiplexing() -> %% Start a dummy connection process - DummyConn = spawn(fun() -> receive stop -> ok end end), + DummyConn = dummy_conn(), %% Register it ok = hackney_pool:register_h3("h3mux.example.com", 443, hackney_ssl, DummyConn, []), @@ -184,8 +184,8 @@ test_h3_multiplexing() -> test_h3_different_hosts() -> %% Start two dummy connections - Conn1 = spawn(fun() -> receive stop -> ok end end), - Conn2 = spawn(fun() -> receive stop -> ok end end), + Conn1 = dummy_conn(), + Conn2 = dummy_conn(), %% Register them for different hosts ok = hackney_pool:register_h3("host1.example.com", 443, hackney_ssl, Conn1, []), @@ -204,3 +204,16 @@ test_h3_different_hosts() -> %% Cleanup Conn1 ! stop, Conn2 ! stop. + +%% A stand-in connection: answers the pool's get_state probe as a connected +%% hackney_conn would, until told to stop. +dummy_conn() -> + spawn(fun Loop() -> + receive + {'$gen_call', From, get_state} -> + gen:reply(From, {ok, connected}), + Loop(); + stop -> + ok + end + end).