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

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

Open
asinghvi17 opened this issue Apr 15, 2024 · 9 comments
Open

Comments

@asinghvi17
Copy link

asinghvi17 commented Apr 15, 2024

The MIME string formats for JSON are hardcoded here:

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

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
Copy link
Author

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?

@TheCedarPrince
Copy link

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
Copy link

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

@asinghvi17
Copy link
Author

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

@pfitzseb
Copy link
Member

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 :)

@asinghvi17
Copy link
Author

asinghvi17 commented Apr 23, 2024

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 :)

@tanmaykm
Copy link
Member

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.

@pfitzseb
Copy link
Member

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

@tanmaykm
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants