-
Notifications
You must be signed in to change notification settings - Fork 63
Description
While the GraphQL specification does not specify the transport layer, it seems like the de-facto standard originated in the article Serving over HTTP, which states that in most cases, 2xx status code should be used:
If a response contains a data key and its value is not null, then the server should respond with a 2xx status code for either the application/graphql-response+json or application/json media types. This is the case even if the response includes errors, since HTTP does not yet have a status code representing “partial success.” (see Status Codes)
To complicate things, 4xx and 5xx are allowed, although discouraged:
For validation errors that prevent the execution of a GraphQL operation, the server will typically send a 400 status code, although some legacy servers may return a 2xx status code when the application/json media type is used.
For legacy servers that respond with the application/json media type, the use of 4xx and 5xx status codes for valid GraphQL requests that fail to execute is discouraged but may be used depending on the implementation. Because of potential ambiguities regarding the source of the server-side error, a 2xx code should be used with this media type instead.
However, for responses with the application/graphql-response+json media type, the server will reply with a 4xx or 5xx status code if a valid GraphQL request fails to execute.
The GraphQL-over-HTTP spec draft, states more clearly that 200 should be used, see Status Codes. Here are a few examples from that draft:
Examples
The following examples provide guidance on how to deal with specific error cases when using the application/json media type to encode the response body:JSON parsing failure
For example a POST request body of NONSENSE or {"query": (note: invalid JSON).Requests that the server cannot interpret SHOULD result in status code 400 (Bad Request).
Invalid parameters
For example a POST request body of {"qeury": "{__typename}"} (note: typo) or {"query": "query Q ($i:Int!) { q(i: $i) }", "variables": [7]} (note: invalid shape for variables).A request that does not constitute a well-formed GraphQL-over-HTTP request SHOULD result in status code 400 (Bad Request).
Document parsing failure
For example a POST request body of {"query": "{"}.Requests where the GraphQL document cannot be parsed SHOULD result in status code 200 (Okay).
Document validation failure
Requests that fail to pass GraphQL validation, the server SHOULD NOT execute the request and SHOULD return a status code of 200 (Okay).
EntityGraphQL returns 400 Bad Request in case of an error. i.e., not (only?) in case of parsing failure of invalid parameters, but also in case the validation fails. The package is able to parse the request and create a proper response with an error, but the returned response uses the 400 status code. It seems like it was done deliberately: 8b012a1
This causes a problem with some client libraries, for example, StrawberryShake. The library first reads the HTTP header here:
private async Task<GraphQLHttpResponse> ExecuteInternalAsync(
GraphQLHttpRequest request,
Uri requestUri,
CancellationToken ct)
{
// The array writer is needed for formatting the request.
// We keep it up here so that the associated memory is being
// kept until the request is done.
// DO NOT move the writer out of this method.
using var arrayWriter = new ArrayWriter();
using var requestMessage = CreateRequestMessage(arrayWriter, request, requestUri);
requestMessage.Version = _http.DefaultRequestVersion;
requestMessage.VersionPolicy = _http.DefaultVersionPolicy;
var responseMessage = await _http
.SendAsync(requestMessage, ResponseHeadersRead, ct)
.ConfigureAwait(false);
return new GraphQLHttpResponse(responseMessage);
}
Then it throws an exception because the status code is not Success:
/// <summary>
/// Reads the GraphQL response as a <see cref="OperationResult"/>.
/// </summary>
/// <param name="cancellationToken">
/// A cancellation token that can be used to cancel the HTTP request.
/// </param>
/// <returns>
/// A <see cref="ValueTask{TResult}"/> that represents the asynchronous read operation
/// to read the <see cref="OperationResult"/> from the underlying <see cref="HttpResponseMessage"/>.
/// </returns>
public ValueTask<OperationResult> ReadAsResultAsync(CancellationToken cancellationToken = default)
{
var contentType = _message.Content.Headers.ContentType;
// The server supports the newer graphql-response+json media type and users are free
// to use status codes.
if (contentType?.MediaType.EqualsOrdinal(ContentType.GraphQL) ?? false)
{
return ReadAsResultInternalAsync(contentType.CharSet, cancellationToken);
}
// The server supports the older application/json media type and the status code
// is expected to be a 2xx for a valid GraphQL response.
if (contentType?.MediaType.EqualsOrdinal(ContentType.Json) ?? false)
{
_message.EnsureSuccessStatusCode();
return ReadAsResultInternalAsync(contentType.CharSet, cancellationToken);
}
_message.EnsureSuccessStatusCode();
throw new InvalidOperationException("Received a successful response with an unexpected content type.");
}
Then it ignores all the content here and returns:
{
DataFactory: {},
Errors: [
{
Message: Response status code does not indicate success: 400 (Bad Request).
}
]
}
So the error message, constructed with AddError via the IGraphQLValidator isn't available for the user, which makes debugging difficult.
Since switching from 400 to 200 may be considered a breaking change by some applications that use EntityGraphQL, I suggest making it opted in, via some initialization argument.
While on the matter, it's worth switching, or adding, the application/graphql-response+json mead type option, instead application/json (here: EntityGraphQLEndpointRouteExtensions.cs). Same as before, for backward compatibility, it probably should be opted in via initialization configuration.
BTW, it's the third ticket I've opened in the last few weeks, but that's only because I'm actively using EntityGraphQL. Thanks a lot for all the effort you are putting into this package. It works well, streamlines the development, and helps a lot - thanks!