HC throws NullReferenceException after updating from 13.7.0 to 13.8.1

[jhpetersen]

Is there an existing issue for this?

• I have searched the existing issues

Product

Hot Chocolate

Describe the bug

After updating HC from 13.7.0 to 13.8.1 a NullReferenceException is thrown on startup initialization.

The stacktrace (see relevant log output) shows MutationConventionTypeInterceptor.TryApplyInputConvention, for which changes were made in https://github.com/ChilliCream/graphql-platform/pull/6776/files.
So i suspect there was introduced a bug not checking for possible null values?

Steps to reproduce

https://github.com/jhpetersen/HC13.8_MutationConventionEnumIssue

Sadly i don't have any clue, from which type this exception actually gets thrown, so i can not provide a minimal repo to reproduce this, but reverting to HC 13.7.0 fixes this error.
I hope the stacktrace and the hint on the aforementioned PR6776 could assist enough to find the cause.

Relevant log output

Object reference not set to an instance of an object.
   bei HotChocolate.Types.EnumType.ParseValue(Object runtimeValue)
   bei HotChocolate.Types.InputFormatter.FormatValueLeaf(Object runtimeValue, ILeafType type, Path path)
   bei HotChocolate.Types.MutationConventionTypeInterceptor.TryApplyInputConvention(IResolverCompiler resolverCompiler, ObjectFieldDefinition mutation, Options options)
   bei HotChocolate.Types.MutationConventionTypeInterceptor.OnBeforeCompleteMutation(ITypeCompletionContext completionContext, ObjectTypeDefinition definition)
   bei HotChocolate.Configuration.TypeInitializer.MergeTypeExtensions()
   bei HotChocolate.Configuration.TypeInitializer.Initialize()
   bei HotChocolate.SchemaBuilder.Setup.InitializeTypes(SchemaBuilder builder, IDescriptorContext context, IReadOnlyList`1 types)
   bei HotChocolate.SchemaBuilder.Setup.Create(SchemaBuilder builder, LazySchema lazySchema, IDescriptorContext context)
   bei HotChocolate.Execution.RequestExecutorResolver.<CreateSchemaAsync>d__19.MoveNext()
   bei HotChocolate.Execution.RequestExecutorResolver.<CreateSchemaServicesAsync>d__18.MoveNext()
   bei HotChocolate.Execution.RequestExecutorResolver.<GetRequestExecutorNoLockAsync>d__14.MoveNext()
   bei HotChocolate.Execution.RequestExecutorResolver.<GetRequestExecutorAsync>d__13.MoveNext()
   bei HotChocolate.AspNetCore.Warmup.ExecutorWarmupService.<ExecuteAsync>d__5.MoveNext()
   bei Microsoft.Extensions.Hosting.Internal.Host.<StartAsync>d__12.MoveNext()
   bei Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.<RunAsync>d__4.MoveNext()
   bei Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.<RunAsync>d__4.MoveNext()
   bei Microsoft.Extensions.Hosting.HostingAbstractionsHostExtensions.Run(IHost host)
   bei Microsoft.AspNetCore.Builder.WebApplication.Run(String url)
   bei Program.<<Main>$>d__0.MoveNext()

Additional Context?

No response

Version

13.8.1

[michaelstaib]

Can you create a small repro?

[jhpetersen]

Hi Michael, i created a minimal repo to reproduce the issue.

There seems to be a problem with MutationConventions and enum input parameters with a default value.
By downgrading to 13.7 or removing .AddMutationConventions() line from program.cs the error can be bypassed..

[jhpetersen]

Hi, just a short ping: is the bug reproducible / can you confirm it with the provided repo?
(This is holding us back from being able to update to 13.8, because there seems to be no workaround without breaking existing schema..)

[jhpetersen]

Hey, as this bug still exists on HC 13.9.0, i cloned the HC repo and added a test case with mutation conventions enabled and an enum input parameter with a default value, to find the root cause:
The error seems to be thrown in MutationConventionTypeInterceptor on lines 248 ff.

           defaultValue =
                    _context.InputFormatter.FormatValue(
                        argument.RuntimeDefaultValue,
                        argumentType,
                        Path.Root);

and if stepping further into IntputFormatter it comes down to EnumType.cs line 156

          if (_valueLookup.TryGetValue(runtimeValue, out var enumValue))

where _valueLookup is actually null!

Digging into EnumType.Initializations.cs on line 101 _valueLookup is getting initialized, so OnCompleteType() method is not called before accessing ParseValue() within EnumType.cs!!

It does not matter, if one uses a mutation signature with C# default parameter value DoSomething(TestEnum something = TestEnum.A) or with HC DefaultValue attribute like DoSomething([DefaultValue(TestEnum.A)] Optional<TestEnum> something), the same exception gets thrown.

Can you please review this and work on a fix?
I would have proposed a PR, but there is more knowledge on HC internals required - is this a bug on EnumType itself (ParseValue can be accessed before Type completion / not catching possible null on _valueLookup) or on EnumType.Initializations or on MutationConventionTypeInterceptor? I can't tell..

[SvdSinner]

I am seeing this issue as well.

Hunting through the code, I believe the issue lies in HotChocolate.Configuration.TypeInitializer.CompleteType (below)

internal bool CompleteType(RegisteredType registeredType)
{
    if (registeredType.Status is TypeStatus.Completed)
    {
        return true;
    }

    if (!registeredType.IsExtension)
    {
        registeredType.Type.CompleteType(registeredType);
        registeredType.Status = TypeStatus.Completed;
    }

    return true;
}

As far as I can tell, having registerdType.IsExtension == true is the only way that '_valueLookup' can be null. (The value of _valueLookup is set during OnCompleteType() of EnumType.cs)

Unfortunately, I don't understand what that if statement if for, and thus I have no guess as to what might be the correct condition for that if statement.
Alternately, that condition might be correct, but maybe EnumType should implement INamedTypeExtensionMerger as that is what .IsExtension keys on to be true or false.

Again, I don't know enough to say much more beyond that on what the correct fix is.

[jhpetersen]

Hey @michaelstaib, it's now 4 month, could you please review this issue and either remove the investigate label or explain which information is still needed to get this fixed?
Thx 🙏

p.s.: i am not alone with this anymore 😉