diff --git a/Project.toml b/Project.toml index b74c5a3..f77e4a7 100644 --- a/Project.toml +++ b/Project.toml @@ -8,6 +8,7 @@ Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f" CloudBase = "85eb1798-d7c4-4918-bb13-c944d38e27ed" CodecZlib = "944b1d66-785c-5afd-91f1-9de20f533193" CodecZlibNG = "642d12eb-acb5-4437-bcfc-a25e07ad685c" +ExceptionUnwrapping = "460bff9d-24e4-43bc-9d9f-a8973cb893f4" HTTP = "cd3eb016-35fb-5094-929b-558a96fad6f3" Mmap = "a63ad114-7e13-5084-954f-fe012c677804" TranscodingStreams = "3bb67fe8-82b1-5028-8e26-92a6c54297fa" @@ -19,16 +20,19 @@ Base64 = "1" CloudBase = "1" CodecZlib = "0.7" CodecZlibNG = "0.1" +ExceptionUnwrapping = "0.1" HTTP = "1.7" Mmap = "1" TranscodingStreams = "0.9.12" +Sockets = "1" WorkerUtilities = "1.1" XMLDict = "0.4" julia = "1.6" [extras] CodecZlib = "944b1d66-785c-5afd-91f1-9de20f533193" +Sockets = "6462fe0b-24de-5631-8697-dd941f90decc" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [targets] -test = ["CodecZlib", "Test"] +test = ["CodecZlib", "Sockets", "Test"] diff --git a/src/CloudStore.jl b/src/CloudStore.jl index 41d8907..62f587d 100644 --- a/src/CloudStore.jl +++ b/src/CloudStore.jl @@ -12,6 +12,7 @@ export Object, PrefetchedDownloadStream, ResponseBodyType, RequestBodyType, using HTTP, CodecZlib, CodecZlibNG, Mmap import WorkerUtilities: OrderedSynchronizer import CloudBase: AbstractStore +using ExceptionUnwrapping """ Controls the automatic use of concurrency when downloading/uploading. diff --git a/src/get.jl b/src/get.jl index 58b3980..aeed24c 100644 --- a/src/get.jl +++ b/src/get.jl @@ -73,6 +73,28 @@ function Base.getindex(b::BufferBatch, i::Int) end end +# For smaller object, we don't do a multipart download, but instead just do a single GET request. +# This changes the exception we get when the provided buffer is too small, as for the multipart +# case, we do a HEAD request first to know the size of the object, which gives us the opportunity +# to throw an ArgumentError. But for the single GET case, we don't know the size of the object +# until we get the response, which would return as a HTTP.RequestError from within HTTP.jl. +# The idea here is to unwrap the HTTP.RequestError and check if it's an ArgumentError, and if so, +# throw that instead, so we same exception type is thrown in this case. +function _check_buffer_too_small_exception(@nospecialize(e::Exception)) + if e isa HTTP.RequestError + request_error = e.error + if request_error isa CompositeException + length(request_error.exceptions) == 1 || return e + request_error = request_error.exceptions[1] + end + request_error = unwrap_exception(request_error) + if request_error isa ArgumentError + return request_error + end + end + return e +end + function getObjectImpl(x::AbstractStore, key::String, out::ResponseBodyType=nothing; multipartThreshold::Int=MULTIPART_THRESHOLD, partSize::Int=MULTIPART_SIZE, @@ -124,8 +146,9 @@ function getObjectImpl(x::AbstractStore, key::String, out::ResponseBodyType=noth elseif out isa AbstractVector{UInt8} resp = try getObject(x, url, headers; response_stream=out, kw...) - catch - throw(ArgumentError("provided output buffer (length = $(length(out))) is too small for actual cloud object size")) + catch e + e = _check_buffer_too_small_exception(e) + rethrow(e) end elseif out isa String if decompress @@ -172,7 +195,8 @@ function getObjectImpl(x::AbstractStore, key::String, out::ResponseBodyType=noth res = body = Vector{UInt8}(undef, contentLength) elseif out isa AbstractVector{UInt8} # user-provided buffer is allowed to be larger than actual object size, but not smaller - length(out) < contentLength && throw(ArgumentError("out ($(length(out))) must at least be of length $contentLength")) + # NOTE: wording of the error message matches what HTTP.jl throws when the buffer is too small + length(out) < contentLength && throw(ArgumentError("Unable to grow response stream IOBuffer $(length(out)) large enough for response body size: $(contentLength)")) res = out body = view(out, 1:contentLength) elseif out isa String diff --git a/test/runtests.jl b/test/runtests.jl index b8712f5..af43f21 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,6 +1,9 @@ using Test, CloudStore, CloudBase.CloudTest import CloudStore: S3, Blobs using CodecZlib +using HTTP: ConnectError, StatusError +using Sockets: DNSError +using ExceptionUnwrapping: unwrap_exception bytes(x) = codeunits(x) @@ -134,6 +137,119 @@ check(x, y) = begin; reset!(x); reset!(y); z = read(x) == read(y); reset!(x); re end end end + + @testset "Exceptions" begin + # conf, p = Minio.run(; debug=true) + Minio.with(; debug=true) do conf + credentials, bucket = conf + global _stale_bucket = bucket + csv = "a,b,c\n1,2,3\n4,5" + obj = S3.put(bucket, "test.csv", bytes(csv); credentials) + @assert obj.size == sizeof(csv) + + @testset "Insufficient output buffer size" begin + out = zeros(UInt8, sizeof(csv) - 1) + try + S3.get(bucket, "test.csv", out; credentials, allowMultipart=false) # single request + @test false # Should have thrown an error + catch e + @test e isa ArgumentError + @test e.msg == "Unable to grow response stream IOBuffer $(sizeof(out)) large enough for response body size: $(sizeof(csv))" + end + + try + S3.get(bucket, "test.csv", out; credentials, allowMultipart=true, multipartThreshold=1) # multipart request + @test false # Should have thrown an error + catch e + @test e isa ArgumentError + @test e.msg == "Unable to grow response stream IOBuffer $(sizeof(out)) large enough for response body size: $(sizeof(csv))" + end + end + + @testset "Missing credentials" begin + try + S3.get(bucket, "test.csv") # single request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 403 + end + + try + S3.get(bucket, "test.csv"; allowMultipart=true, multipartThreshold=1) # multipart request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 403 + end + + try + S3.put(bucket, "test2.csv", csv) + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 403 + end + end + + @testset "Non-existing file" begin + try + S3.get(bucket, "doesnt_exist.csv"; credentials, allowMultipart=false) # single request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 404 + end + + try + S3.get(bucket, "doesnt_exist.csv"; credentials, allowMultipart=true, multipartThreshold=1) # multipart request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 404 + end + end + + @testset "Connection error: DNSError" begin + non_existent_bucket_name = string(bucket.name, "doesntexist") + non_existent_baseurl = replace(bucket.baseurl, bucket.name => non_existent_bucket_name) + non_existent_bucket = S3.Bucket(non_existent_bucket_name, non_existent_baseurl) + try + S3.get(non_existent_bucket, "doesnt_exist.csv"; credentials) + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa DNSError + end + + try + S3.put(non_existent_bucket, "doesnt_exist.csv", csv; credentials) + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa DNSError + end + end + end + # Minio doesn't run at this point + @testset "Connection error: IOError" begin + try + S3.get(_stale_bucket, "doesnt_exist.csv") + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa Base.IOError + end + + try + S3.put(_stale_bucket, "doesnt_exist.csv", "my,da,ta") + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa Base.IOError + end + end + end end @time @testset "Blobs" begin @@ -235,6 +351,119 @@ end end end end + + @testset "Exceptions" begin + # conf, p = Azurite.run(; debug=true) + Azurite.with(; debug=true) do conf + credentials, container = conf + global _stale_container = container + csv = "a,b,c\n1,2,3\n4,5" + obj = Blobs.put(container, "test.csv", bytes(csv); credentials) + @assert obj.size == sizeof(csv) + + @testset "Insufficient output buffer size" begin + out = zeros(UInt8, sizeof(csv) - 1) + try + Blobs.get(container, "test.csv", out; credentials, allowMultipart=false) # single request + @test false # Should have thrown an error + catch e + @test e isa ArgumentError + @test e.msg == "Unable to grow response stream IOBuffer $(sizeof(out)) large enough for response body size: $(sizeof(csv))" + end + + try + Blobs.get(container, "test.csv", out; credentials, allowMultipart=true, multipartThreshold=1) # multipart request + @test false # Should have thrown an error + catch e + @test e isa ArgumentError + @test e.msg == "Unable to grow response stream IOBuffer $(sizeof(out)) large enough for response body size: $(sizeof(csv))" + end + end + + @testset "Missing credentials" begin + try + Blobs.get(container, "test.csv") # single request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 403 + end + + try + Blobs.get(container, "test.csv"; allowMultipart=true, multipartThreshold=1) # multipart request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 403 + end + + try + Blobs.put(container, "test2.csv", csv) + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 403 + end + end + + @testset "Non-existing file" begin + try + Blobs.get(container, "doesnt_exist.csv"; credentials, allowMultipart=false) # single request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 404 + end + + try + Blobs.get(container, "doesnt_exist.csv"; credentials, allowMultipart=true, multipartThreshold=1) # multipart request + @test false # Should have thrown an error + catch e + @test e isa StatusError + @test e.status == 404 + end + end + + @testset "Connection error: DNSError" begin + non_existent_container_name = string(container.name, "doesntexist") + non_existent_baseurl = replace(container.baseurl, container.name => non_existent_container_name) + non_existent_container = Blobs.Container(non_existent_container_name, non_existent_baseurl) + try + Blobs.get(non_existent_container, "doesnt_exist.csv"; credentials) + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa DNSError + end + + try + Blobs.put(non_existent_container, "doesnt_exist.csv", csv; credentials) + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa DNSError + end + end + end + # Azurite is not running at this point + @testset "Connection error: IOError" begin + try + Blobs.get(_stale_container, "doesnt_exist.csv") + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa Base.IOError + end + + try + Blobs.put(_stale_container, "doesnt_exist.csv", "my,da,ta") + @test false # Should have thrown an error + catch e + @test e isa ConnectError + @test unwrap_exception(e.error) isa Base.IOError + end + end + end end @testset "URL Parsing Unit Tests" begin