ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.27k stars 748 forks source link

Apollo Tracing changes status code to 500 on error #5118

Closed oravecjakub closed 1 year ago

oravecjakub commented 2 years ago

Is there an existing issue for this?

Describe the bug

Hot Chocolate returns status code 500 Internal server error instead of 400 Bad request for invalid or too complex queries when Apollo Tracing feature is enabled Always or OnDemand. image

Same behavior was observed when the query exceeded the complexity limit -- 500 with Apollo Tracing, 400 without it.

According to @michaelstaib, tracing should not change the status code https://github.com/ChilliCream/hotchocolate/issues/5116#issuecomment-1145813987

Steps to reproduce

  1. Enable Apollo Tracing feature in On-demand mode (TracingPreference = TracingPreference.OnDemand)
  2. Fire invalid request (e.g. request nonexistent field) without GraphQL-Tracing header
  3. Observe 400 Bad request status ✔
  4. Fire the same request but this time with the GraphQL-Tracing=1 header
  5. Observe 500 Internal server error ❌

Relevant log output

No response

Additional Context?

No response

Product

Hot Chocolate

Version

12.11.0-preview.1

michaelstaib commented 2 years ago

I have tried it in our tests and I always get the correct status code 400. Can you create a small project that has this error?

oravecjakub commented 2 years ago

Thanks for looking into this. I've created an asp.net core project where the issue manifests. Nugets:

Program.cs

using HotChocolate.Execution.Options;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddGraphQLServer()
    .AddApolloTracing(TracingPreference.OnDemand)
    .AddQueryType<QueryType>();

var app = builder.Build();

app.UseRouting();
app.UseEndpoints(endpoints =>
{
    endpoints.MapGraphQL("/graphql");
});

app.Run();

public class QueryType
{
    public string GetHello(string name) => $"Hello, {name}!";
}

Result without X-Apollo-Tracing header image

Result with X-Apollo-Tracing header image

Source code HC5118_Repro.zip

Hope this helps

michaelstaib commented 2 years ago

I have verified the error ... we will do a fix for 12.12.1

michaelstaib commented 2 years ago

This one is now fixed for version 12.12.1. I have issued the build and it should be available on NuGet in the next 30 min. I will still keep this issue open until this issue is also fixed for version 13.

michaelstaib commented 2 years ago

Thank you for the repro!