Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry on TimeoutException, as we did in HTTP v1.9 #1114

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/clientlayers/RetryRequest.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module RetryRequest

import ConcurrentUtilities
using Sockets, LoggingExtras, MbedTLS, OpenSSL, ExceptionUnwrapping
using ..IOExtras, ..Messages, ..Strings, ..ExceptionRequest, ..Exceptions

Expand Down Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions test/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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://example.com:81", 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
Expand All @@ -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"))
Expand Down
Loading