Ensure connect_timeout doesn't count acquire time on the connection pool (#1075)

* Ensure connect_timeout doesn't count acquire time on the connection pool

* Fix connect_timeout defaults/docs

Author: quinnj

src/Connections.jl

@@ -446,6 +446,7 @@ function newconnection(::Type{T},
                        connection_limit=nothing,
                        forcenew::Bool=false,
                        idle_timeout=typemax(Int),
+                       connect_timeout::Int=30,
                        require_ssl_verification::Bool=NetworkOptions.verify_host(host, "SSL"),
                        keepalive::Bool=true,
                        kw...) where {T <: IO}
@@ -457,8 +458,13 @@ function newconnection(::Type{T},
             isvalid=c->connection_isvalid(c, Int(idle_timeout))) do
         Connection(host, port,
             idle_timeout, require_ssl_verification, keepalive,
-            getconnection(T, host, port;
-                require_ssl_verification=require_ssl_verification, keepalive=keepalive, kw...)
+            connect_timeout > 0 ?
+                try_with_timeout(_ ->
+                    getconnection(T, host, port;
+                        require_ssl_verification=require_ssl_verification, keepalive=keepalive, kw...),
+                    connect_timeout) :
+                getconnection(T, host, port;
+                    require_ssl_verification=require_ssl_verification, keepalive=keepalive, kw...)
         )
     end
 end

src/HTTP.jl

@@ -137,7 +137,7 @@ Supported optional keyword arguments:
     will be written to this stream instead of returned as a `Vector{UInt8}`.
  - `verbose = 0`, set to `1` or `2` for increasingly verbose logging of the
     request and response process
- - `connect_timeout = 10`, close the connection after this many seconds if it
+ - `connect_timeout = 30`, close the connection after this many seconds if it
    is still attempting to connect. Use `connect_timeout = 0` to disable.
  - `pool = nothing`, an `HTTP.Pool` object to use for managing the reuse of connections between requests.
     By default, a global pool is used, which is shared across all requests. To create a pool for a specific set of requests,

src/clientlayers/ConnectionRequest.jl

@@ -55,7 +55,7 @@ Close the connection if the request throws an exception.
 Otherwise leave it open so that it can be reused.
 """
 function connectionlayer(handler)
-    return function connections(req; proxy=getproxy(req.url.scheme, req.url.host), socket_type::Type=TCPSocket, socket_type_tls::Type=SOCKET_TYPE_TLS[], readtimeout::Int=0, connect_timeout::Int=60, logerrors::Bool=false, logtag=nothing, kw...)
+    return function connections(req; proxy=getproxy(req.url.scheme, req.url.host), socket_type::Type=TCPSocket, socket_type_tls::Type=SOCKET_TYPE_TLS[], readtimeout::Int=0, connect_timeout::Int=30, logerrors::Bool=false, logtag=nothing, kw...)
         local io, stream
         if proxy !== nothing
             target_url = req.url
@@ -77,13 +77,7 @@ function connectionlayer(handler)
         IOType = sockettype(url, socket_type, socket_type_tls)
         start_time = time()
         try
-            io = if connect_timeout > 0
-                try_with_timeout(connect_timeout) do _
-                    newconnection(IOType, url.host, url.port; readtimeout=readtimeout, kw...)
-                end
-            else
-                newconnection(IOType, url.host, url.port; readtimeout=readtimeout, kw...)
-            end
+            io = newconnection(IOType, url.host, url.port; readtimeout=readtimeout, connect_timeout=connect_timeout, kw...)
         catch e
             if logerrors
                 err = current_exceptions_to_string()