From ea3bd55601a73a4dcceebf7617e8c52114cb90c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Drvo=C5=A1t=C4=9Bp?= Date: Wed, 27 Sep 2023 11:26:56 +0200 Subject: [PATCH] Retry on TimeoutException, as we did in HTTP v1.9 --- src/clientlayers/RetryRequest.jl | 2 ++ test/client.jl | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/clientlayers/RetryRequest.jl b/src/clientlayers/RetryRequest.jl index 001c22128..8232573d5 100644 --- a/src/clientlayers/RetryRequest.jl +++ b/src/clientlayers/RetryRequest.jl @@ -1,5 +1,6 @@ module RetryRequest +import ConcurrentUtilities using Sockets, LoggingExtras, MbedTLS, OpenSSL, ExceptionUnwrapping using ..IOExtras, ..Messages, ..Strings, ..ExceptionRequest, ..Exceptions @@ -79,6 +80,7 @@ end 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::ConcurrentUtilities.TimeoutException) = true 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 diff --git a/test/client.jl b/test/client.jl index 868ac8ded..46f6f3cc6 100644 --- a/test/client.jl +++ b/test/client.jl @@ -692,6 +692,35 @@ end end end +@testset "Connection TimeoutException is retried" begin + # Since our request fails and we don't get to see the response, we + # add this layer just after the retry layer to capture the context + # of the request which we can use to test that we attempted the retries + function test_context_layer(handler) + return function(req; test_context::Dict, kw...) + merge!(test_context, req.context) + return handler(req; kw...) + end + end + + test_context = Dict{Symbol,Any}() + try + HTTP.pushlayer!(test_context_layer) + # 10.0.0.0 is non-routeable and will result in a connection timeout + HTTP.get("http://10.0.0.0", connect_timeout=1, retries=3, retry_delays=[0.1, 0.1, 0.1], test_context=test_context) + catch e + @assert e isa HTTP.ConnectError + @test e.error isa ConcurrentUtilities.TimeoutException + finally + HTTP.poplayer!() + end + + @test test_context[:retrylimitreached] + @test test_context[:retryattempt] == 3 + @test test_context[:connect_errors] == 3 +end + + @testset "Retry with ConnectError" begin mktemp() do path, io redirect_stdout(io) do @@ -717,6 +746,7 @@ end # isrecoverable tests @test !HTTP.RetryRequest.isrecoverable(nothing) + @test HTTP.RetryRequest.isrecoverable(ConcurrentUtilities.TimeoutException(1.0)) @test !HTTP.RetryRequest.isrecoverable(ErrorException("")) @test !HTTP.RetryRequest.isrecoverable(ArgumentError("yikes")) @test HTTP.RetryRequest.isrecoverable(ArgumentError("stream is closed or unusable"))