Support / adoption of GraphQL over HTTP spec 2.0 (e.g. non 2xx-Status Code for errors with `application/graphql-response+json`)

[frittentheke]

This is somewhat a mix of feature request and discussion ....

I was wondering about your take and support of the upcoming 2.0 revision of the GraphQL over HTTP spec , especially in regards to status codes.

The appropriate section about the application/graphql-response+json response type (https://graphql.github.io/graphql-over-http/draft/#sec-application-graphql-response-json) states:

If the GraphQL response does not contain the data entry then the server MUST reply with a 4xx or 5xx status code as appropriate.
The GraphQL specification indicates that the only situation in which the GraphQL response does not include the data entry is one in which the errors entry is populated.

Currently it reads as if there is no support / adoption of non-2xx responses (https://github.com/spring-projects/spring-graphql/blob/36bfd40d66494987931d68142a0e26b6f7d156c2/spring-graphql-docs/modules/ROOT/pages/transports.adoc#http) yet and one would need to implement this by overriding handlers or other custom code?

[bclozel]

This was discussed in #568 and #783. I don't think things have changed much since.

[frittentheke]

This was discussed in #568 and #783. I don't think things have changed much since.

Thanks for the pointers and sorry about obviously not having looked hard enough through older issues.
I do believe things did change somewhat. At the very least more than a year has passed and the new revision of the GraphQL over HTTP spec is out there, proposing changes in regards to HTTP status usage to allow for better interoperability with e.g. observability tooling.

Sorry again for creating a duplicate issue @bclozel . Maybe it's best to revive #783 and continue the already active discussion there?

[bclozel]

@frittentheke no worries, thanks for raising this. I'll discuss this again with Rossen.

I think the main problem here is that this particular bit is in my opinion only covering edge cases and won't solve the observability problem. GraphQL uses HTTP/RSocket/WebSocket/others as transports and unlike REST HTTP, the semantics are simply not present at the protocol level. Have you considered using Spring for GraphQL's observability feature? Maybe something is missing to better support GraphQL apps in production?

[frittentheke]

I'll let others fill in their use-cases. But in case of GraphQL federation there are multiple GraphQL subgaphs, potentially implemented in different languages and also using different frameworks and libraries. As @oliverjumpertz and @JayGhiya mentioned in #783 it's about allowing traffic handling components such as service meshes to observe the request success or failure rate on a more technical level. So this is not about GraphQL queries returning an empty result set (data), but those that are only sending errors back to the client.

In case of federation, e.g. via Apollo Router, there also is a difference between the federated traffic (towards subgraphs) and that to and from the actual clients.

[bclozel]

Thanks for your feedback.

But in case of GraphQL federation there are multiple GraphQL subgaphs, potentially implemented in different languages and also using different frameworks and libraries.

So this means that this is a "fallback observability" solution if the graphql library doesn't support metrics/traces in the first place?

... to observe the request success or failure rate on a more technical level. So this is not about GraphQL queries returning an empty result set (data), but those that are only sending errors back to the client.

I'm not sure I understand. By "more technical level" you mean HTTP transport level, even if the information is really incomplete as it's missing most errors? GraphQL are often using multiple data sources, so one data source can be consistently failing and others won't. The GraphQL API will be consistently broken but the monitoring solution won't know about it.

In case of federation, e.g. via Apollo Router, there also is a difference between the federated traffic (towards subgraphs) and that to and from the actual clients.

Does this mean that federation routers do react differently in case they see those 4xx/5xx response statuses? Are they taking the instance out of the pool, for example?

[oliverjumpertz]

Hey @bclozel :)

So this means that this is a "fallback observability" solution if the graphql library doesn't support metrics/traces in the first place?

This, and a solution in case a team does not properly handle observability themselves (it happens. More often than I'd like to admit).

I'm not sure I understand. By "more technical level" you mean HTTP transport level, even if the information is really incomplete as it's missing most errors? GraphQL are often using multiple data sources, so one data source can be consistently failing and others won't. The GraphQL API will be consistently broken but the monitoring solution won't know about it.

Yes, this is why I'd ask for more flexibility in the API, and not for a one-size-fits-all solution. :)

Does this mean that federation routers do react differently in case they see those 4xx/5xx response statuses? Are they taking the instance out of the pool, for example?

In the end, the Router does not react differently. It's all about fallback-observability in our case, and additionally about configured service mesh functionality like circuit breaking, and co.

[frittentheke]

But in case of GraphQL federation there are multiple GraphQL subgaphs, potentially implemented in different languages and also using different frameworks and libraries.

So this means that this is a "fallback observability" solution if the graphql library doesn't support metrics/traces in the first place?

More "complementary" to the GraphQL error space with its still existing and transported error field ... just in case there is not even any partial data. And if no data can be returned due to an error, the spec in its current form calls for non 2xx status code, thus this very issue starting the discussion around supporting it without custom HTTP handlers and the like.
If I might also point to the examples right in the spec at https://graphql.github.io/graphql-over-http/draft/#sec-application-graphql-response-json.Examples

There are also use-cases like rate-limiting (returning e.g. HTTP 429 to the client).

[rstoyanchev]

Thanks for your attention on this, but I think the assumption here is incorrect. The GraphQL over HTTP spec expects a non-200 status for the case where there is no "data" key. It is essential to understand what this means in the GraphQL spec.

Section 7.1 Response Fromat:

If the request included execution, the response map must contain an entry with key data.

Section 7.1.1 Data:

If an error was raised before execution begins, the data entry should not be present in the response.

In other words, non-200 status codes are specifically for requests that are not well-formed, and could not even be passed to the GraphQL engine, and there is a note the appropriate status code will be 400 (Bad Request) typically, but could also be a 406 (content negotiation), 401 (security), etc. Those are transport layer issues, and it is already what our transport handlers do.

Our documentation could be a bit more clear in reference to this, but it does qualify "once successfully decoded":

Once the JSON body has been successfully decoded, the HTTP response status is always 200 (OK), and any errors from GraphQL request execution appear in the "errors" section of the GraphQL response.

If a request is well-formed and passed to the GraphQL engine, it should have a "data" key, but that may be null if it fails with a validation error for example. This is also a request failure but one that happens during execution and for which there will be a GraphQL response with "errors". This is expected to be a 200 response.

In short, I believe we are already aligned with the spec. However, the status section of the GraphQL over HTTP spec contains a number of examples, and we are going to go over those to double check things, but on a first read, I expect those to work as described. Feel free to do your own checks, and let us know.

[bclozel]

Rossen and I discussed this earlier today and we are in the process of identifying existing gaps with the specification.
I'm closing this issue in favor of #1117 where we have a concrete plan for 1.4.0.