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/test/client.jl b/test/client.jl index f886db705..e9c60e7c3 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 = [1, 2, 3], retries=3) # ~ 6 secs + max_wait = 3 + + 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/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