Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions .JuliaFormatter.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
style="blue"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lets me use "format" command in VSCode

2 changes: 0 additions & 2 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
MbedTLS = "739be429-bea8-5141-9913-cc70e7f3736d"
Mocking = "78c3b35d-d492-501b-9361-3d52fe80e533"
OrderedCollections = "bac558e1-5e72-5ebc-8fee-abe8a469f55d"
Retry = "20febd7b-183b-5ae2-ac4a-720e7ce64774"
Sockets = "6462fe0b-24de-5631-8697-dd941f90decc"
URIs = "5c2747f8-b7ea-4ff2-ba2e-563bfd36b1d4"
UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
Expand All @@ -30,7 +29,6 @@ JSON = "0.18, 0.19, 0.20, 0.21"
MbedTLS = "0.6, 0.7, 1"
Mocking = "0.7"
OrderedCollections = "1.3"
Retry = "0.3, 0.4"
URIs = "1"
XMLDict = "0.3, 0.4"
julia = "1.3"
Expand Down
1 change: 0 additions & 1 deletion src/AWS.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ using HTTP
using MbedTLS
using Mocking
using OrderedCollections: LittleDict, OrderedDict
using Retry
using Sockets
using URIs
using UUIDs: UUIDs
Expand Down
26 changes: 18 additions & 8 deletions src/utilities/downloads_backend.jl
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ function _http_request(backend::DownloadsBackend, request::Request, response_str

local buffer
local response
try
@repeat 4 try

get_response =
() -> begin
Comment thread
ericphanson marked this conversation as resolved.
Outdated
# Use a sacrificial I/O stream so that we only write the `response_stream` once
# even with multiple attempts.
buffer = Base.BufferStream()
Expand All @@ -100,13 +101,22 @@ function _http_request(backend::DownloadsBackend, request::Request, response_str
)

response = _http_response(request, r; throw=true)
catch e
@delay_retry if (
(isa(e, HTTP.StatusError) && AWS._http_status(e) >= 500) ||
isa(e, Downloads.RequestError)
)
end
# We'll rely on lexical scoping; `buffer` and `response`
# are bindings in the outer scope, so we don't need to return here.
return nothing
end

check =
(s, e) -> begin
Comment thread
ericphanson marked this conversation as resolved.
Outdated
return (isa(e, HTTP.StatusError) && AWS._http_status(e) >= 500) ||
isa(e, Downloads.RequestError)
end

get_response_with_retry = retry(
get_response; check=check, delays=Base.ExponentialBackOff(; n=3)
)
try
get_response_with_retry()
finally
close(buffer)

Expand Down
134 changes: 83 additions & 51 deletions src/utilities/request.jl
Original file line number Diff line number Diff line change
Expand Up @@ -116,57 +116,75 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers
request.headers["Host"] = HTTP.URI(request.url).host
stream = @something request.response_stream IOBuffer()

@repeat 3 try
credentials(aws) === nothing || sign!(aws, request)

aws_response = @mock _http_request(request.backend, request, stream)
response = aws_response.response

if response.status in REDIRECT_ERROR_CODES
if HTTP.header(response, "Location") != ""
request.url = HTTP.header(response, "Location")
else
e = HTTP.StatusError(response.status, response)
throw(AWSException(e, stream))
local aws_response
local response

get_response =
() -> begin
credentials(aws) === nothing || sign!(aws, request)

aws_response = @mock _http_request(request.backend, request, stream)
response = aws_response.response

if response.status in REDIRECT_ERROR_CODES
if HTTP.header(response, "Location") != ""
request.url = HTTP.header(response, "Location")
else
e = HTTP.StatusError(response.status, response)
throw(AWSException(e, stream))
end
end
end
Comment thread
ericphanson marked this conversation as resolved.
Outdated
catch e
if e isa HTTP.StatusError
e = AWSException(e, stream)
elseif !(e isa AWSException)
rethrow(e)
end

@retry if occursin("Signature expired", e.message)
function upgrade_error(f)
() -> try
return f()
catch e
if e isa HTTP.StatusError
e = AWSException(e, stream)
rethrow(e)
end
rethrow()
end
end
Comment thread
omus marked this conversation as resolved.

# Handle ExpiredToken...
# https://github.com/aws/aws-sdk-go/blob/v1.31.5/aws/request/retryer.go#L98
@retry if e isa AWSException && e.code in EXPIRED_ERROR_CODES
check_credentials(credentials(aws); force_refresh=true)
end
check =
(s, e) -> begin
Comment thread
ericphanson marked this conversation as resolved.
Outdated
occursin("Signature expired", e.message) && return true

# Throttle handling
# https://github.com/boto/botocore/blob/1.16.17/botocore/data/_retry.json
# https://docs.aws.amazon.com/general/latest/gr/api-retries.html
@delay_retry if _http_status(e.cause) == TOO_MANY_REQUESTS ||
e.code in THROTTLING_ERROR_CODES
end
# Handle ExpiredToken...
# https://github.com/aws/aws-sdk-go/blob/v1.31.5/aws/request/retryer.go#L98
if e isa AWSException && e.code in EXPIRED_ERROR_CODES
check_credentials(credentials(aws); force_refresh=true)
return true
end

# Handle BadDigest error and CRC32 check sum failure
@retry if _header(e.cause, "crc32body") == "x-amz-crc32" ||
e.code in ("BadDigest", "RequestTimeout", "RequestTimeoutException")
end
# Throttle handling
# https://github.com/boto/botocore/blob/1.16.17/botocore/data/_retry.json
# https://docs.aws.amazon.com/general/latest/gr/api-retries.html
if _http_status(e.cause) == TOO_MANY_REQUESTS ||
e.code in THROTTLING_ERROR_CODES
return true
end

if occursin("Missing Authentication Token", e.message) &&
aws.credentials === nothing
return throw(
NoCredentials(
"You're attempting to perform a request without credentials set."
),
)
# Handle BadDigest error and CRC32 check sum failure
if _header(e.cause, "crc32body") == "x-amz-crc32" ||
e.code in ("BadDigest", "RequestTimeout", "RequestTimeoutException")
return true
end

if occursin("Missing Authentication Token", e.message) &&
aws.credentials === nothing
return throw(
NoCredentials(
"You're attempting to perform a request without credentials set.",
),
)
end
return false
end
end

retry(upgrade_error(get_response); delays=Base.ExponentialBackOff(; n=2), check=check)()

if request.use_response_type
return aws_response
Expand All @@ -185,8 +203,9 @@ function _http_request(http_backend::HTTPBackend, request::Request, response_str

local buffer
local response
try
@repeat 4 try

get_response =
() -> begin
# Use a sacrificial I/O stream so that we only write to the `response_stream`
# once even with multiple attempted requests. Additionally this works around the
# HTTP.jl issue (https://github.com/JuliaWeb/HTTP.jl/issues/543) where the
Expand All @@ -206,17 +225,30 @@ function _http_request(http_backend::HTTPBackend, request::Request, response_str
response_stream=buffer,
http_options...,
)
catch e

# We'll rely on lexical scoping; `buffer` and `response`
# are bindings in the outer scope, so we don't need to return here.
return nothing
end

check =
(s, e) -> begin
Comment thread
ericphanson marked this conversation as resolved.
Outdated
# `Base.IOError` is needed because HTTP.jl can often have errors that aren't
# caught and wrapped in an `HTTP.IOError`
# https://github.com/JuliaWeb/HTTP.jl/issues/382
@delay_retry if isa(e, Sockets.DNSError) ||
isa(e, HTTP.ParseError) ||
isa(e, HTTP.IOError) ||
isa(e, Base.IOError) ||
(isa(e, HTTP.StatusError) && _http_status(e) >= 500)
end
return isa(e, Sockets.DNSError) ||
isa(e, HTTP.ParseError) ||
isa(e, HTTP.IOError) ||
isa(e, Base.IOError) ||
(isa(e, HTTP.StatusError) && _http_status(e) >= 500)
end

get_response_with_retry = retry(
get_response; check=check, delays=Base.ExponentialBackOff(; n=3)
)

try
get_response_with_retry()
finally
# We're unable to read from the `Base.BufferStream` until it has been closed.
# HTTP.jl will close passed in `response_stream` keyword. This ensures that it
Expand Down
1 change: 0 additions & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ using MbedTLS: digest, MD_SHA256, MD_MD5
using Mocking
using Pkg
using Random
using Retry
using Suppressor
using Test
using UUIDs
Expand Down