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

Error code from GraphQL is not correct in StrawberryShake #4596

Closed thomastvedt closed 1 year ago

thomastvedt commented 2 years ago

Is there an existing issue for this?

Describe the bug

I expected my StrawberryShake client to return error codes from the GraphQL response, but result.Errors[0].Code is null. The error code can be found inside result.Errors[0].Extensions["body"], but I assume that this is not how this is intended to work? Perhaps some sort of serialization bug?

I made a quick reproduction here: https://github.com/thomastvedt/HotChocolateBug

The following mutation:

mutation {
  saveUserEmail(userId:"123", email:"incalid_email") {
    id
    name
    age
    email
  }
}

results in the following GraphQL response:

{
  "errors": [
    {
      "message": "Invalid email",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "saveUserEmail"
      ],
      "extensions": {
        "message": "Invalid email",
        "stackTrace": "   at Server.Domain.UserService.SaveUserEmail(String id, String email) in /Users/thomastvedt/projects/HotChocolateBug/Server/Domain/UserService.cs:line 13\n   at Server.UserMutation.SaveUserEmail(String userId, String email, UserService userService) in /Users/thomastvedt/projects/HotChocolateBug/Server/UserMutation.cs:line 10\n   at HotChocolate.Resolvers.Expressions.ExpressionHelper.AwaitTaskHelper[T](Task`1 task)\n   at HotChocolate.Types.Helpers.FieldMiddlewareCompiler.<>c__DisplayClass9_0.<<CreateResolverMiddleware>b__0>d.MoveNext()\n--- End of stack trace from previous location ---\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.ExecuteResolverPipelineAsync(CancellationToken cancellationToken)\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.TryExecuteAsync(CancellationToken cancellationToken)",
        "code": "CUSTOM_ERROR_CODE"
      }
    }
  ]
}

When running the console app with the generated StrawberryShake client I get the following result:

image

I was expecting to find my CUSTOM_ERROR_CODE under result.Errors[0].Code, but this is null.

Is this a bug or expected?

Steps to reproduce

I made a quick reproduction here: https://github.com/thomastvedt/HotChocolateBug

  1. Run server (HotChocolate server)
  2. Run console app (StrawberryShake client)

Relevant log output

No response

Additional Context?

No response

Product

Strawberry Shake

Version

12.4.1

thomastvedt commented 2 years ago

https://github.com/ChilliCream/hotchocolate/blob/6ff9f9730aa16ec70ed0d17d8750d44c62f224b4/src/StrawberryShake/Client/test/Core.Tests/Json/JsonErrorParserTests.cs#L170-L198

?

thomastvedt commented 2 years ago

In the generated code:

        public global::StrawberryShake.IOperationResult<ISaveUserEmailResult> Build(global::StrawberryShake.Response<global::System.Text.Json.JsonDocument> response)
        {
            (ISaveUserEmailResult Result, SaveUserEmailResultInfo Info)? data = null;
            global::System.Collections.Generic.IReadOnlyList<global::StrawberryShake.IClientError>? errors = null;
            if (response.Exception is null)
            {
                try
                {
                    if (response.Body != null)
                    {
                        if (response.Body.RootElement.TryGetProperty("data", out global::System.Text.Json.JsonElement dataElement) && dataElement.ValueKind == global::System.Text.Json.JsonValueKind.Object)
                        {
                            data = BuildData(dataElement);
                        }

                        if (response.Body.RootElement.TryGetProperty("errors", out global::System.Text.Json.JsonElement errorsElement))
                        {
                            errors = global::StrawberryShake.Json.JsonErrorParser.ParseErrors(errorsElement);
                        }
                    }
                }
                catch (global::System.Exception ex)
                {
                    errors = new global::StrawberryShake.IClientError[]{new global::StrawberryShake.ClientError(ex.Message, exception: ex, extensions: new global::System.Collections.Generic.Dictionary<global::System.String, global::System.Object?>{{"body", response.Body?.RootElement.ToString()}})};
                }
            }
            else
            {
                errors = new global::StrawberryShake.IClientError[]{new global::StrawberryShake.ClientError(response.Exception.Message, exception: response.Exception, extensions: new global::System.Collections.Generic.Dictionary<global::System.String, global::System.Object?>{{"body", response.Body?.RootElement.ToString()}})};
            }

            return new global::StrawberryShake.OperationResult<ISaveUserEmailResult>(data?.Result, data?.Info, _resultDataFactory, errors);
        }

I guess response.Exception is not null. The GraphQL request returns http status 500. So the strawberryshake client handles this as a "serious" unknown 500 bad exception? Am I supposed to return http status code 200 from the server in this case?

thomastvedt commented 2 years ago

https://github.com/thomastvedt/HotChocolateBug/blob/8a4723e40afddd18fe40577b44b45bf72f92602d/Server/CustomHttpResultSerializer.cs#L25-L27

If I add a custom IHttpResultSerializer and make sure that my server returns http status code 200 for my custom, "expected" errors, everything is mapped ok in StrawberryShake.

But I'm still not sure how this is supposed to work.
Am I supposed to return http status 200 for "known errors"?
GraphQL doesn't really care about http status codes, perhaps SS shouldn't care either?

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

chrisdrobison commented 1 year ago

@michaelstaib @PascalSenn I'd like to take a crack at this one

michaelstaib commented 1 year ago

Are you on slack? Slack.Chillicream.com? Ping me there

chrisdrobison commented 1 year ago

Just pinged you

michaelstaib commented 1 year ago

This issue is now fixed.