diff --git a/docs/src/guides/client.md b/docs/src/guides/client.md index 9d72fcdc9..bcf4b68f1 100644 --- a/docs/src/guides/client.md +++ b/docs/src/guides/client.md @@ -201,8 +201,9 @@ Recognized client defaults: - `default_query`: dict, named-tuple, or vector-of-pairs of query parameters; per-call keys override matching defaults - `default_basicauth`: applied unless the call passes `basicauth` or an explicit `Authorization` header - `connect_timeout`, `request_timeout`, `response_header_timeout`, - `read_idle_timeout`, `write_idle_timeout`: applied when the per-call timeout - is `0` (the default) + `read_idle_timeout`, `write_idle_timeout`: applied when the call does not pass the + matching keyword. When neither the call nor the client sets it the timeouts are disabled + (i.e. set to `0`) except `connect_timeout` which defaults to `30` ### Positional `Client` calls @@ -454,6 +455,9 @@ read timeout: - `expect_continue_timeout` controls how long HTTP/1 uploads wait on `100-continue` before sending the body anyway +`connect_timeout` defaults to 30 seconds when neither the call nor a `Client` +sets it; the other timeouts are disabled by default. + `readtimeout` is still accepted for compatibility, but it is deprecated and now behaves like `read_idle_timeout`. diff --git a/src/http_client.jl b/src/http_client.jl index a450ce54d..b722dac8b 100644 --- a/src/http_client.jl +++ b/src/http_client.jl @@ -1822,6 +1822,17 @@ For timeout kwargs left at `0`, fall back to the client's default for the named return getfield(client, field) end +const _DEFAULT_CONNECT_TIMEOUT = 30 + +@inline function _resolve_connect_timeout(client::Union{Nothing,Client}, value::Union{Nothing,Real})::Real + if value === nothing + client === nothing && return _DEFAULT_CONNECT_TIMEOUT + default = client.default_connect_timeout + return default > 0 ? default : _DEFAULT_CONNECT_TIMEOUT + end + return _client_default_timeout(client, value, :default_connect_timeout) +end + function request( method::Union{AbstractString,Symbol}, @@ -1854,7 +1865,7 @@ function request( max_sse_event_bytes::Integer=_DEFAULT_SSE_CLIENT_MAX_EVENT_BYTES, client::Union{Nothing,Client}=nothing, context::Union{Nothing,RequestContext}=nothing, - connect_timeout::Real=30, + connect_timeout::Union{Nothing,Real}=nothing, request_timeout::Real=0, response_header_timeout::Real=0, read_idle_timeout::Real=0, @@ -1907,7 +1918,7 @@ function request( max_sse_line_bytes >= 0 || throw(ArgumentError("max_sse_line_bytes must be >= 0")) max_sse_event_bytes >= 0 || throw(ArgumentError("max_sse_event_bytes must be >= 0")) # Merge per-call values with client defaults (per-call wins) - connect_timeout = _client_default_timeout(client, connect_timeout, :default_connect_timeout) + connect_timeout = _resolve_connect_timeout(client, connect_timeout) request_timeout = _client_default_timeout(client, request_timeout, :default_request_timeout) response_header_timeout = _client_default_timeout(client, response_header_timeout, :default_response_header_timeout) read_idle_timeout = _client_default_timeout(client, read_idle_timeout, :default_read_idle_timeout) @@ -2071,7 +2082,8 @@ Keyword arguments: is created - `connect_timeout`: connection establishment timeout in seconds, covering DNS, TCP connect, HTTP proxy `CONNECT` or SOCKS5 handshakes, TLS handshake, and - HTTP/2 session setup in the high-level client paths + HTTP/2 session setup in the high-level client paths. When not passed, the + `client`'s `connect_timeout` default applies if set, else 30; `0` disables - `request_timeout`: overall request deadline in seconds - `response_header_timeout`: maximum time to wait for response headers after the request has been sent diff --git a/src/http_stream.jl b/src/http_stream.jl index da9c538bc..6b2dd434a 100644 --- a/src/http_stream.jl +++ b/src/http_stream.jl @@ -450,7 +450,7 @@ function open( basicauth=nothing, client::Union{Nothing,Client}=nothing, context::Union{Nothing,RequestContext}=nothing, - connect_timeout::Real=30, + connect_timeout::Union{Nothing,Real}=nothing, request_timeout::Real=0, response_header_timeout::Real=0, read_idle_timeout::Real=0, @@ -486,7 +486,7 @@ function open( logtag=logtag, ) # Apply client defaults (per-call wins) - connect_timeout = _client_default_timeout(client, connect_timeout, :default_connect_timeout) + connect_timeout = _resolve_connect_timeout(client, connect_timeout) request_timeout = _client_default_timeout(client, request_timeout, :default_request_timeout) response_header_timeout = _client_default_timeout(client, response_header_timeout, :default_response_header_timeout) read_idle_timeout = _client_default_timeout(client, read_idle_timeout, :default_read_idle_timeout) diff --git a/test/http_client_tests.jl b/test/http_client_tests.jl index ff8bb3ab6..baab49b3e 100644 --- a/test/http_client_tests.jl +++ b/test/http_client_tests.jl @@ -2576,6 +2576,84 @@ end end end +@testset "HTTP client-level connect_timeout resolution" begin + client = HT.Client(connect_timeout=5) + try + # Unset kwarg falls back to the client default. + @test HT._resolve_connect_timeout(client, nothing) == 5.0 + # Explicit per-call values override the client default, including an + # explicit 30 (the built-in default value). + @test HT._resolve_connect_timeout(client, 7) == 7 + @test HT._resolve_connect_timeout(client, 30) == 30 + # Explicit 0 falls through to the client default, like the other + # timeout kwargs. + @test HT._resolve_connect_timeout(client, 0) == 5.0 + finally + close(client) + end + unset = HT.Client() + try + # Neither the call nor the client sets it: built-in 30s default. + @test HT._resolve_connect_timeout(unset, nothing) == 30 + # Explicit 0 without a client default disables the connect timeout. + @test HT._resolve_connect_timeout(unset, 0) == 0 + finally + close(unset) + end + @test HT._resolve_connect_timeout(nothing, nothing) == 30 + @test HT._resolve_connect_timeout(nothing, 12) == 12 + @test HT._resolve_connect_timeout(nothing, 0) == 0 +end + +@testset "HTTP Client(connect_timeout=...) default applies to requests" begin + listener = ND.listen("tcp", "127.0.0.1:0"; backlog = 8) + laddr = NC.addr(listener)::NC.SocketAddrV4 + address = ND.join_host_port("127.0.0.1", Int(laddr.port)) + server_task = errormonitor(Threads.@spawn begin + for _ in 1:2 + conn = NC.accept(listener) + try + sleep(0.20) + finally + HTTP.@try_ignore NC.close(conn) + end + end + return nothing + end) + client = HT.Client( + transport=HT.Transport(tls_config=TL.Config(verify_peer=false), max_idle_per_host=4, max_idle_total=4), + connect_timeout=0.05, + ) + try + # The request does not pass connect_timeout, so the client default + # must bound the (stalled) TLS handshake. + err = try + HT.get("https://$(address)/stall"; client=client, retry=false) + nothing + catch ex + ex + end + @test err !== nothing + @test _is_timeout_error_client(err::Exception) + + # Same through the HTTP.open streaming path. + stream_err = try + HT.open(:GET, "https://$(address)/stall"; client=client, retry=false) do stream + nothing + end + nothing + catch ex + ex + end + @test stream_err !== nothing + @test _is_timeout_error_client(stream_err::Exception) + _wait_task_client!(server_task) + finally + close(client.transport) + HTTP.@try_ignore NC.close(listener) + end +end + @testset "HTTP transport error wrapping" begin refused = ND.OpError("dial", "tcp", nothing, nothing, Base.SystemError("connect", Base.Libc.ECONNREFUSED)) wrapped_refused = HT._wrap_client_transport_error(refused, "request", Int64(0), Int64(0))