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

IError's WithCode only sets error code in extension, not at root error body #7742

Closed nbrosz closed 1 week ago

nbrosz commented 1 week ago

Product

Hot Chocolate

Version

14.1.0

Link to minimal reproduction

https://github.com/nbrosz/hot-chocolate-bug-demo

Steps to reproduce

  1. Set up an ASP.Net web app with HotChocolate.AspNetCore installed.
  2. Configure HotChocolate to use an error filter on the query.
  3. Set an error code for filtered errors within the error filter.
  4. Throw an exception from within a query.
  5. Observe that the error code is only present in the extensions and not as a stand-alone field on the top-level error response.

What is expected?

Given that the IError type has a Code property, and it is not uncommon for errors to have both a message and a code, I would expect calling the .WithCode("CUSTOM_ERROR_CODE") extension method would, at a minimum, cause a code field to be present in the returned JSON response.

{
  "errors": [
    {
      "message": "Unexpected Execution Error",
      "code": "CUSTOM_ERROR_CODE",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "testError"
      ]
    }
  ],
  "data": null
}

What is actually happening?

The error code gets added only to the extensions collection, but not as a stand-alone error code.

{
  "errors": [
    {
      "message": "Unexpected Execution Error",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "testError"
      ],
      "extensions": {
        "code": "CUSTOM_ERROR_CODE"
      }
    }
  ],
  "data": null
}

Relevant log output

Additional context

The company I work for is very stringent about consistent structure between errors. Top-level errors containing a code is a given, and an inability to supply a code at the top level likely will necessitate a far more cumbersome manual solution, like introducing a query-based payload structure, rather than requiring clients to drill down into the nebulous extensions object.

nbrosz commented 1 week ago

Go figure, after I write all this up, I thought to google for Apollo's GraphQL error documentation and, sure enough, HotChocolate's approach follows Apollo.

It would be nice to have the option of diverging from that pattern if one desires, as clients that respond to errors and don't just bubble them up typically will code against an error code, so having the code prominently available on the error seems like it would be convenient, but I also understand if that's a hard line in the sand while following Apollo's designs.

tobias-tengler commented 1 week ago

The fields on the error object are defined by the GraphQL specification (not Apollo). You can find the relevant parts here.

It explicitly warns about extending the error object with unspecified fields like code: Image

Closing as introducing such a change, even opt-in, for the sole purpose of convenience, doesn't make much sense.