Relay Guid Identifier Unable to convert type from `Object` to `Guid`

[PHILLIPS71]

Product

Hot Chocolate

Version

14.0.0-p.100

Link to minimal reproduction

https://github.com/PHILLIPS71/HC-7110

Steps to reproduce

I've previously used Guid identifiers in relay without any issues, though after upgrading to version 14 it doesn't seem to work anymore.

What is expected?

The relay identifiers can be converted into a GUID

What is actually happening?

{
  library(where: { id: { eq: "TGlicmFyeTq+UuauxXXqSp5/h9rbTMR0" } }) {
    id
    name
    slug
  }
}

{
  "errors": [
    {
      "message": "Unexpected Execution Error",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "library"
      ],
      "extensions": {
        "message": "Unable to convert type from `Object` to `Guid`",
        "stackTrace": "   at HotChocolate.Utilities.DefaultTypeConverter.Convert(Type from, Type to, Object source)\r\n   at HotChocolate.Data.Filters.Expressions.QueryableComparableOperationHandler.ParseValue(IValueNode node, Object parsedValue, IType type, QueryableFilterContext context)\r\n   at HotChocolate.Data.Filters.Expressions.QueryableComparableEqualsHandler.HandleOperation(QueryableFilterContext context, IFilterOperationField field, IValueNode value, Object parsedValue)\r\n   at HotChocolate.Data.Filters.Expressions.QueryableOperationHandlerBase.TryHandleOperation(QueryableFilterContext context, IFilterOperationField field, ObjectFieldNode node, Expression& result)\r\n   at HotChocolate.Data.Filters.FilterOperationHandler`2.TryHandleEnter(TContext context, IFilterField field, ObjectFieldNode node, ISyntaxVisitorAction& action)\r\n   at HotChocolate.Data.Filters.FilterVisitor`2.OnFieldEnter(TContext context, IFilterField field, ObjectFieldNode node)\r\n   at HotChocolate.Data.Filters.FilterVisitorBase`2.Enter(ObjectFieldNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxWalker`1.Enter(ISyntaxNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.Visit[TNode,TParent](TNode node, TParent parent, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.VisitChildren(ObjectValueNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.VisitChildren(ISyntaxNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.Visit[TNode,TParent](TNode node, TParent parent, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.VisitChildren(ObjectFieldNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.VisitChildren(ISyntaxNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.Visit[TNode,TParent](TNode node, TParent parent, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.VisitChildren(ObjectValueNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.VisitChildren(ISyntaxNode node, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.Visit[TNode,TParent](TNode node, TParent parent, TContext context)\r\n   at HotChocolate.Language.Visitors.SyntaxVisitor`1.Visit(ISyntaxNode node, TContext context)\r\n   at HotChocolate.Data.Filters.Expressions.QueryableFilterProvider.<ConfigureField>g__VisitFilterArgumentExecutor|11_0(IValueNode valueNode, IFilterInputType filterInput, Boolean inMemory)\r\n   at HotChocolate.Data.Filters.Expressions.QueryableFilterProvider.<>c__DisplayClass15_0`1.<CreateApplicator>b__0(IResolverContext context, Object input)\r\n   at HotChocolate.Data.Filters.Expressions.QueryableFilterProvider.QueryableQueryBuilder.Apply(IMiddlewareContext context)\r\n   at HotChocolate.Types.UnwrapFieldMiddlewareHelper.<>c__DisplayClass0_1.<<CreateDataMiddleware>b__1>d.MoveNext()\r\n--- End of stack trace from previous location ---\r\n   at HotChocolate.Types.UnwrapFieldMiddlewareHelper.<>c__DisplayClass0_1.<<CreateDataMiddleware>b__1>d.MoveNext()\r\n--- End of stack trace from previous location ---\r\n   at HotChocolate.Data.Projections.FirstOrDefaultMiddleware`1.InvokeAsync(IMiddlewareContext context)\r\n   at HotChocolate.Utilities.MiddlewareCompiler`1.ExpressionHelper.AwaitTaskHelper(Task task)\r\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.ExecuteResolverPipelineAsync(CancellationToken cancellationToken)\r\n   at HotChocolate.Execution.Processing.Tasks.ResolverTask.TryExecuteAsync(CancellationToken cancellationToken)"
      }
    }
  ],
  "data": {
    "library": null
  }
}

Relevant log output

No response

Additional context

If you decode TGlicmFyeTq+UuauxXXqSp5/h9rbTMR0 it looks invalid (Library:RuJLt)

[michaelstaib]

Thanks for the repro ... we will have a look at this.

[Hanskrogh]

Is there any status on this?

[michaelstaib]

this is still an issue with 130

[A360JMaxxgamer]

I ran into the same issue. I already start to dig into the problem and created a unittest which reproduces the problem. It seems that the ids are deserialized two times. I hope to fix this problem soon and will do a pr then.

[A360JMaxxgamer]

I created a branch which reproduces the problem https://github.com/ChilliCream/graphql-platform/tree/mha/7110-id-filter-bug.

The problem seems to be that Member type is null in RelayIdFilterFieldExtensions. That's why a wrong Id serializer is created.

So far I did not find the right spot where the Member should be set because my knowledge of the code is not good enough. I will try to find the solution but maybe somehow knows it right away.

   public static IFilterOperationFieldDescriptor ID(
        this IFilterOperationFieldDescriptor descriptor)
    {
        if (descriptor is null)
        {
            throw new ArgumentNullException(nameof(descriptor));
        }

        descriptor
            .Extend()
            .OnBeforeCompletion((c, d) =>
            {
                var returnType = d.Member is null ? typeof(string) : d.Member.GetReturnType(); // <-- d.Member is null
                var returnTypeInfo = c.DescriptorContext.TypeInspector.CreateTypeInfo(returnType);
                d.Formatters.Push(CreateSerializer(c, returnTypeInfo.NamedType));
            });

        return descriptor;
    }

[A360JMaxxgamer]

@michaelstaib I am sorry. I don't find the correct spot. Maybe the unittest and the text above helps you to find the bug. Give me a ping if there is anything I can do.

[PHILLIPS71]

After further investigation, I was able to resolve the issue, though I don't have enough context to be sure it won't cause issues elsewhere.

It seems the problem lies in the OptimizedNodeIdSerializer class, specifically in the Parse function in this section:

if (serializer.ValueSerializer.IsSupported(runtimeType))
{
    valueSerializer = serializer.ValueSerializer;
}
else if (valueSerializer is null)
{
    valueSerializer = TryResolveSerializer(runtimeType);
}

if (valueSerializer is null)
{
    throw SerializerMissing(typeName, rentedBuffer);
}

var value = ParseValue(valueSerializer, serializer.TypeName, span.Slice(delimiterIndex + delimiterOffset));
Clear(rentedBuffer);
return value;

In this case, runtimeType is a string, and we want to use the GuidNodeIdValueSerializer to parse the string. However, this condition if (serializer.ValueSerializer.IsSupported(runtimeType)) fails because GuidNodeIdValueSerializer doesn't support strings. As a result, it falls into TryResolveSerializer which returns a StringNodeIdValueSerializer.

While this works for types like integers, it doesn't for GUIDs when using the new compressed format, which is enabled by default. When the compressed string is later processed to convert it to a GUID, it fails giving us the Unable to convert type from 'Object' to 'Guid' error as we cannot convert RuJ�Lt to a GUID.

To fix this, I think we need to modify the IsSupported method in GuidNodeIdValueSerializer to accept strings. This would allow us to decode the compressed string into a GUID string and then eventually convert that into the GUID as expected.

@michaelstaib If this approach seems reasonable and you're not concerned about it causing any issues, I'm happy to submit a PR.

[michaelstaib]

This would not fix the issue just just put a workaround in because another component passes on the wrong type information. The issue is with filtering, and we have to fix it there.

[michaelstaib]

In filtering probably the runtime type is not set, as far as I can tell from this issue.

[michaelstaib]

@A360JMaxxgamer was on the right track.

[PHILLIPS71]

@michaelstaib I see. In that case, it looks like the FilterOperationFieldDescriptor doesn't have a constructor that initialize the Member property, at least none that I can find. I've attached the relevant code where it's being created, but I'm not entirely sure how everything connects or where you would retrieve the MemberInfo to pass in or set it, if it's available.

public IFilterOperationFieldDescriptor Operation(int operationId)
{
    var fieldDescriptor =
        Operations.FirstOrDefault(t => t.Definition.Id == operationId);

    if (fieldDescriptor is null)
    {
        fieldDescriptor = FilterOperationFieldDescriptor.New(
            Context,
            operationId,
            Definition.Scope);
        Operations.Add(fieldDescriptor);
    }

    return fieldDescriptor;
}

[A360JMaxxgamer]

I created a pr. It fixes the issue but it feels a bit hacky. I did not find a better solution. But I now understand the problem. before we tried to set the member data which was used to deserialize the value. Actually we could never use that or set that. The member type on the filterfieldoperation can be different for various inputs. The field type for e.g. "nin" on the schema is always IdFilterOperationNin therefore we can not determine the runtime type for a specific id when we complete that type.

I tried to move the logic to the input parser. The input parser has a target type. So when the parser sees a formatter for ids it tries to pass the targetType to it.

[michaelstaib]

@A360JMaxxgamer I will have a look through the PR and then we see how we can make it work.

[jbenettius]

Checking in on this issue. I just came across this when upgrading to 14.1.0. My scenario is slightly different but also related to deserializing Guids from a node id. https://github.com/jbenettius/Hotchocolate_Issues/tree/main/NodeIdIssue

[A360JMaxxgamer]

@jbenettius I had the same error message. I was able to fix it by registering the serializer with usage of url safe base64 encoding

 .AddDefaultNodeIdSerializer(useUrlSafeBase64: true)
 .AddGlobalObjectIdentification()

[jbenettius]

@A360JMaxxgamer thank you for the recommendation, but unfortunately that did not work for me.

[adraut]

The .AddDefaultNodeIdSerializer(useUrlSafeBase64: true) did not work for me either on version 14.0. What did work was turning off the new id format. .AddDefaultNodeIdSerializer(outputNewIdFormat: false)

@A360JMaxxgamer @jbenettius

[jbenettius]

@adraut thank you for the suggestion, but that also did not work for me.

I was able to successful deserialize the Id by implementing my own INodeIdValueSerializer, for Guid, changing the parsing to the N format. This directly relates to @A360JMaxxgamer's findings of it using the wrong serializer for Guids.

[jonmaylandvitrolife]

@michaelstaib Are there any updates on this? We're really looking forward to try out HC14, but this issue holds us back.

[Badabum]

workaround for those who want to specify custom Guid format (tested on v 15.0.0-pre3)

// add custom node id value serializer
internal sealed class GuidCustomNodeIdValueSerializer : INodeIdValueSerializer
{
    public bool IsSupported(Type type) => type == typeof(Guid) || type == typeof(Guid?);

    public NodeIdFormatterResult Format(Span<byte> buffer, object value, out int written)
    {
        if (value is Guid g)
        {
            return Utf8Formatter.TryFormat(g, buffer, out written, format: 'D') // desired format
                ? NodeIdFormatterResult.Success
                : NodeIdFormatterResult.BufferTooSmall;
        }

        written = 0;
        return NodeIdFormatterResult.InvalidValue;
    }

    public bool TryParse(ReadOnlySpan<byte> buffer, [NotNullWhen(true)] out object? value)
    {
        if (Utf8Parser.TryParse(buffer, out Guid parsedValue, out _, 'D')) // specify format here (or pass as constructor param)
        {
            value = parsedValue;
            return true;
        }

        value = null;
        return false;
    }
}

// where DI for hotchocolate is configured
services
.AddNodeIdValueSerializer<GuidCustomNodeIdValueSerializer>()
.AddGlobalObjectIdentification();
//...other registrations

// remove default `GuidNodeIdValueSerializer` which is internal
 services
            .Where(t => 
                t.ServiceType == typeof(INodeIdValueSerializer) 
                && t.ImplementationInstance != null 
                && t.ImplementationInstance.GetType().Name.Contains("GuidNodeIdValueSerializer"))
            .ToList()
            .ForEach(s => services.Remove(s));

[BryanEuton]

I can confirm that this is still an issue. Disabling the compression fixed the filtering for us. Please let us know when we can turn compression back on.