graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

Exceptions thrown in type converters are not surfaced

Open cmeeren opened this issue 1 year ago • 4 comments

Product

Hot Chocolate

Version

14.0.0

Link to minimal reproduction

See zip below

Steps to reproduce

Repro solution: HotChocolateBugRepro.zip

Code for quick reference:

var builder = WebApplication.CreateBuilder(args);
builder.Services
    .AddGraphQLServer()
    .AddQueryType<Query>()
    .AddTypeConverter<string, UserId>(UserId.Parse)
    .AddTypeConverter<UserId, string>(id => id.Value.ToString())
    .BindRuntimeType<UserId, StringType>();
var app = builder.Build();
app.MapGraphQL();
app.Run();

public record UserId(int Value)
{
    public static UserId Parse(string id)
    {
        if (Int32.TryParse(id, out int result))
        {
            return new UserId(result);
        }

        throw new SerializationException("User ID must be an integer", new StringType());
    }
}

public class Query
{
    public string Test(UserId arg) => "";
}

Run this query:

query {
  test(arg: "invalid")
}

What is expected?

An error containing the message User ID must be an integer. Furthermore, the error, being targeted to clients, should not in any way reference the type name UserId (which is an internal Server implementation detail and not part of the schema).

What is actually happening?

The following error, which:

  • does not use my supplied error message
  • references the internal type name in both message and extensions.requestedType
  • is generally clearly oriented to server implementors
{
  "errors": [
    {
      "message": "Unable to convert the value of the argument `arg` to `UserId`. Check if the requested type is correct or register a custom type converter.",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "test"
      ],
      "extensions": {
        "fieldName": "test",
        "argumentName": "arg",
        "requestedType": "UserId"
      }
    }
  ],
  "data": {
    "test": null
  }
}

Relevant log output

No response

Additional context

No response

cmeeren avatar Aug 23 '24 21:08 cmeeren

This is even worse for fields on input types (as opposed to direct field parameters), where there is absolutely no details whatsoever:

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

This is causing confusion for clients.

Error handling is important for a good clients integration experience. Now, our carefully crafted error messages are not used at all. Would it be possible to have a look at fixing this?

cmeeren avatar Oct 24 '24 08:10 cmeeren

is it in your repo ... for inputs?

michaelstaib avatar Oct 24 '24 14:10 michaelstaib

The repro solution contains only the initially reported input parameter case. I'm happy to update it with the input type case, too. Let me know if you'd like that.

cmeeren avatar Oct 24 '24 16:10 cmeeren

Note that the repro behaves differently if you change UserId arg to List<UserId> arg. Then I get this error:

{
  "errors": [
    {
      "message": "Unexpected Execution Error",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "test"
      ],
      "extensions": {
        "message": "Unable to convert type from `Object` to `UserId`",
        "stackTrace": "   at HotChocolate.Utilities.DefaultTypeConverter.Convert(Type from, Type to, Object source)\r\n   at HotChocolate.Utilities.DictionaryToObjectConverter.VisitValue(Object value, ConverterContext context)\r\n   at HotChocolate.Utilities.DictionaryVisitor`1.Visit(Object value, TContext context)\r\n   at HotChocolate.Utilities.DictionaryToObjectConverter.VisitList(IReadOnlyList`1 list, ConverterContext context)\r\n   at HotChocolate.Utilities.DictionaryVisitor`1.Visit(Object value, TContext context)\r\n   at HotChocolate.Utilities.DictionaryToObjectConverter.Convert(Object from, Type to)\r\n   at HotChocolate.Types.InputParser.ConvertValue(Type requestedType, Object value)\r\n   at HotChocolate.Types.InputParser.ParseLiteral(IValueNode value, IInputFieldInfo field, Type targetType)\r\n   at HotChocolate.Execution.Processing.MiddlewareContext.CoerceArgumentValue[T](ArgumentValue argument)\r\n   at HotChocolate.Execution.Processing.MiddlewareContext.ArgumentValue[T](String name)\r\n   at lambda_method1(Closure, IResolverContext)\r\n   at HotChocolate.Types.Helpers.FieldMiddlewareCompiler.<>c__DisplayClass9_0.<<CreateResolverMiddleware>b__0>d.MoveNext()\r\n--- End of stack trace from previous location ---\r\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.ExecuteResolverPipelineAsync(CancellationToken cancellationToken)\r\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.TryExecuteAsync(CancellationToken cancellationToken)"
      }
    }
  ],
  "data": {
    "test": null
  }
}

cmeeren avatar Mar 24 '25 12:03 cmeeren

We are on version 15.1.6.0, and I can confirm that the issue is still there. I also tried to use ErrorFilter to intercept it, but we don't get the original exception there.

Image

nZeus avatar Jun 25 '25 09:06 nZeus