graphql / graphql-over-http

Working draft of "GraphQL over HTTP" specification
https://graphql.github.io/graphql-over-http
MIT License
385 stars 59 forks source link

Response body for non-well-formed GraphQL-over-HTTP requests #293

Open atrigent opened 3 months ago

atrigent commented 3 months ago

Unless I missed it, this specification doesn't specify the contents of the response body when a non-well-formed GraphQL-over-HTTP request is received, only specifying the response status codes to use. I can't even find anything in the GitHub issues about it. It seems to take the opinion that it is not possible to create a well-formed response in this case. In practice implementers will all decide on different ways to respond, which seems counter to the intent of creating this spec in the first place. Since the new application/graphql-response+json media type is not fully defined anywhere, the response body is seemingly not even required to be valid json, despite the name seemingly implying that, making it very unclear how these responses should be handled. In fact I think it can be argued that this spec is even MORE unclear than not having any spec at all. In the "legacy" world, where application/json was used as the response media type, I could see implementers making the decision themselves to ensure that all responses are at least valid JSON. However, now that this spec exists, I could imagine some implementers reading it and interpreting it to mean that these responses should not have any body at all, which is really unnecessarily awkward to handle on the client side and provides no clarity on what error occurred.

Can anyone explain why this spec doesn't define this behavior? Was it considered but rejected? Was it never considered at all? If so, why? I'm really having a hard time understanding why you wouldn't use the chance you had to define everything you possibly could to provide much needed clarity to implementers of both clients and servers.

benjie commented 3 months ago

this specification doesn't specify the contents of the response body when a non-well-formed GraphQL-over-HTTP request is received

Indeed, that is by design. If the request is not a well formed GraphQL over HTTP request then the server may respond to it however it wants. This is leveraged by #264 for example: since a persisted operation request is not a well-formed GraphQL-over-HTTP request, we may apply our own (additional) logic to the request. It can also be leveraged to return e.g. a GraphiQL interface when a plain HTTP GET request is made.

Roughly speaking, if the request doesn't "smell like" a GraphQL request, then this spec doesn't cover it (even if it's a request made to your /graphql endpoint), leaving you free to reply however you deem appropriate.

It seems to take the opinion that it is not possible to create a well-formed response in this case.

No, it simply places no requirements on such as response since it is outside of the scope of the spec.

In practice implementers will all decide on different ways to respond, which seems counter to the intent of creating this spec in the first place.

It is not; this spec only covers well-formed GraphQL-over-HTTP requests. Note that the GraphQL document itself does not have to be valid, but the query property must be present and it must be of type string (see spec for further requirements).

Since the new application/graphql-response+json media type is not fully defined anywhere, the response body is seemingly not even required to be valid json, despite the name seemingly implying that, making it very unclear how these responses should be handled.

This media type is defined under Media Types to be the media type for a GraphQL response, but it could indeed do with further definition.

Note that you can reply to a request that is not a well-formed GraphQL-over-HTTP request with application/graphql-response+json; this must still be a JSON representation of the GraphQL response as specified by the linked GraphQL response text, but this allows you to accept GraphQL queries in other non-standard formats and still respond in a standard way - for example, #264 leverages this to effectively "upgrade" a persisted operation request to well-forked GraphQL-over-HTTP request.

benjie commented 3 months ago

Oh, I missed one thing:

the response body is seemingly not even required to be valid json, despite the name seemingly implying that

This is covered by RFC6839: https://datatracker.ietf.org/doc/html/rfc6839#section-3.1

[RFC4627] defines the "application/json" media type. The suffix "+json" MAY be used with any media type whose representation follows that established for "application/json".