diff --git a/Project.toml b/Project.toml index bbb0cf7ed..565193acb 100644 --- a/Project.toml +++ b/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" diff --git a/src/Connections.jl b/src/Connections.jl index 4534401e0..60d6eb9b7 100644 --- a/src/Connections.jl +++ b/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...) diff --git a/src/Exceptions.jl b/src/Exceptions.jl index 914aff317..ba7a30df0 100644 --- a/src/Exceptions.jl +++ b/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 \ No newline at end of file +end # module Exceptions diff --git a/src/HTTP.jl b/src/HTTP.jl index cc3d027b8..941bab9f5 100644 --- a/src/HTTP.jl +++ b/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 diff --git a/src/Handlers.jl b/src/Handlers.jl index a121015c7..f4078ecd5 100644 --- a/src/Handlers.jl +++ b/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 diff --git a/src/Messages.jl b/src/Messages.jl index 51ad50bcb..bc61d10e9 100644 --- a/src/Messages.jl +++ b/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 diff --git a/src/Servers.jl b/src/Servers.jl index 6afb16590..4a8f37194 100644 --- a/src/Servers.jl +++ b/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 diff --git a/src/Streams.jl b/src/Streams.jl index 116bf2000..220d11c4e 100644 --- a/src/Streams.jl +++ b/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 diff --git a/src/WebSockets.jl b/src/WebSockets.jl index 56b6933a9..ab9e532da 100644 --- a/src/WebSockets.jl +++ b/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 diff --git a/src/clientlayers/ConnectionRequest.jl b/src/clientlayers/ConnectionRequest.jl index 4b69187f3..564a8f088 100644 --- a/src/clientlayers/ConnectionRequest.jl +++ b/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 diff --git a/src/clientlayers/RetryRequest.jl b/src/clientlayers/RetryRequest.jl index f4e8d1c19..001c22128 100644 --- a/src/clientlayers/RetryRequest.jl +++ b/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" : "") diff --git a/src/clientlayers/StreamRequest.jl b/src/clientlayers/StreamRequest.jl index 3ba642702..4442574b6 100644 --- a/src/clientlayers/StreamRequest.jl +++ b/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() diff --git a/src/clientlayers/TimeoutRequest.jl b/src/clientlayers/TimeoutRequest.jl index 94e58b587..13486c081 100644 --- a/src/clientlayers/TimeoutRequest.jl +++ b/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 diff --git a/test/client.jl b/test/client.jl index f886db705..868ac8ded 100644 --- a/test/client.jl +++ b/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) diff --git a/test/handlers.jl b/test/handlers.jl index c0b7e7ff5..0602e272b 100644 --- a/test/handlers.jl +++ b/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 diff --git a/test/resources/TestRequest.jl b/test/resources/TestRequest.jl index 98f5e65b0..01caa71b8 100644 --- a/test/resources/TestRequest.jl +++ b/test/resources/TestRequest.jl @@ -55,3 +55,25 @@ end HTTP.@client (first=[testouterrequestlayer], last=[testinnerrequestlayer]) (first=[testouterstreamlayer], last=[testinnerstreamlayer]) end + +module ErrorRequest + +using HTTP + +function throwingrequestlayer(handler) + return function(req; request_exception=nothing, kw...) + !isnothing(request_exception) && throw(request_exception) + return handler(req; kw...) + end +end + +function throwingstreamlayer(handler) + return function(stream; stream_exception=nothing, kw...) + !isnothing(stream_exception) && throw(stream_exception) + return handler(stream; kw...) + end +end + +HTTP.@client (throwingrequestlayer,) (throwingstreamlayer,) + +end diff --git a/test/server.jl b/test/server.jl index 7b35396f9..af1a98741 100644 --- a/test/server.jl +++ b/test/server.jl @@ -191,17 +191,17 @@ const echostreamhandler = HTTP.streamhandler(echohandler) HTTP.startwrite(http) write(http, "response body\n") end - + port = HTTP.port(server) - + sock = connect(host, port) close(sock) - + r = HTTP.get("https://$(host):$(port)/"; readtimeout=30, require_ssl_verification = false) @test r.status == 200 close(server) - end + end end # @testset @testset "on_shutdown" begin @@ -222,7 +222,7 @@ end # @testset # First shutdown function errors, second adds 1 shutdown_throw() = throw(ErrorException("Broken")) server = HTTP.listen!(x -> nothing; listenany=true, on_shutdown=[shutdown_throw, shutdown_add]) - @test_logs (:error, r"shutdown function .* failed") close(server) + @test_logs (:error, r"shutdown function .* failed.*ERROR: Broken.*"s) close(server) @test TEST_COUNT[] == 4 end # @testset