AddTypeConverter<T1, T2> errors result in NotSupportedException but without an inner exception

[stevewgh]

Is there an existing issue for this?

• I have searched the existing issues

Product

Hot Chocolate

Describe the bug

When using type converters, if the type converter fails then the root cause of the failure is lost and can't be used by the error filter.

The exception is a NotSupportedException but the inner exception is null. The resulting error message is the very generic and unhelpful "Unable to convert type from 'x' to 'y'".

One use case where we would find the route cause useful is within our IError filter where we could check for the inner exception type and then change the error message to something more meaningful or change the error code.

services
  .AddGraphQLServer()
  .AddTypeConverter<T1, T2>(source => throw new CustomException("I want to use this in the error filter below"))
  .AddErrorFilter<MyErrorErrorFilter>();

public class MyErrorErrorFilter : IErrorFilter {
    public IError OnError(IError error)
    {
        if (error.Exception is NotSupportedException { InnerException: CustomException } custom )
        {
            // do something with the custom exception
        }
        return error;
    }
}

Steps to reproduce

1. Add a type converter which throws an exception
2. e.g. .AddTypeConverter<T1, T2>(source => throw new CustomException("not good"))
3. Add an IErrorFilter which tries to use the root cause exception
4. Observe that the NotSupportedException does not contain the CustomException in the inner exception property

Relevant log output

None.

Additional Context?

I have a PR in progress which contains changes isolated to the DefaultTypeConverter to include the inner exception.
If you give me the thumbs up on this issue I'll try to finish it off ASAP.

Version

13.0.5

[michaelstaib]

The converter is designed around a TryConvert and does not really expose exceptions. Most execution engine parts will try to convert things but then have different paths of resolution.

So even if the did that it might have a limited and inconsistent effect.

What do you trie to achieve? Create a scalar by reusing a scalar with type conversion?

[stevewgh]

What do you trie to achieve? Create a scalar by reusing a scalar with type conversion?

We are migrating from the GraphQL.Conventions library and we have a suite of tests which check the error message and error code returned when a scalar is not in the correct format.

Something like this:

  .AddTypeConverter<string, CustomScalar>(source => CustomScalar.Parse(source))

The Parse method could return a number of known custom exceptions and we'd like to detect which exception and convert the error text and error code to match the original implementation but the actual exception thrown is not available to us.

The errors block should look something like this when the Parse method fails.

"errors": 
[
{
    "message": "You provided 'abc' but we expected something like 'A-B-C' ",
    "path": [
    "brokenResolverWithScalarArgument"
    ],
    "extensions": {
    "code": "INVALID_COMPANY_CODE_FORMAT"
    }
}
]

If the exception thrown here included the inner exception then we could achieve our goal.

graphql-platform/src/HotChocolate/Core/src/Types/Utilities/DefaultTypeConverter.cs

Lines 34 to 38 in 6ab57c1

	throw new NotSupportedException(
	string.Format(
	TypeResources.TypeConvertion_ConvertNotSupported,
	from.Name,
	to.Name));

It would require a refactor to make the TryConvert method depend on the Convert method instead of the other way around and then we could simply

throw new NotSupportedException(
                string.Format(
                    TypeResources.TypeConvertion_ConvertNotSupported,
                    from.Name,
                    to.Name),
                innerException);

Is there any another way that we can get the custom exception thrown during the type converter stage?

[michaelstaib]

I have put it on the backlog ... not sure yet if we put it in 13.5 or 13.6