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

Invalid GraphQL requests must result in HTTP 400 responses #568

Closed
bclozel opened this issue Dec 7, 2022 · 4 comments
Closed

Invalid GraphQL requests must result in HTTP 400 responses #568

bclozel opened this issue Dec 7, 2022 · 4 comments
Assignees
Labels
in: web Issues related to web handling status: invalid An issue that we don't feel is valid

Comments

@bclozel
Copy link
Member

bclozel commented Dec 7, 2022

As explained in the GraphQL over HTTP spec:

If the GraphQL response does not contain the {data} entry then the server MUST reply with a 4xx or 5xx status code as appropriate.

Note: 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.

If the GraphQL request is invalid (e.g. it is malformed, or does not pass validation) then the server SHOULD reply with 400 status code.

Currently Spring for GraphQL responds with an HTTP 200 OK response in this case:

{
  "errors": [
    {
      "message": "Validation error (FieldUndefined@[spring]) : Field 'spring' in type 'Query' is undefined",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "extensions": {
        "classification": "ValidationError"
      }
    }
  ]
}
@bclozel bclozel added type: bug A general bug in: web Issues related to web handling for: team-attention An issue we need to discuss as a team to make progress labels Dec 7, 2022
@bclozel bclozel self-assigned this Dec 7, 2022
@erwinc1
Copy link

erwinc1 commented Dec 8, 2022

If you example response is the complete response you got, then "data" block is missing so this must (as opposed to your title) resolve in a 4xx or 5xx according to your citation, am I right?

@bclozel bclozel changed the title Invalid GraphQL requests should result in HTTP 400 responses Invalid GraphQL requests must result in HTTP 400 responses Dec 8, 2022
@bclozel
Copy link
Member Author

bclozel commented Dec 8, 2022

@erwinc1 Yes. I've updated the title.
We discussed this issue this morning and we are pausing it to get some clarification about this. Other issue comments seem to contradict this statement in the spec.

@bclozel
Copy link
Member Author

bclozel commented Feb 14, 2023

I'm closing this issue as this PR clarifies the fact that only requests that cannot be parsed should result in 4xx/5xx response status codes.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Feb 14, 2023
@bclozel bclozel added status: invalid An issue that we don't feel is valid and removed type: bug A general bug for: team-attention An issue we need to discuss as a team to make progress status: pending-design-work labels Feb 14, 2023
@benjie
Copy link

benjie commented Feb 14, 2023

Just to add "from the horse's mouth" to this:

If the GraphQL response does not contain the {data} entry then the server MUST reply with a 4xx or 5xx status code as appropriate.

This is still true for responses that are served using the application/graphql-response+json media type.

Note: all servers are expected to support the application/graphql-response+json media type by 1st Jan 2025, and clients should start adding it to their Accept headers now so that they can get the goodness of HTTP status codes if the server supports them, without the ambiguities inherent in using HTTP status codes with legacy GraphQL-over-HTTP.

only requests that cannot be parsed should result in 4xx/5xx response status codes.

This is true for responses using the application/json legacy media type only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues related to web handling status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants