Merge branch 'master' into pg/HEAD-no-body-4

Project.toml

@@ -1,7 +1,7 @@
 name = "HTTP"
 uuid = "cd3eb016-35fb-5094-929b-558a96fad6f3"
 authors = ["Jacob Quinn", "contributors: https://github.com/JuliaWeb/HTTP.jl/graphs/contributors"]
-version = "1.9.14"
+version = "1.10.0"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

src/Connections.jl

@@ -478,9 +478,9 @@ function keepalive!(tcp)
     Base.iolock_begin()
     try
         Base.check_open(tcp)
-        err = ccall(:uv_tcp_keepalive, Cint, (Ptr{Nothing}, Cint, Cuint),
+        msg = ccall(:uv_tcp_keepalive, Cint, (Ptr{Nothing}, Cint, Cuint),
                                             tcp.handle, 1, 1)
-        Base.uv_error("failed to set keepalive on tcp socket", err)
+        Base.uv_error("failed to set keepalive on tcp socket", msg)
     finally
         Base.iolock_end()
     end
@@ -571,9 +571,10 @@ function getconnection(::Type{SSLContext},
 end
 
 function getconnection(::Type{SSLStream},
-    host::AbstractString,
-    port::AbstractString;
-    kw...)::SSLStream
+                       host::AbstractString,
+                       port::AbstractString;
+                       kw...)::SSLStream
+
     port = isempty(port) ? "443" : port
     @debugv 2 "SSL connect: $host:$port..."
     tcp = getconnection(TCPSocket, host, port; kw...)

src/Exceptions.jl

@@ -100,8 +100,13 @@ function current_exceptions_to_string()
     buf = IOBuffer()
     println(buf)
     println(buf, "\n===========================\nHTTP Error message:\n")
-    Base.display_error(buf, Base.catch_stack())
+    exc = @static if VERSION >= v"1.8.0-"
+        Base.current_exceptions()
+    else
+        Base.catch_stack()
+    end
+    Base.display_error(buf, exc)
     return String(take!(buf))
 end
 
-end # module Exceptions
+end # module Exceptions

src/HTTP.jl

@@ -113,7 +113,7 @@ as an additional keyword argument to the request.
  - any `AbstractString` or `AbstractVector{UInt8}` which will be sent "as is" for the request body
  - a readable `IO` stream or any `IO`-like type `T` for which
    `eof(T)` and `readavailable(T)` are defined. This stream will be read and sent until `eof` is `true`.
-   This object should support the `mark`/`reset` methods if request retires are desired (if not, no retries will be attempted).
+   This object should support the `mark`/`reset` methods if request retries are desired (if not, no retries will be attempted).
  - Any collection or iterable of the above (`AbstractDict`, `AbstractString`, `AbstractVector{UInt8}`, or `IO`)
    which will result in a "chunked" request body, where each iterated element will be sent as a separate chunk
  - a [`HTTP.Form`](@ref), which will be serialized as the "multipart/form-data" content-type

src/Handlers.jl

@@ -498,6 +498,6 @@ are expected to be stored in the `req.context[:cookies]` of the
 request context as implemented in the [`HTTP.Handlers.cookie_middleware`](@ref)
 middleware.
 """
-getcookies(req) = get(() => Cookie[], req.context, :cookies)
+getcookies(req) = get(() -> Cookie[], req.context, :cookies)
 
 end # module

src/Messages.jl

@@ -235,8 +235,8 @@ issafe(method) = method in ["GET", "HEAD", "OPTIONS", "TRACE"]
 Does this `Response` have an error status?
 """
 iserror(r::Response) = iserror(r.status)
-iserror(status) = status != 0 && status != 100 && status != 101 &&
-                 (status < 200 || status >= 300) && !isredirect(status)
+iserror(status::Integer) = status != 0 && status != 100 && status != 101 &&
+                          (status < 200 || status >= 300) && !isredirect(status)
 
 """
     isredirect(::Response)
@@ -245,7 +245,7 @@ Does this `Response` have a redirect status?
 """
 isredirect(r::Response) = isredirect(r.status)
 isredirect(r::Request) = allow_redirects(r) && !redirectlimitreached(r)
-isredirect(status) = status in (301, 302, 303, 307, 308)
+isredirect(status::Integer) = status in (301, 302, 303, 307, 308)
 
 # whether the redirect limit has been reached for a given request
 # set in the RedirectRequest layer once the limit is reached

src/Servers.jl

@@ -192,8 +192,9 @@ shutdown(::Nothing) = nothing
 function shutdown(fn::Function)
     try
         fn()
-    catch e
-        @error "shutdown function $fn failed" exception=(e, catch_backtrace())
+    catch
+        msg = current_exceptions_to_string()
+        @error "shutdown function $fn failed. $msg"
     end
 end
 
@@ -392,7 +393,8 @@ function listenloop(f, listener, conns, tcpisvalid,
             if e isa Base.IOError && e.code == Base.UV_ECONNABORTED
                 verbose >= 0 && @infov 1 "Server on $(listener.hostname):$(listener.hostport) closing"
             else
-                @errorv 2 "Server on $(listener.hostname):$(listener.hostport) errored" exception=(e, catch_backtrace())
+                msg = current_exceptions_to_string()
+                @errorv 2 "Server on $(listener.hostname):$(listener.hostport) errored. $msg"
                 # quick little sleep in case there's a temporary
                 # local error accepting and this might help avoid quickly re-erroring
                 sleep(0.05 + rand() * 0.05)
@@ -431,7 +433,8 @@ function handle_connection(f, c::Connection, listener, readtimeout, access_log)
                 if e isa ParseError
                     write(c, Response(e.code == :HEADER_SIZE_EXCEEDS_LIMIT ? 431 : 400, string(e.code)))
                 end
-                @debugv 1 "handle_connection startread error" exception=(e, catch_backtrace())
+                msg = current_exceptions_to_string()
+                @debugv 1 "handle_connection startread error. $msg"
                 break
             end
 
@@ -458,7 +461,8 @@ function handle_connection(f, c::Connection, listener, readtimeout, access_log)
                 # The remote can close the stream whenever it wants to, but there's nothing
                 # anyone can do about it on this side. No reason to log an error in that case.
                 level = e isa Base.IOError && !isopen(c) ? Logging.Debug : Logging.Error
-                @logmsgv 1 level "handle_connection handler error" exception=(e, stacktrace(catch_backtrace()))
+                msg = current_exceptions_to_string()
+                @logmsgv 1 level "handle_connection handler error. $msg"
 
                 if isopen(http) && !iswritable(http)
                     request.response.status = 500
@@ -473,9 +477,10 @@ function handle_connection(f, c::Connection, listener, readtimeout, access_log)
                 end
             end
         end
-    catch e
+    catch
         # we should be catching everything inside the while loop, but just in case
-        @errorv 1 "error while handling connection" exception=(e, catch_backtrace())
+        msg = current_exceptions_to_string()
+        @errorv 1 "error while handling connection. $msg"
     finally
         if readtimeout > 0
             wait_for_timeout[] = false

src/Streams.jl

@@ -326,7 +326,7 @@ function IOExtras.readuntil(http::Stream, f::Function)::ByteView
         bytes = IOExtras.readuntil(http.stream, f)
         update_ntoread(http, length(bytes))
         return bytes
-    catch e
+    catch
         # if we error, it means we didn't find what we were looking for
         return UInt8[]
     end

src/WebSockets.jl

@@ -3,6 +3,7 @@ module WebSockets
 using Base64, LoggingExtras, UUIDs, Sockets, Random
 using MbedTLS: digest, MD_SHA1, SSLContext
 using ..IOExtras, ..Streams, ..Connections, ..Messages, ..Conditions, ..Servers
+using ..Exceptions: current_exceptions_to_string
 import ..open
 import ..HTTP # for doc references
 
@@ -439,7 +440,8 @@ function upgrade(f::Function, http::Streams.Stream; suppress_close_error::Bool=f
         f(ws)
     catch e
         if !isok(e)
-            suppress_close_error || @error "$(ws.id): Unexpected websocket server error" exception=(e, catch_backtrace())
+            msg = current_exceptions_to_string()
+            suppress_close_error || @error "$(ws.id): Unexpected websocket server error. $msg"
         end
         if !isclosed(ws)
             if e isa WebSocketError && e.message isa CloseFrameBody

src/clientlayers/ConnectionRequest.jl

@@ -80,8 +80,8 @@ function connectionlayer(handler)
             io = newconnection(IOType, url.host, url.port; readtimeout=readtimeout, connect_timeout=connect_timeout, kw...)
         catch e
             if logerrors
-                err = current_exceptions_to_string()
-                @error err type=Symbol("HTTP.ConnectError") method=req.method url=req.url context=req.context logtag=logtag
+                msg = current_exceptions_to_string()
+                @error msg type=Symbol("HTTP.ConnectError") method=req.method url=req.url context=req.context logtag=logtag
             end
             req.context[:connect_errors] = get(req.context, :connect_errors, 0) + 1
             throw(ConnectError(string(url), e))
@@ -119,22 +119,22 @@ function connectionlayer(handler)
             stream = Stream(req.response, io)
             return handler(stream; readtimeout=readtimeout, logerrors=logerrors, logtag=logtag, kw...)
         catch e
+            shouldreuse = false
             # manually unwrap CompositeException since it's not defined as a "wrapper" exception by ExceptionUnwrapping
             while e isa CompositeException
                 e = e.exceptions[1]
             end
             root_err = ExceptionUnwrapping.unwrap_exception_to_root(e)
             # don't log if it's an HTTPError since we should have already logged it
-            if logerrors && err isa StatusError
-                err = current_exceptions_to_string()
-                @error err type=Symbol("HTTP.StatusError") method=req.method url=req.url context=req.context logtag=logtag
+            if logerrors && root_err isa StatusError
+                msg = current_exceptions_to_string()
+                @error msg type=Symbol("HTTP.StatusError") method=req.method url=req.url context=req.context logtag=logtag
             end
             if logerrors && !ExceptionUnwrapping.has_wrapped_exception(e, HTTPError)
-                err = current_exceptions_to_string(e)
-                @error err type=Symbol("HTTP.ConnectionRequest") method=req.method url=req.url context=req.context logtag=logtag
+                msg = current_exceptions_to_string()
+                @error msg type=Symbol("HTTP.ConnectionRequest") method=req.method url=req.url context=req.context logtag=logtag
             end
-            @debugv 1 "❗️  ConnectionLayer $e. Closing: $io"
-            shouldreuse = false
+            @debugv 1 "❗️  ConnectionLayer $root_err. Closing: $io"
             if @isdefined(stream) && stream.nwritten == -1
                 # we didn't write anything, so don't need to worry about
                 # idempotency of the request

src/clientlayers/RetryRequest.jl

@@ -1,6 +1,6 @@
 module RetryRequest
 
-using Sockets, LoggingExtras, MbedTLS, OpenSSL
+using Sockets, LoggingExtras, MbedTLS, OpenSSL, ExceptionUnwrapping
 using ..IOExtras, ..Messages, ..Strings, ..ExceptionRequest, ..Exceptions
 
 export retrylayer
@@ -76,9 +76,10 @@ function retrylayer(handler)
     end
 end
 
-isrecoverable(ex) = true
-isrecoverable(ex::CapturedException) = isrecoverable(ex.ex)
-isrecoverable(ex::ConnectError) = isrecoverable(ex.error)
+isrecoverable(ex) = is_wrapped_exception(ex) ? isrecoverable(unwrap_exception(ex)) : false
+isrecoverable(::Union{Base.EOFError, Base.IOError, MbedTLS.MbedException, OpenSSL.OpenSSLError}) = true
+isrecoverable(ex::ArgumentError) = ex.msg == "stream is closed or unusable"
+isrecoverable(ex::CompositeException) = all(isrecoverable, ex.exceptions)
 # Treat all DNS errors except `EAI_AGAIN`` as non-recoverable
 # Ref: https://github.com/JuliaLang/julia/blob/ec8df3da3597d0acd503ff85ac84a5f8f73f625b/stdlib/Sockets/src/addrinfo.jl#L108-L112
 isrecoverable(ex::Sockets.DNSError) = (ex.code == Base.UV_EAI_AGAIN)
@@ -92,9 +93,11 @@ end
 
 function no_retry_reason(ex, req)
     buf = IOBuffer()
+    unwrapped_ex = unwrap_exception(ex)
     show(IOContext(buf, :compact => true), req)
     print(buf, ", ",
-        ex isa StatusError ? "HTTP $(ex.status): " :
+        unwrapped_ex isa StatusError ? "HTTP $(ex.status): " :
+        !isrecoverable(unwrapped_ex) ? "unrecoverable exception: " :
         !isbytes(req.body) ? "request streamed, " : "",
         !isbytes(req.response.body) ? "response streamed, " : "",
         !isidempotent(req) ? "$(req.method) non-idempotent" : "")

src/clientlayers/StreamRequest.jl

@@ -68,12 +68,12 @@ function streamlayer(stream::Stream; iofunction=nothing, decompress::Union{Nothi
                 end
             end
         end
-    catch e
+    catch
         if timedout === nothing || !timedout[]
             req.context[:io_errors] = get(req.context, :io_errors, 0) + 1
             if logerrors
-                err = current_exceptions_to_string()
-                @error err type=Symbol("HTTP.IOError") method=req.method url=req.url context=req.context logtag=logtag
+                msg = current_exceptions_to_string()
+                @error msg type=Symbol("HTTP.IOError") method=req.method url=req.url context=req.context logtag=logtag
             end
         end
         rethrow()

src/clientlayers/TimeoutRequest.jl

@@ -2,6 +2,7 @@ module TimeoutRequest
 
 using ..Connections, ..Streams, ..Exceptions, ..Messages
 using LoggingExtras, ConcurrentUtilities
+using ..Exceptions: current_exceptions_to_string
 
 export timeoutlayer
 
@@ -25,8 +26,8 @@ function timeoutlayer(handler)
                 req = stream.message.request
                 req.context[:timeout_errors] = get(req.context, :timeout_errors, 0) + 1
                 if logerrors
-                    err = current_exceptions_to_string()
-                    @error err type=Symbol("HTTP.TimeoutError") method=req.method url=req.url context=req.context timeout=readtimeout logtag=logtag
+                    msg = current_exceptions_to_string()
+                    @error msg type=Symbol("HTTP.TimeoutError") method=req.method url=req.url context=req.context timeout=readtimeout logtag=logtag
                 end
                 e = Exceptions.TimeoutError(readtimeout)
             end

test/client.jl

@@ -645,6 +645,53 @@ end
     end
 end
 
+@testset "Don't retry on internal exceptions" begin
+    kws = (retry_delays = [10, 20, 30], retries=3) # ~ 60 secs
+    max_wait = 30
+
+    function test_finish_within(f, secs)
+        timedout = Ref(false)
+        t = Timer((t)->(timedout[] = true), secs)
+        try
+            f()
+        finally
+            close(t)
+        end
+        @test !timedout[]
+    end
+
+    expected = ErrorException("request")
+    test_finish_within(max_wait) do
+        @test_throws expected ErrorRequest.get("https://$httpbin/ip"; request_exception=expected, kws...)
+    end
+    expected = ArgumentError("request")
+    test_finish_within(max_wait) do
+        @test_throws expected ErrorRequest.get("https://$httpbin/ip"; request_exception=expected, kws...)
+    end
+
+    test_finish_within(max_wait) do
+        expected = ErrorException("stream")
+        e = try
+            ErrorRequest.get("https://$httpbin/ip"; stream_exception=expected, kws...)
+        catch e
+            e
+        end
+        @assert e isa HTTP.RequestError
+        @test e.error == expected
+    end
+
+    test_finish_within(max_wait) do
+        expected = ArgumentError("stream")
+        e = try
+            ErrorRequest.get("https://$httpbin/ip"; stream_exception=expected, kws...)
+        catch e
+            e
+        end
+        @assert e isa HTTP.RequestError
+        @test e.error == expected
+    end
+end
+
 @testset "Retry with ConnectError" begin
     mktemp() do path, io
         redirect_stdout(io) do
@@ -668,12 +715,41 @@ end
     end
 
     # isrecoverable tests
-    @test HTTP.RetryRequest.isrecoverable(nothing)
-    @test HTTP.RetryRequest.isrecoverable(ErrorException(""))
-    @test HTTP.RetryRequest.isrecoverable(Sockets.DNSError("localhost", Base.UV_EAI_AGAIN))
-    @test !HTTP.RetryRequest.isrecoverable(Sockets.DNSError("localhost", Base.UV_EAI_NONAME))
-    @test HTTP.RetryRequest.isrecoverable(HTTP.Exceptions.ConnectError("http://localhost", Sockets.DNSError("localhost", Base.UV_EAI_AGAIN)))
-    @test !HTTP.RetryRequest.isrecoverable(HTTP.Exceptions.ConnectError("http://localhost", Sockets.DNSError("localhost", Base.UV_EAI_NONAME)))
+    @test !HTTP.RetryRequest.isrecoverable(nothing)
+
+    @test !HTTP.RetryRequest.isrecoverable(ErrorException(""))
+    @test !HTTP.RetryRequest.isrecoverable(ArgumentError("yikes"))
+    @test HTTP.RetryRequest.isrecoverable(ArgumentError("stream is closed or unusable"))
+
+    @test HTTP.RetryRequest.isrecoverable(HTTP.RequestError(nothing, ArgumentError("stream is closed or unusable")))
+    @test !HTTP.RetryRequest.isrecoverable(HTTP.RequestError(nothing, ArgumentError("yikes")))
+
+    @test HTTP.RetryRequest.isrecoverable(CapturedException(ArgumentError("stream is closed or unusable"), Any[]))
+
+    recoverable_dns_error = Sockets.DNSError("localhost", Base.UV_EAI_AGAIN)
+    unrecoverable_dns_error = Sockets.DNSError("localhost", Base.UV_EAI_NONAME)
+    @test HTTP.RetryRequest.isrecoverable(recoverable_dns_error)
+    @test !HTTP.RetryRequest.isrecoverable(unrecoverable_dns_error)
+    @test HTTP.RetryRequest.isrecoverable(HTTP.Exceptions.ConnectError("http://localhost", recoverable_dns_error))
+    @test !HTTP.RetryRequest.isrecoverable(HTTP.Exceptions.ConnectError("http://localhost", unrecoverable_dns_error))
+    @test HTTP.RetryRequest.isrecoverable(CompositeException([
+        recoverable_dns_error,
+        HTTP.Exceptions.ConnectError("http://localhost", recoverable_dns_error),
+    ]))
+    @test HTTP.RetryRequest.isrecoverable(CompositeException([
+        recoverable_dns_error,
+        HTTP.Exceptions.ConnectError("http://localhost", recoverable_dns_error),
+        CompositeException([recoverable_dns_error])
+    ]))
+    @test !HTTP.RetryRequest.isrecoverable(CompositeException([
+        recoverable_dns_error,
+        HTTP.Exceptions.ConnectError("http://localhost", unrecoverable_dns_error),
+    ]))
+    @test !HTTP.RetryRequest.isrecoverable(CompositeException([
+        recoverable_dns_error,
+        HTTP.Exceptions.ConnectError("http://localhost", recoverable_dns_error),
+        CompositeException([unrecoverable_dns_error])
+    ]))
 end
 
 findnewline(bytes) = something(findfirst(==(UInt8('\n')), bytes), 0)

test/handlers.jl

@@ -77,4 +77,8 @@ using HTTP, Test
     @test r(HTTP.Request("GET", "/api/widgets/abc/subwidget")) == 15
     @test r(HTTP.Request("GET", "/api/widgets/abc/subwidgetname")) == 16
 
+    cookie = HTTP.Cookie("abc", "def")
+    req = HTTP.Request("GET", "/")
+    req.context[:cookies] = [cookie]
+    @test HTTP.getcookies(req)[1] == cookie
 end