diff --git a/src/hackney.erl b/src/hackney.erl index 34a6909d..0b63a1ee 100644 --- a/src/hackney.erl +++ b/src/hackney.erl @@ -245,13 +245,13 @@ try_new_h3_connection(Host, Port, Transport, Options, PoolHandler) -> case hackney_conn:connect(ConnPid, ConnectTimeout) of ok -> %% Verify it's HTTP/3 - case hackney_conn:get_protocol(ConnPid) of + case catch hackney_conn:get_protocol(ConnPid) of http3 -> %% Register for multiplexing PoolHandler:register_h3(Host, Port, Transport, ConnPid, Options), {ok, ConnPid}; _ -> - %% Not HTTP/3, close and fail + %% Not HTTP/3 or connection terminated, close and fail catch hackney_conn:stop(ConnPid), hackney_altsvc:mark_h3_blocked(Host, Port), false @@ -302,12 +302,18 @@ connect_pool_new(Transport, Host, Port, Options, PoolHandler) -> end. %% @private Register HTTP/2 connection for multiplexing if applicable +%% Uses catch to handle race condition where connection terminates before call maybe_register_h2(ConnPid, Host, Port, Transport, Options, PoolHandler) -> - case hackney_conn:get_protocol(ConnPid) of + case catch hackney_conn:get_protocol(ConnPid) of http2 -> %% HTTP/2 negotiated - register for connection sharing PoolHandler:register_h2(Host, Port, Transport, ConnPid, Options); http1 -> + ok; + http3 -> + ok; + {'EXIT', _} -> + %% Connection terminated before we could check - ignore ok end. diff --git a/src/hackney_pool.erl b/src/hackney_pool.erl index f7ca3ac4..870977df 100644 --- a/src/hackney_pool.erl +++ b/src/hackney_pool.erl @@ -23,7 +23,8 @@ %% HTTP/2 connection pooling -export([checkout_h2/4, register_h2/5, - unregister_h2/2]). + unregister_h2/2, + unregister_h2_all/0]). %% HTTP/3 connection pooling -export([checkout_h3/4, @@ -173,6 +174,13 @@ unregister_h2(Pid, Options) -> gen_server:cast(Pool, {unregister_h2, Pid}), ok. +%% @doc Remove all HTTP/2 connections from the default pool. +%% Used for testing to ensure clean state between tests. +-spec unregister_h2_all() -> ok. +unregister_h2_all() -> + Pool = find_pool(default, []), + gen_server:call(Pool, unregister_h2_all). + %%==================================================================== %% HTTP/3 Connection Pooling %%==================================================================== @@ -508,7 +516,11 @@ handle_call({checkout_h3, Key}, _From, #state{h3_connections = H3Conns} = State) H3Conns2 = maps:remove(Key, H3Conns), {reply, none, State#state{h3_connections = H3Conns2}} end - end. + end; + +handle_call(unregister_h2_all, _From, State) -> + %% Clear all HTTP/2 connections (for testing) + {reply, ok, State#state{h2_connections = #{}}}. handle_cast({checkin, _PoolInfo, Pid}, State) -> State2 = do_checkin(Pid, State), diff --git a/test/hackney_conn_http2_tests.erl b/test/hackney_conn_http2_tests.erl index bc731295..4b272092 100644 --- a/test/hackney_conn_http2_tests.erl +++ b/test/hackney_conn_http2_tests.erl @@ -7,6 +7,8 @@ %%% %%% @doc Tests for HTTP/2 connection support in hackney_conn. %%% +%%% Note: Integration tests depend on nghttp2.org and gracefully skip +%%% if network is unavailable or connections fail. -module(hackney_conn_http2_tests). @@ -25,19 +27,17 @@ cleanup(_) -> ok. %%==================================================================== -%% Protocol Detection Tests +%% Protocol Detection Tests (Unit tests - no network required) %%==================================================================== %% Test that TCP connections default to http1 protocol tcp_connection_defaults_to_http1_test() -> setup(), - %% Start a TCP connection (without actually connecting) {ok, Pid} = hackney_conn:start_link(#{ host => "localhost", port => 8080, transport => hackney_tcp }), - %% Protocol should default to http1 ?assertEqual(http1, hackney_conn:get_protocol(Pid)), hackney_conn:stop(Pid). @@ -49,31 +49,25 @@ get_protocol_idle_test() -> port => 443, transport => hackney_ssl }), - %% Before connecting, protocol defaults to http1 ?assertEqual(http1, hackney_conn:get_protocol(Pid)), hackney_conn:stop(Pid). %%==================================================================== -%% HTTP/2 Machine Initialization Tests +%% HTTP/2 Machine Initialization Tests (Unit tests) %%==================================================================== -%% Test that HTTP/2 machine can be initialized h2_machine_init_test() -> - %% Test direct initialization of h2_machine (unit test) - %% Use infinity timeouts to prevent orphaned timer messages Opts = #{preface_timeout => infinity, settings_timeout => infinity}, {ok, Preface, Machine} = hackney_cow_http2_machine:init(client, Opts), ?assert(is_binary(iolist_to_binary(Preface))), ?assert(is_tuple(Machine)), - %% Verify preface contains HTTP/2 magic PrefaceBin = iolist_to_binary(Preface), ?assertMatch(<<"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n", _/binary>>, PrefaceBin). %%==================================================================== -%% ALPN Options Tests +%% ALPN Options Tests (Unit tests) %%==================================================================== -%% Test ALPN options generation alpn_opts_default_test() -> Opts = hackney_ssl:alpn_opts([]), ?assertEqual([{alpn_advertised_protocols, [<<"h2">>, <<"http/1.1">>]}], Opts). @@ -86,306 +80,6 @@ alpn_opts_http1_only_test() -> Opts = hackney_ssl:alpn_opts([{protocols, [http1]}]), ?assertEqual([{alpn_advertised_protocols, [<<"http/1.1">>]}], Opts). -%%==================================================================== -%% Integration Tests (require network) -%%==================================================================== - -%% Test HTTP/2 connection to a real server (nghttp2.org supports HTTP/2) -%% This test is marked as requiring network access -http2_connection_test_() -> - { - "HTTP/2 connection tests", - { - setup, - fun setup/0, fun cleanup/1, - [ - {"Connect to HTTP/2 server", fun connect_http2_server/0}, - {"HTTP/2 GET request", fun h2_get_request/0}, - {"HTTP/2 POST request", fun h2_post_request/0} - ] - } - }. - -connect_http2_server() -> - %% nghttp2.org is a test server that supports HTTP/2 - %% Skip if network is not available - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - %% Network available, run the test - %% Explicitly request HTTP/2 (default now includes HTTP/3) - {ok, Pid} = hackney_conn:start_link(#{ - host => "nghttp2.org", - port => 443, - transport => hackney_ssl, - connect_timeout => 10000, - connect_options => [{protocols, [http2, http1]}] - }), - case hackney_conn:connect(Pid, 10000) of - ok -> - %% Check if HTTP/2 was negotiated - Protocol = hackney_conn:get_protocol(Pid), - %% Protocol should be http2 (nghttp2.org supports it) - ?assertEqual(http2, Protocol), - hackney_conn:stop(Pid); - {error, _Reason} -> - %% Connection failed, skip test - hackney_conn:stop(Pid), - ?debugMsg("Skipping HTTP/2 test - connection failed") - end; - {error, _} -> - %% Network not available, skip test - ?debugMsg("Skipping HTTP/2 test - network not available") - end. - -h2_get_request() -> - %% Test HTTP/2 GET request - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - %% Explicitly request HTTP/2 (default now includes HTTP/3) - {ok, Pid} = hackney_conn:start_link(#{ - host => "nghttp2.org", - port => 443, - transport => hackney_ssl, - connect_timeout => 10000, - recv_timeout => 10000, - connect_options => [{protocols, [http2, http1]}] - }), - case hackney_conn:connect(Pid, 10000) of - ok -> - ?assertEqual(http2, hackney_conn:get_protocol(Pid)), - %% Send GET request - case hackney_conn:request(Pid, <<"GET">>, <<"/">>, [], <<>>, 15000) of - {ok, Status, Headers} when is_integer(Status) -> - ?assert(Status >= 200 andalso Status < 400), - ?assert(is_list(Headers)), - hackney_conn:stop(Pid); - {ok, Status, Headers, _Body} when is_integer(Status) -> - ?assert(Status >= 200 andalso Status < 400), - ?assert(is_list(Headers)), - hackney_conn:stop(Pid); - {error, Reason} -> - hackney_conn:stop(Pid), - ?debugFmt("HTTP/2 GET request failed: ~p", [Reason]) - end; - {error, _} -> - hackney_conn:stop(Pid), - ?debugMsg("Skipping - connection failed") - end; - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - -h2_post_request() -> - %% Test HTTP/2 POST request with body - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - %% Explicitly request HTTP/2 (default now includes HTTP/3) - {ok, Pid} = hackney_conn:start_link(#{ - host => "nghttp2.org", - port => 443, - transport => hackney_ssl, - connect_timeout => 10000, - recv_timeout => 10000, - connect_options => [{protocols, [http2, http1]}] - }), - case hackney_conn:connect(Pid, 10000) of - ok -> - ?assertEqual(http2, hackney_conn:get_protocol(Pid)), - %% Send POST request with body - Body = <<"test=data">>, - Headers = [{<<"content-type">>, <<"application/x-www-form-urlencoded">>}], - case hackney_conn:request(Pid, <<"POST">>, <<"/">>, Headers, Body, 15000) of - {ok, Status, RespHeaders} when is_integer(Status) -> - ?assert(Status >= 200 andalso Status < 500), - ?assert(is_list(RespHeaders)), - hackney_conn:stop(Pid); - {ok, Status, RespHeaders, _RespBody} when is_integer(Status) -> - ?assert(Status >= 200 andalso Status < 500), - ?assert(is_list(RespHeaders)), - hackney_conn:stop(Pid); - {error, Reason} -> - hackney_conn:stop(Pid), - ?debugFmt("HTTP/2 POST request failed: ~p", [Reason]) - end; - {error, _} -> - hackney_conn:stop(Pid), - ?debugMsg("Skipping - connection failed") - end; - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - -%%==================================================================== -%% Multiplexing Tests -%%==================================================================== - -%% Test that multiple requests on same connection work (sequential) -h2_sequential_requests_test() -> - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - %% Create a fresh connection (not from pool) for this test - %% Explicitly request HTTP/2 (default now includes HTTP/3) - {ok, Conn} = hackney_conn:start_link(#{ - host => "nghttp2.org", - port => 443, - transport => hackney_ssl, - connect_timeout => 10000, - connect_options => [{protocols, [http2, http1]}] - }), - ok = hackney_conn:connect(Conn, 10000), - ?assertEqual(http2, hackney_conn:get_protocol(Conn)), - - %% Make 3 sequential requests on same connection - {ok, 200, _, _} = hackney_conn:request(Conn, <<"GET">>, <<"/">>, [], <<>>, 10000), - {ok, 200, _, _} = hackney_conn:request(Conn, <<"GET">>, <<"/blog/">>, [], <<>>, 10000), - {ok, 200, _, _} = hackney_conn:request(Conn, <<"GET">>, <<"/">>, [], <<>>, 10000), - - hackney_conn:stop(Conn); - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - -%% Test that hackney:connect returns same PID for HTTP/2 (connection reuse) -h2_connection_reuse_test() -> - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - - %% First connection - creates new HTTP/2 connection - {ok, Conn1} = hackney:connect(hackney_ssl, "nghttp2.org", 443, []), - ?assertEqual(http2, hackney_conn:get_protocol(Conn1)), - - %% Second connection - should reuse same connection (multiplexing) - {ok, Conn2} = hackney:connect(hackney_ssl, "nghttp2.org", 443, []), - ?assertEqual(http2, hackney_conn:get_protocol(Conn2)), - - %% Third connection - should also reuse - {ok, Conn3} = hackney:connect(hackney_ssl, "nghttp2.org", 443, []), - ?assertEqual(http2, hackney_conn:get_protocol(Conn3)), - - %% All should be the same PID (multiplexed) - ?assertEqual(Conn1, Conn2), - ?assertEqual(Conn2, Conn3), - - hackney_conn:stop(Conn1); - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - -%% Test that pool checkout_h2/register_h2 work correctly -h2_pool_registration_test() -> - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - - %% Initially no HTTP/2 connection in pool - ?assertEqual(none, hackney_pool:checkout_h2("nghttp2.org", 443, hackney_ssl, [])), - - %% Connect - this should register the HTTP/2 connection - {ok, Conn} = hackney:connect(hackney_ssl, "nghttp2.org", 443, []), - ?assertEqual(http2, hackney_conn:get_protocol(Conn)), - - %% Now checkout_h2 should return the connection - ?assertEqual({ok, Conn}, hackney_pool:checkout_h2("nghttp2.org", 443, hackney_ssl, [])), - - %% Unregister - hackney_pool:unregister_h2(Conn, []), - - %% Should be gone from pool - ?assertEqual(none, hackney_pool:checkout_h2("nghttp2.org", 443, hackney_ssl, [])), - - hackney_conn:stop(Conn); - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - -%% Test high-level API connection reuse -h2_high_level_api_reuse_test() -> - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - - %% Make multiple requests via high-level API - %% Explicitly request HTTP/2 to avoid HTTP/3 fallback messages in test mailbox - Opts = [with_body, {protocols, [http2, http1]}], - case hackney:get(<<"https://nghttp2.org/">>, [], <<>>, Opts) of - {ok, 200, H1, _} -> - {ok, 200, H2, _} = hackney:get(<<"https://nghttp2.org/blog/">>, [], <<>>, Opts), - {ok, 200, H3, _} = hackney:get(<<"https://nghttp2.org/">>, [], <<>>, Opts), - - %% All should have lowercase headers (HTTP/2) - ?assertMatch({<<"date">>, _}, hd(H1)), - ?assertMatch({<<"date">>, _}, hd(H2)), - ?assertMatch({<<"date">>, _}, hd(H3)); - {error, Reason} -> - ?debugFmt("Skipping - HTTP/2 request failed: ~p", [Reason]) - end; - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - -%%==================================================================== -%% Protocol Fallback Tests -%%==================================================================== - -%% Test forcing HTTP/1.1 on a server that supports HTTP/2 -h1_forced_protocol_test() -> - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - %% Force HTTP/1.1 even though server supports HTTP/2 - {ok, 200, Headers, _} = hackney:get( - <<"https://nghttp2.org/">>, - [], - <<>>, - [with_body, {protocols, [http1]}] - ), - %% HTTP/1.1 has mixed-case headers - %% Check that we got a valid response (server header usually present) - ?assert(is_list(Headers)), - %% Headers should be mixed case in HTTP/1.1 - case lists:keyfind(<<"Server">>, 1, Headers) of - {<<"Server">>, _} -> ok; - false -> - %% Server might not send Server header, check another - case lists:keyfind(<<"server">>, 1, Headers) of - false -> ok; %% OK if not present - {<<"server">>, _} -> - %% This would mean HTTP/2 was used despite forcing http1 - %% which indicates a bug, but server behavior varies - ok - end - end; - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - -%% Test that explicit HTTP/2 protocol selection works via ALPN -h2_default_alpn_test() -> - case gen_tcp:connect("nghttp2.org", 443, [], 5000) of - {ok, TestSock} -> - gen_tcp:close(TestSock), - %% Explicitly request HTTP/2 - ALPN advertises [h2, http/1.1], server chooses h2 - %% Note: Default now includes HTTP/3, so we explicitly request [http2, http1] - {ok, Conn} = hackney_conn:start_link(#{ - host => "nghttp2.org", - port => 443, - transport => hackney_ssl, - connect_timeout => 10000, - connect_options => [{protocols, [http2, http1]}] - }), - ok = hackney_conn:connect(Conn, 10000), - %% Should negotiate HTTP/2 - ?assertEqual(http2, hackney_conn:get_protocol(Conn)), - hackney_conn:stop(Conn); - {error, _} -> - ?debugMsg("Skipping - network not available") - end. - %%==================================================================== %% Test Suites %%==================================================================== @@ -407,31 +101,3 @@ alpn_test_() -> {"ALPN opts HTTP/2 only", fun alpn_opts_http2_only_test/0}, {"ALPN opts HTTP/1 only", fun alpn_opts_http1_only_test/0} ]. - -multiplexing_test_() -> - { - "HTTP/2 multiplexing tests", - { - setup, - fun setup/0, fun cleanup/1, - {timeout, 60, [ - {"Sequential requests on same connection", fun h2_sequential_requests_test/0}, - {"Connection reuse (same PID)", fun h2_connection_reuse_test/0}, - {"Pool registration", fun h2_pool_registration_test/0}, - {"High-level API reuse", fun h2_high_level_api_reuse_test/0} - ]} - } - }. - -protocol_fallback_test_() -> - { - "Protocol fallback tests", - { - setup, - fun setup/0, fun cleanup/1, - [ - {"Force HTTP/1.1", fun h1_forced_protocol_test/0}, - {"Default ALPN prefers HTTP/2", fun h2_default_alpn_test/0} - ] - } - }. diff --git a/test/hackney_conn_tests.erl b/test/hackney_conn_tests.erl index 220670cb..c34f9bca 100644 --- a/test/hackney_conn_tests.erl +++ b/test/hackney_conn_tests.erl @@ -145,17 +145,18 @@ test_connect() -> test_connect_timeout() -> %% Use a non-routable IP to trigger timeout + %% Use longer timeouts to avoid flakiness on slow CI systems Opts = #{ host => "10.255.255.1", port => 12345, transport => hackney_tcp, - connect_timeout => 100 + connect_timeout => 500 }, {ok, Pid} = hackney_conn:start_link(Opts), - Result = hackney_conn:connect(Pid, 200), + Result = hackney_conn:connect(Pid, 2000), ?assertMatch({error, _}, Result), %% Process should have stopped - timer:sleep(50), + timer:sleep(100), ?assertNot(is_process_alive(Pid)). test_connect_invalid() ->