Not able to get Spring GraphQL adhere to GraphQL Over Http Spec

[JayGhiya]

As per GraphQL over HTTP spec when using the application/graphql-response+json(https://graphql.github.io/graphql-over-http/draft/#sec-application-graphql-response-json) during a situation when the graphQL response does not contain the data entry then the server must reply with a 4xx or 5xx as appropriate.

Here is the poc : https://github.com/JayGhiya/Spring-Graphql-Experiments where we have a graphql query fetching data from sql db. During fetch if the database is unavailable then the operation itself has failed and we have to return 5xx. So The code uses a custom web interceptor (https://github.com/JayGhiya/Spring-Graphql-Experiments/blob/main/src/main/java/com/graphql/demo/CustomGraphQLWebInterceptor.java) to modify the response to remove data as it holds no significance. Still post modifying the response using webinterceptor the spring graphql http servlet returns 2xx which is wrong.

Post intercepting spring graphql servlet should have made 5xx response as there is no data field. Could anybody point me to a example of using spring graphql http servet to modify http status code as this results in a observability nightmare. As we are using HTTP as the transport protocol we would want to adhere to RFC7231 - HTTP semantics and content.

[bclozel]

Thanks but your sample project is not using spring graphql but graphql kickstart which is a different project. I'm closing this issue as a result.

[JayGhiya]

@bclozel i might have added those dependencies but using only spring graphql . i might have not pushed the latest pom. let me do that in some time. request you to open the issue post that.

[JayGhiya]

@bclozel i looked at the code the code makes use of only spring graphql. the pom is just old. will update.

[bclozel]

The sample is using and registering a "graphql.kickstart.servlet.GraphQLHttpServlet".

[JayGhiya]

yes that is correct. but the result is same . apologies for inconvenience. i will remove the graphql-java lib code completely and update the readme with result @bclozel :)

[JayGhiya]

@bclozel https://github.com/JayGhiya/Spring-Graphql-Experiments/blob/main/README.md . It is updated. Right now the project only uses spring-graphql only and readme is updated with steps to reproduce. please have a look and reopen the issue thanks in advance

[rstoyanchev]

I believe this is the same discussion, or at least related to #568 where we decided to hold off on making these changes for now.

[JayGhiya]

@rstoyanchev @bclozel sure even if due to some reasons the team has decided to hold off on making these changes but why does the custom web interceptor or even the controller advise with graphql exception handler not allow to modify http response codes. in a increasingly cloud native world api gateways/service meshes all of it get affected by this non compliance with http rfc when you have a failure in connecting to downstream services. Is there any other way to modify the http response through a custom config that you are aware of ?

[bclozel]

The current draft version of the spec says the following.

This response should have a 4xx or 5xx response status.

{
  "errors": [
    {
      "message": "Could not connect to database",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "books"
      ]
    }
  ]
}

This response should have a 200 response status.

{
  "data": null,
  "errors": [
    {
      "message": "Could not connect to database",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "books"
      ]
    }
  ]
}

As far as I understand the only difference is about when the error happen (before or during the request execution, and this doesn't seem really required by the original GraphQL spec).

In a increasingly cloud native world, HTTP is not the only transport out there and I think that the transport-agnostic nature of GraphQL is one of its strength. How should this behavior be mapped to RSocket, WebSocket, other messaging protocols and other transports? This is why so far we chose to decouple transports and the actual request execution. This allows us to support multiple transports and variants (Servlet-based, Reactive Streams based) without leaking abstractions.

As for the observability angle you've shared, I believe our observability story is quite compelling so far. Relying on HTTP statuses for that would be a mistake since the behavior you're sharing in the spec also depends on the response content-type (you would not get the same behavior if clients request the popular "application/json" media type).