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
4.96k stars 722 forks source link

Only one error should be returned per field #7074

Open cmeeren opened 3 weeks ago

cmeeren commented 3 weeks ago

Product

Hot Chocolate

Version

14.0.0-p.93

Link to minimal reproduction

See below

Steps to reproduce

Repro solution: HotChocolateBugRepro.zip

Code for reference:

var builder = WebApplication.CreateBuilder(args);

builder
    .Services
    .AddGraphQLServer()
    .AddQueryType<Query>();

var app = builder.Build();

app.MapGraphQL();
app.Run();

public class Query
{
    public string Test() =>
        throw new AggregateException(
            new GraphQLException("Test1"),
            new GraphQLException("Test2")
        );
}

What is expected?

{
  "errors": [
    {
      "message": "Test1",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "test"
      ]
    },
  ],
  "data": null
}

What is actually happening?

{
  "errors": [
    {
      "message": "Test1",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "test"
      ]
    },
    {
      "message": "Test2",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "test"
      ]
    }
  ],
  "data": null
}

Relevant log output

No response

Additional context

If you now go "what?!", then I agree.

The GraphQL spec's section "Handling Field Errors" seems absolutely clear:

If the field returns null because of a field error which has already been added to the "errors" list in the response, the "errors" list must not be further affected. That is, only one error should be added to the errors list per field.

(Emphasis mine.)

I am surprised by this. I think it is useful to be able to return multiple errors per field, for example if a field has multiple arguments and more than one of them are determined (during execution) to be invalid. I am unsure why the spec says this is not allowed. But I can't find any ambiguity here.

I'm OK with this being closed as "wontfix". That, however, puts the responsibility of following the spec on users, which I don't like.

Assuming this will not be fixed, I would very much like the opinions of more experienced GraphQL devs on whether breaking this part of the spec is OK or even the right thing to do.