OpenAPI.jl icon indicating copy to clipboard operation
OpenAPI.jl copied to clipboard

JSON is not parsed in responses for APIs that set custom mime types

Open asinghvi17 opened this issue 1 year ago • 9 comments

The MIME string formats for JSON are hardcoded here:

https://github.com/JuliaComputing/OpenAPI.jl/blob/6c820c5b0c5e235a6a1468ee0ab6bf237aa16d6c/src/client.jl#L261

but some servers like Phylopic respond in their own application-specific MIME types that are nevertheless JSON. There needs to be some form of switch to control which MIME type is accepted for JSON parsing. Otherwise, OpenAPI calls fail with inscrutable errors like:

julia> APIClient.get_image(APIClient.ImagesApi(c), "abb918ae-6d3d-4dd3-a6eb-e5457199f25f")
ERROR: MethodError: Cannot `convert` an object of type Vector{UInt8} to an object of type Main.APIClient.ImageWithEmbedded

Closest candidates are:
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:84
  convert(::Type{T}, ::Dict{String, Any}) where T<:OpenAPI.APIModel
   @ OpenAPI ~/.julia/packages/OpenAPI/GMeGW/src/client.jl:715
  convert(::Type{T}, ::Nothing) where T<:OpenAPI.APIModel
   @ OpenAPI ~/.julia/packages/OpenAPI/GMeGW/src/client.jl:716
  ...

Stacktrace:
 [1] response(::Type{Main.APIClient.ImageWithEmbedded}, data::Vector{UInt8})
   @ OpenAPI.Clients ~/.julia/packages/OpenAPI/GMeGW/src/client.jl:427
 [2] response(::Type{Main.APIClient.ImageWithEmbedded}, is_json::Bool, body::Vector{UInt8})
   @ OpenAPI.Clients ~/.julia/packages/OpenAPI/GMeGW/src/client.jl:417
 [3] response(::Type{Main.APIClient.ImageWithEmbedded}, resp::Downloads.Response, body::Vector{UInt8})
   @ OpenAPI.Clients ~/.julia/packages/OpenAPI/GMeGW/src/client.jl:412
 [4] exec(ctx::OpenAPI.Clients.Ctx, stream_to::Nothing)
   @ OpenAPI.Clients ~/.julia/packages/OpenAPI/GMeGW/src/client.jl:634
 [5] exec
   @ ~/.julia/packages/OpenAPI/GMeGW/src/client.jl:614 [inlined]
 [6] #get_image#186
   @ ~/.julia/dev/SpeciesDistributionToolkit/Phylopic/gen/src/apis/api_ImagesApi.jl:56 [inlined]
 [7] get_image(_api::Main.APIClient.ImagesApi, uuid::String)
   @ Main.APIClient ~/.julia/dev/SpeciesDistributionToolkit/Phylopic/gen/src/apis/api_ImagesApi.jl:54

cc @thecedarprince who is possibly having the same issue.

asinghvi17 avatar Apr 15 '24 00:04 asinghvi17

A quick monkeypatch for my current issue is:

@eval OpenApi.Clients is_json_mime(mime::T) where {T <: AbstractString} = ("*/*" == mime) || occursin(r"(?i)application\/json(;.*)?", mime) || occursin(r"(?i)application\/(.*)-patch\+json(;.*)?", mime) || occursin(r"(?i)application\/vnd.(.*)\+json(;.*)?", mime)

Can we look for occursin("json", mime) instead?

asinghvi17 avatar Apr 15 '24 00:04 asinghvi17

Fascinating! I had no idea this was the failure mode as yes, these errors are inscrutable and I still do not know how to fix them. My terrible fix is to basically pirate myself in the package I am working on here:

https://github.com/JuliaHealth/IPUMS.jl/blob/tcp-extracts/src/piracy.jl

Meaning, I am overwriting the calls that OpenAPI should've generated correctly for me using my OpenAPI yaml. Not sure really what to do from here... Thanks @asinghvi17 for CC'ing me! Still don't know how to fix this, but I am curious to learn more.

TheCedarPrince avatar Apr 15 '24 01:04 TheCedarPrince

Turns out after some sleuthing @asinghvi17 and I discovered these are two different issues -- opened an issue here: #73

TheCedarPrince avatar Apr 15 '24 02:04 TheCedarPrince

Just wanted to bump this issue - what's the best way forward here?

asinghvi17 avatar Apr 19 '24 15:04 asinghvi17

Imo we should be looking for the +json suffix specifically (doesn't really matter what the actual type or subtype are, but it seems sensible to restrict those to text and application or maybe only the latter).

A PR would be appreciated, of course :)

pfitzseb avatar Apr 22 '24 16:04 pfitzseb

possibly startswith("application", string) && endswith("json", string), or maybe even startswith("application", string) && occursin("json", string)? I can whip up a quick PR once we settle on what the criterion should be :)

asinghvi17 avatar Apr 23 '24 00:04 asinghvi17

Imo we should be looking for the +json suffix specifically (doesn't really matter what the actual type or subtype are, but it seems sensible to restrict those to text and application or maybe only the latter).

I agree, we could expand the accepted mime types to include those. The application/vnd.api+json is usually used in the JSON API protocol. So I think we could accept r"(?i)text\/json(;.*)?" and r"(?i)application\/vnd.(.*)\+json(;.*)? in addition to what we do now.

tanmaykm avatar Apr 24 '24 08:04 tanmaykm

Technically any mime with a +json suffix is guaranteed (for some definition of guaranteed) to be parsable as json.

pfitzseb avatar Apr 24 '24 11:04 pfitzseb

Technically any mime with a +json suffix is guaranteed (for some definition of guaranteed) to be parsable as json.

True. RFC6838 is very generic. IANA mime type list can be a good reference point.

tanmaykm avatar Apr 24 '24 13:04 tanmaykm