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

Support HTTP.jl 1.0 #560

Merged
merged 9 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
6 changes: 1 addition & 5 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ jobs:
include:
# Add a job using the earliest version of Julia supported by this package
- os: ubuntu-latest
version: 1.3
arch: x64
# Add a 1.5 job because that's what Invenia actually uses
- os: ubuntu-latest
version: "1.5"
version: 1.6
arch: x64
env:
MINIO_ACCESS_KEY: minio
Expand Down
4 changes: 2 additions & 2 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ XMLDict = "228000da-037f-5747-90a9-8195ccbf91a5"
[compat]
Compat = "3.29"
GitHub = "5"
HTTP = "0.9.17"
HTTP = "1"
IniFile = "0.5"
JSON = "0.18, 0.19, 0.20, 0.21"
MbedTLS = "0.6, 0.7, 1"
Expand All @@ -33,7 +33,7 @@ OrderedCollections = "1.3"
StableRNGs = "1"
URIs = "1"
XMLDict = "0.3, 0.4"
julia = "1.3"
julia = "1.6"

[extras]
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
Expand Down
3 changes: 1 addition & 2 deletions bors.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
status = [
"Julia 1.3 - ubuntu-latest - x64",
"Julia 1.5 - ubuntu-latest - x64",
"Julia 1.6 - ubuntu-latest - x64",
"Julia 1 - macOS-latest - x64",
"Julia 1 - ubuntu-latest - x64",
]
Expand Down
3 changes: 1 addition & 2 deletions src/AWSExceptions.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module AWSExceptions

using HTTP
using HTTP.MessageRequest: body_was_streamed
using JSON
using XMLDict
using XMLDict: XMLDictElement
Expand Down Expand Up @@ -114,7 +113,7 @@ function AWSException(e::HTTP.StatusError, body::AbstractString)
message = get(info, "Message", message)
message = get(info, "message", message)

streamed_body = e.response.body == body_was_streamed ? body : nothing
streamed_body = !HTTP.isbytes(e.response.body) ? body : nothing

return AWSException(code, message, info, e, streamed_body)
end
Expand Down
6 changes: 5 additions & 1 deletion src/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ function legacy_response(

# When a user defined I/O stream is passed in use the actual `HTTP.Response` body
# instead of the `AWS.Response` body which requires the I/O stream to be seekable.
body = request.response_stream !== nothing ? response.response.body : response.body
body = if request.response_stream !== nothing
b"[Message Body was streamed]"
mattBrzezinski marked this conversation as resolved.
Show resolved Hide resolved
else
response.body
end

# The stored service name is always lowercase and may not match the module name
# specified by the user. We'll assume that the typical casing used is titlecase.
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/credentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ function _ec2_metadata(metadata_endpoint::String)

return request === nothing ? nothing : String(request.body)
catch e
e isa HTTP.IOError || e isa HTTP.StatusError && e.status == 404 || rethrow(e)
e isa HTTP.RequestError || e isa HTTP.StatusError && e.status == 404 || rethrow(e)
end

return nothing
Expand Down
6 changes: 1 addition & 5 deletions src/utilities/downloads_backend.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using HTTP.MessageRequest: body_was_streamed

"""
DownloadsBackend <: AWS.AbstractBackend

Expand Down Expand Up @@ -125,9 +123,7 @@ function _http_request(backend::DownloadsBackend, request::Request, response_str
end

function _http_response(req::Request, res::Downloads.Response; throw::Bool=true)
response = HTTP.Response(
res.status, res.headers; body=body_was_streamed, request=nothing
)
response = HTTP.Response(res.status, res.headers; body=IOBuffer(), request=nothing)

if throw && HTTP.iserror(response)
target = HTTP.resource(HTTP.URI(req.url))
Expand Down
18 changes: 4 additions & 14 deletions src/utilities/request.jl
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,6 @@ end
function _http_request(http_backend::HTTPBackend, request::Request, response_stream::IO)
http_options = merge(http_backend.http_options, request.http_options)

# HTTP options such as `status_exception` need to be used when creating the stack
http_stack = HTTP.stack(;
redirect=false, retry=false, aws_authorization=false, http_options...
quinnj marked this conversation as resolved.
Show resolved Hide resolved
)

local buffer
local response

Expand All @@ -219,12 +214,12 @@ function _http_request(http_backend::HTTPBackend, request::Request, response_str
buffer = Base.BufferStream()

response = @mock HTTP.request(
http_stack,
request.request_method,
HTTP.URI(request.url),
HTTP.mkheaders(request.headers),
request.content;
require_ssl_verification=false,
quinnj marked this conversation as resolved.
Show resolved Hide resolved
redirect=false,
retry=false,
response_stream=buffer,
http_options...,
)
Expand All @@ -235,13 +230,8 @@ function _http_request(http_backend::HTTPBackend, request::Request, response_str
end

check = function (s, e)
# `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
return isa(e, Sockets.DNSError) ||
isa(e, HTTP.ParseError) ||
isa(e, HTTP.IOError) ||
quinnj marked this conversation as resolved.
Show resolved Hide resolved
isa(e, Base.IOError) ||
return isa(e, HTTP.ConnectError) ||
isa(e, HTTP.RequestError) ||
(isa(e, HTTP.StatusError) && _http_status(e) >= 500)
end

Expand Down
6 changes: 3 additions & 3 deletions test/AWSExceptions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"status_code" => 400,
)

expected["body"] = HTTP.MessageRequest.body_was_streamed
expected["body"] = IOBuffer()
expected["streamed_body"] = """
<?xml version="1.0" encoding="UTF-8"?>
<$err>
Expand All @@ -50,7 +50,7 @@

@testset "XMLRequest - Invalid XML" begin
expected = Dict(
"body" => HTTP.MessageRequest.body_was_streamed,
"body" => IOBuffer(),
"streamed_body" => """<?xml version="1.0" encoding="UTF-8"?>InvalidXML""",
"headers" => ["Content-Type" => "application/xml"],
"status_code" => 404,
Expand All @@ -74,7 +74,7 @@
"status_code" => 400,
)

expected["body"] = HTTP.MessageRequest.body_was_streamed
expected["body"] = IOBuffer()
expected["streamed_body"] = """
{
"__type": "$(expected["code"])",
Expand Down
4 changes: 2 additions & 2 deletions test/issues.jl
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ try
attempt_num += 1
if attempt_num <= num_attempts_to_fail
write(response_stream, data[1:(n - 1)]) # an incomplete stream that shouldn't be retained
throw(HTTP.IOError(EOFError(), "msg"))
throw(HTTP.RequestError(HTTP.Request(), EOFError()))
else
write(response_stream, data)
return HTTP.Response(200, "{\"Location\": \"us-east-1\"}")
Expand Down Expand Up @@ -212,7 +212,7 @@ try

@testset "Fail all 4 attempts then throw" begin
err_t = if AWS.DEFAULT_BACKEND[] isa AWS.HTTPBackend
HTTP.IOError
HTTP.RequestError
else
Downloads.RequestError
end
Expand Down
5 changes: 3 additions & 2 deletions test/patch.jl
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ end
_http_options_patches = [
@patch function HTTP.request(args...; kwargs...)
options = Dict(kwargs)
delete!(options, :require_ssl_verification)
delete!(options, :redirect)
delete!(options, :retry)
delete!(options, :response_stream)
return options
end
Expand Down Expand Up @@ -171,7 +172,7 @@ function gen_http_options_400_patches(message)
if response_stream !== nothing
write(response_stream, body)
close(response_stream) # Simulating current HTTP.jl 0.9.14 behaviour
body = HTTP.MessageRequest.body_was_streamed
body = IOBuffer()
end

response = HTTP.Response(400, headers; body=body, request=request)
Expand Down
5 changes: 5 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ using StableRNGs

Mocking.activate()

# backwards compat
function HTTP.StatusError(status, resp)
return HTTP.StatusError(status, resp.request.method, resp.request.target, resp)
end

Copy link
Member

Choose a reason for hiding this comment

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

This is bad piracy; this actually gets called by submit_request as far as I can tell.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, forgot to clean this up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pushed a clean up

include("patch.jl")

const TEST_MINIO = begin
Expand Down