Skip to content

Commit 296f7f5

Browse files
fredrikekreclaude
andauthored
Respect the Client's configured connect_timeout value (#1327)
Since connect_timeout have a different non-zero default value compared to the other timeouts we never picked up the value from the Client since it looked like the connect_timeout = 30 was always explicitly set by the caller. Co-authored-by: Claude Code <noreply@anthropic.com>
1 parent 6665dc9 commit 296f7f5

4 files changed

Lines changed: 101 additions & 7 deletions

File tree

docs/src/guides/client.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,9 @@ Recognized client defaults:
201201
- `default_query`: dict, named-tuple, or vector-of-pairs of query parameters; per-call keys override matching defaults
202202
- `default_basicauth`: applied unless the call passes `basicauth` or an explicit `Authorization` header
203203
- `connect_timeout`, `request_timeout`, `response_header_timeout`,
204-
`read_idle_timeout`, `write_idle_timeout`: applied when the per-call timeout
205-
is `0` (the default)
204+
`read_idle_timeout`, `write_idle_timeout`: applied when the call does not pass the
205+
matching keyword. When neither the call nor the client sets it the timeouts are disabled
206+
(i.e. set to `0`) except `connect_timeout` which defaults to `30`
206207

207208
### Positional `Client` calls
208209

@@ -454,6 +455,9 @@ read timeout:
454455
- `expect_continue_timeout` controls how long HTTP/1 uploads wait on
455456
`100-continue` before sending the body anyway
456457

458+
`connect_timeout` defaults to 30 seconds when neither the call nor a `Client`
459+
sets it; the other timeouts are disabled by default.
460+
457461
`readtimeout` is still accepted for compatibility, but it is deprecated and now
458462
behaves like `read_idle_timeout`.
459463

src/http_client.jl

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,6 +1822,17 @@ For timeout kwargs left at `0`, fall back to the client's default for the named
18221822
return getfield(client, field)
18231823
end
18241824

1825+
const _DEFAULT_CONNECT_TIMEOUT = 30
1826+
1827+
@inline function _resolve_connect_timeout(client::Union{Nothing,Client}, value::Union{Nothing,Real})::Real
1828+
if value === nothing
1829+
client === nothing && return _DEFAULT_CONNECT_TIMEOUT
1830+
default = client.default_connect_timeout
1831+
return default > 0 ? default : _DEFAULT_CONNECT_TIMEOUT
1832+
end
1833+
return _client_default_timeout(client, value, :default_connect_timeout)
1834+
end
1835+
18251836

18261837
function request(
18271838
method::Union{AbstractString,Symbol},
@@ -1854,7 +1865,7 @@ function request(
18541865
max_sse_event_bytes::Integer=_DEFAULT_SSE_CLIENT_MAX_EVENT_BYTES,
18551866
client::Union{Nothing,Client}=nothing,
18561867
context::Union{Nothing,RequestContext}=nothing,
1857-
connect_timeout::Real=30,
1868+
connect_timeout::Union{Nothing,Real}=nothing,
18581869
request_timeout::Real=0,
18591870
response_header_timeout::Real=0,
18601871
read_idle_timeout::Real=0,
@@ -1907,7 +1918,7 @@ function request(
19071918
max_sse_line_bytes >= 0 || throw(ArgumentError("max_sse_line_bytes must be >= 0"))
19081919
max_sse_event_bytes >= 0 || throw(ArgumentError("max_sse_event_bytes must be >= 0"))
19091920
# Merge per-call values with client defaults (per-call wins)
1910-
connect_timeout = _client_default_timeout(client, connect_timeout, :default_connect_timeout)
1921+
connect_timeout = _resolve_connect_timeout(client, connect_timeout)
19111922
request_timeout = _client_default_timeout(client, request_timeout, :default_request_timeout)
19121923
response_header_timeout = _client_default_timeout(client, response_header_timeout, :default_response_header_timeout)
19131924
read_idle_timeout = _client_default_timeout(client, read_idle_timeout, :default_read_idle_timeout)
@@ -2071,7 +2082,8 @@ Keyword arguments:
20712082
is created
20722083
- `connect_timeout`: connection establishment timeout in seconds, covering DNS,
20732084
TCP connect, HTTP proxy `CONNECT` or SOCKS5 handshakes, TLS handshake, and
2074-
HTTP/2 session setup in the high-level client paths
2085+
HTTP/2 session setup in the high-level client paths. When not passed, the
2086+
`client`'s `connect_timeout` default applies if set, else 30; `0` disables
20752087
- `request_timeout`: overall request deadline in seconds
20762088
- `response_header_timeout`: maximum time to wait for response headers after
20772089
the request has been sent

src/http_stream.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ function open(
450450
basicauth=nothing,
451451
client::Union{Nothing,Client}=nothing,
452452
context::Union{Nothing,RequestContext}=nothing,
453-
connect_timeout::Real=30,
453+
connect_timeout::Union{Nothing,Real}=nothing,
454454
request_timeout::Real=0,
455455
response_header_timeout::Real=0,
456456
read_idle_timeout::Real=0,
@@ -486,7 +486,7 @@ function open(
486486
logtag=logtag,
487487
)
488488
# Apply client defaults (per-call wins)
489-
connect_timeout = _client_default_timeout(client, connect_timeout, :default_connect_timeout)
489+
connect_timeout = _resolve_connect_timeout(client, connect_timeout)
490490
request_timeout = _client_default_timeout(client, request_timeout, :default_request_timeout)
491491
response_header_timeout = _client_default_timeout(client, response_header_timeout, :default_response_header_timeout)
492492
read_idle_timeout = _client_default_timeout(client, read_idle_timeout, :default_read_idle_timeout)

test/http_client_tests.jl

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,6 +2576,84 @@ end
25762576
end
25772577
end
25782578

2579+
@testset "HTTP client-level connect_timeout resolution" begin
2580+
client = HT.Client(connect_timeout=5)
2581+
try
2582+
# Unset kwarg falls back to the client default.
2583+
@test HT._resolve_connect_timeout(client, nothing) == 5.0
2584+
# Explicit per-call values override the client default, including an
2585+
# explicit 30 (the built-in default value).
2586+
@test HT._resolve_connect_timeout(client, 7) == 7
2587+
@test HT._resolve_connect_timeout(client, 30) == 30
2588+
# Explicit 0 falls through to the client default, like the other
2589+
# timeout kwargs.
2590+
@test HT._resolve_connect_timeout(client, 0) == 5.0
2591+
finally
2592+
close(client)
2593+
end
2594+
unset = HT.Client()
2595+
try
2596+
# Neither the call nor the client sets it: built-in 30s default.
2597+
@test HT._resolve_connect_timeout(unset, nothing) == 30
2598+
# Explicit 0 without a client default disables the connect timeout.
2599+
@test HT._resolve_connect_timeout(unset, 0) == 0
2600+
finally
2601+
close(unset)
2602+
end
2603+
@test HT._resolve_connect_timeout(nothing, nothing) == 30
2604+
@test HT._resolve_connect_timeout(nothing, 12) == 12
2605+
@test HT._resolve_connect_timeout(nothing, 0) == 0
2606+
end
2607+
2608+
@testset "HTTP Client(connect_timeout=...) default applies to requests" begin
2609+
listener = ND.listen("tcp", "127.0.0.1:0"; backlog = 8)
2610+
laddr = NC.addr(listener)::NC.SocketAddrV4
2611+
address = ND.join_host_port("127.0.0.1", Int(laddr.port))
2612+
server_task = errormonitor(Threads.@spawn begin
2613+
for _ in 1:2
2614+
conn = NC.accept(listener)
2615+
try
2616+
sleep(0.20)
2617+
finally
2618+
HTTP.@try_ignore NC.close(conn)
2619+
end
2620+
end
2621+
return nothing
2622+
end)
2623+
client = HT.Client(
2624+
transport=HT.Transport(tls_config=TL.Config(verify_peer=false), max_idle_per_host=4, max_idle_total=4),
2625+
connect_timeout=0.05,
2626+
)
2627+
try
2628+
# The request does not pass connect_timeout, so the client default
2629+
# must bound the (stalled) TLS handshake.
2630+
err = try
2631+
HT.get("https://$(address)/stall"; client=client, retry=false)
2632+
nothing
2633+
catch ex
2634+
ex
2635+
end
2636+
@test err !== nothing
2637+
@test _is_timeout_error_client(err::Exception)
2638+
2639+
# Same through the HTTP.open streaming path.
2640+
stream_err = try
2641+
HT.open(:GET, "https://$(address)/stall"; client=client, retry=false) do stream
2642+
nothing
2643+
end
2644+
nothing
2645+
catch ex
2646+
ex
2647+
end
2648+
@test stream_err !== nothing
2649+
@test _is_timeout_error_client(stream_err::Exception)
2650+
_wait_task_client!(server_task)
2651+
finally
2652+
close(client.transport)
2653+
HTTP.@try_ignore NC.close(listener)
2654+
end
2655+
end
2656+
25792657
@testset "HTTP transport error wrapping" begin
25802658
refused = ND.OpError("dial", "tcp", nothing, nothing, Base.SystemError("connect", Base.Libc.ECONNREFUSED))
25812659
wrapped_refused = HT._wrap_client_transport_error(refused, "request", Int64(0), Int64(0))

0 commit comments

Comments
 (0)