Don't assume the only error in getObject is an ArgumentError (#47)

* Don't assume the only error in getObject is an ArgumentError

* Use consistent error messages

Author: Drvi

Diff for: 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 for: 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 for: 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 for: 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