graphql-over-http icon indicating copy to clipboard operation
graphql-over-http copied to clipboard

Server can choose the media type on Accept fail

Open benjie opened this issue 1 year ago • 3 comments

If the client issues Accept: application/graphql-response+json, application/json and the server does not support this for the request (e.g. the request uses @stream/@defer so needs a multipart response) then the server should be able to choose to respond with another suitable media type, for example multipart/mixed;type=application/graphql-response+json. Thanks to @michaelstaib for pointing out this oversight.

(Note: v1 doesn't explicitly specify @stream/@defer or subscription related concerns, but we should still be future proof where possible.)

benjie avatar Sep 06 '22 15:09 benjie

I thought we want the exact opposite? ~> https://github.com/robrichard/defer-stream-wg/discussions/48

If the client cannot support a multipart response, and the accept header reflects this — then dont send it. For example using an older version of graphiql that doesn't support defer stream, but the server its targeting does.

maraisr avatar Sep 07 '22 06:09 maraisr

@maraisr that is exactly how I feel :D I would like it if we made it strict, so that if you send in the accept header that is what the server abides by. This would give predictability. If the server cannot meet the requested accept header it would be great if it would fail.

But I think this is separate from this PR, this PR just refines already existing spec text and we can have a discussion on this in the next working group meeting or create another PR that discusses this.

michaelstaib avatar Sep 07 '22 07:09 michaelstaib

Yeah, this is just meant to be a fix; the major change in behaviour needs more discussion 👍

benjie avatar Sep 07 '22 08:09 benjie

The "Accept" header is specified such that:

If the header field is present in a request and none of the available representations for the response have a media type that is listed as acceptable, the origin server can either honor the header field by sending a 406 (Not Acceptable) response or disregard the header field by treating the response as if it is not subject to content negotiation.

In the text currently we extend this same wording effectively to our server implementors; since GraphQL-over-HTTP's purpose is to map more natively to HTTP semantics.

Having pondered this for a while now (!) I definitely see the value in honouring the Accept header and doing a 406 Not Acceptable; but this also severely limits the choices of servers - forcing them to reject the request otherwise they are not spec compliant. The client can (and should!) still check the Content-Type header on the response (since legacy or spec non-compliant servers may send the wrong Content-Type, as may intermediary servers such as proxies), so I don't see that allowing the server to accept the response and return another format of GraphQL is a dealbreaker.

I have a counter-proposal; we can say that the server SHOULD return a 406, but MAY ignore the header. This puts weight on the 406 approach, but does not forbid the "accept it anyway" approach.

What are your thoughts on this @michaelstaib and @maraisr?

benjie avatar Oct 03 '23 07:10 benjie

My thoughts:

  • There are multiple methods to stream responses, such as the graphql-ws protocol or multipart/mixed
  • I personally would expect that by default every GraphQL server responds in a basic (json) format, and does not prefer one streaming format over another -- although currently the spec states this:

If a client does not supply the Accept header then the server may respond with an error, or with any content type it chooses

  • Existing text would force a compliant server that receives a @defer or @stream request to return an application/json format. Seems that it should be allowed to return any format, such as would be the case if there were no Accept header.
  • HOWEVER, the written text only applies when the client specifically requests an unsupported format. As such, there should be no expectation that the client can interpret the response at all. (Otherwise it would have specified the format in the Accept header.)
  • In the sample at the top of this PR, it is quite unlikely that the client is going to be able to parse a multipart response. If it were able to do so, it would/should have been specified in the request. Responding with 406 would seem appropriate.

In conclusion, I agree with @benjie 's latest suggestion, that the spec should state that the server SHOULD return 406 (when the client requests an unsupported format), but MAY return any other format - both to mimic the HTTP spec and match the behavior when the Accept header is not specified.

Shane32 avatar Oct 03 '23 12:10 Shane32

I've updated the text to reflect this.

benjie avatar Oct 03 '23 13:10 benjie