JuliaServices / CloudStore.jl

A simple, consistent, and performant API for interacting with common cloud storage abstractions
Other
16 stars 8 forks source link

`301 Moved Permanently` are not handled and result in invalid content range error #26

Closed bachdavi closed 1 year ago

bachdavi commented 1 year ago

Somehow the 301 does not throw an exception in HTTP.jl even though status_exception is true. Downstream code assumes that the response has the Content-Range header set and fails to parse it.

julia> using CloudStore: S3

julia> S3.get("s3://real-bucket/fake-object.csv")
┌ Warning: `region` keyword argument not provided to `S3.get` and undetected from url.  Defaulting to `us-east-1`
└ @ CloudStore.S3 ~/.julia/dev/CloudStore/src/s3.jl:68
ERROR: invalid Content-Range: 
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] parseContentRange(str::String)
   @ CloudStore.API ~/.julia/dev/CloudStore/src/get.jl:41
 [3] getObjectImpl(x::CloudBase.AWS.Bucket, key::String, out::Nothing; multipartThreshold::Int64, partSize::Int64, batchSize::Int64, allowMultipart::Bool, decompress::Bool, headers::Vector{Pair{SubString{String}, SubString{String}}}, kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ CloudStore.API ~/.julia/dev/CloudStore/src/get.jl:84
 [4] getObjectImpl (repeats 2 times)
   @ ~/.julia/dev/CloudStore/src/get.jl:45 [inlined]
 [5] #get#7
   @ ~/.julia/dev/CloudStore/src/s3.jl:33 [inlined]
 [6] get
   @ ~/.julia/dev/CloudStore/src/s3.jl:33 [inlined]
 [7] get(::String; region::Nothing, nowarn::Bool, kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ CloudStore.S3 ~/.julia/dev/CloudStore/src/s3.jl:72
 [8] get(::String)
   @ CloudStore.S3 ~/.julia/dev/CloudStore/src/s3.jl:64
 [9] top-level scope
   @ REPL[17]:1
bachdavi commented 1 year ago

This is the response to the head request:

┌ Info: HTTP.Messages.Response:                                                                                                 
│ """                                                                                                                           
│ HTTP/1.1 301 Moved Permanently                                                                                                
│ x-amz-bucket-region: us-west-1                                                                                                
│ x-amz-request-id: PNPS5G4WAANC4VVM                                                                                            
│ x-amz-id-2: M7LtMXlTn2Ckkrze1BLk5fTUZgqEviTt6PXwQyHNjBasCmAw0OnYz8K55YPzzHl70PnyS76UqNw=                                      
│ Content-Type: application/xml                                                                                                 
│ Date: Tue, 10 Jan 2023 12:32:26 GMT                  
│ Server: AmazonS3  
│                                                               
└ """    
quinnj commented 1 year ago

Thanks for the report @bachdavi! I indeed totally forgot that 301 redirects don't get thrown as StatusErrors (I actually think they probably should, but that's a potentially breaking change for HTTP.jl). Here's a PR that should account more properly for this case: https://github.com/JuliaServices/CloudStore.jl/pull/27.