Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Unable to input UUID as argument for an ID field #49

Open
anshulshah96 opened this issue Nov 1, 2023 · 8 comments
Open

bug: Unable to input UUID as argument for an ID field #49

anshulshah96 opened this issue Nov 1, 2023 · 8 comments

Comments

@anshulshah96
Copy link

anshulshah96 commented Nov 1, 2023

In ArgumentParser.TryGetValue<T>, the casting of all string value node type is done in the following manner:

value = (T)scalarType.ParseLiteral(valueNode)!;

Refer here.

This causes the following error from the subgraph when the input type for an ID field is UUID:

{
  "errors": [
    {
      "message": "Unexpected Execution Error",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "_entities"
      ],
      "extensions": {
        "message": "Unable to cast object of type 'System.String' to type 'System.Guid'.",
        "stackTrace": "   at ApolloGraphQL.HotChocolate.Federation.Helpers.ArgumentParser.TryGetValue[T](IValueNode valueNode, IType type, String[] path, Int32 i, T& value)\n   at ApolloGraphQL.HotChocolate.Federation.Helpers.ArgumentParser.TryGetValue[T](IValueNode valueNode, IType type, String[] path, Int32 i, T& value)\n   at ApolloGraphQL.HotChocolate.Federation.Helpers.ArgumentParser.GetValue[T](IValueNode valueNode, IType type, String[] path)\n   at lambda_method15(Closure, IResolverContext)\n   at ApolloGraphQL.HotChocolate.Federation.Helpers.EntitiesResolver.ResolveAsync(ISchema schema, IReadOnlyList`1 representations, IResolverContext context)\n   at HotChocolate.Types.ResolveObjectFieldDescriptorExtensions.<>c__DisplayClass3_0`1.<<Resolve>b__0>d.MoveNext()\n--- End of stack trace from previous location ---\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)"
      }
    }
  ]
}
@anshulshah96 anshulshah96 changed the title bug: Unable to input UUID as argument bug: Unable to input UUID as argument for an ID field Nov 1, 2023
@dariuszkuc
Copy link
Member

Hello 👋
UUID is a custom scalar so the logic would have to account for mapping custom scalar (Any) to a custom scalar (UUID). I am unsure whether we will be able to support it.

As a workaround you will need to manually parse it

[ReferenceResolver]
public static Foo GetByFooBar(
    [LocalState] ObjectValueNode data
    Data repository)
{
    // TODO implement logic here by manually reading values from local state data
}

@LavaToaster
Copy link

@dariuszkuc Is this intentional, because this is handled transparently in the previous federation implementation?

@dariuszkuc
Copy link
Member

Did it actually work? The code is pretty much the same this vs that.

Looks like the only difference is around handling non-null types - I only applied the check for object types as it seemed working fine for other types (looks like their fix made it to 13.6.0).

@dariuszkuc
Copy link
Member

The underlying issue is with the IdType. Fields using Guid by itself work fine.

When we invoke the TryGetValue it attempts to fetch the IdType which serializes as a string and it doesn't know that the underlying type is actually Guid (i.e. we don't know that IdType should be wrapping UuidType). So the logic there would somehow need to know to parse the string value (ID) and then try to parse it again to Guid. You would encounter the same problem with any other custom scalar.

Since ID scalar is serialized to String we can also do following workaround

[ReferenceResolver]
public static Foo GetByGuid(
    string id
    Data repository)
{
    var guid = Guid.Parse(id);
    // TODO implement logic here by manually reading values from local state data
}

I am unsure whether HotChocolate allows you to provide custom biding for existing scalar types - IF it does support it then maybe you could provide your own ParseLiteral logic to also support GuidType.

@dariuszkuc
Copy link
Member

related: #19 and #20

@LavaToaster
Copy link

Ah ok that makes sense, we weren't using ID types before so that would explain why we didn't see it.

@anshulshah96
Copy link
Author

I tried the above suggestion of using string in the ReferenceResolvers. But I still run into the following error:

{
  "data": {},
  "errors": [
    {
      "message": "invalid type for variable: '<variable1>'",
      "extensions": {
        "name": "<variable1>",
        "code": "VALIDATION_INVALID_TYPE_VARIABLE"
      }
    },
    {
      "message": "invalid type for variable: '<variable2>'",
      "extensions": {
        "name": "<variable2>",
        "code": "VALIDATION_INVALID_TYPE_VARIABLE"
      }
    }
  ]
}

@dariuszkuc
Copy link
Member

Please provide a link to a repository that reproduces the issue. Otherwise it is very hard to determine what is the underlying issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants