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

When implementing AsyncNodeGraphType ExecutionStrategy throws InvalidOperationException #171

Open
wtorricos opened this issue Aug 10, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@wtorricos
Copy link

I found that when implementing AsyncNodeGraphType the ExecutionStrategy class will throw an InvalidOperationException since the following call returns null:
https://github.com/graphql-dotnet/graphql-dotnet/blob/ed720f46e26704ed457b723801637d9d726f45bd/src/GraphQL/Execution/ExecutionStrategy.cs#L636

The problem seems to be that the execution strategy is not awaiting the result and is trying to find the GraphType of Task instead of just T.

Our implementation broke after we upgraded GraphQL, here you can see the version of our packages:

<PackageReference Include="GraphQL" Version="5.3.3" />
<PackageReference Include="GraphQL.DataLoader" Version="5.3.3" />
<PackageReference Include="GraphQL.MemoryCache" Version="5.3.3" />
<PackageReference Include="GraphQL.MicrosoftDI" Version="5.3.3" />
<PackageReference Include="GraphQL.SystemTextJson" Version="5.3.3" />
<PackageReference Include="GraphQL.Relay" Version="0.8.0" />

Hope this gives you enough information to fix it

@Shane32
Copy link
Member

Shane32 commented Aug 10, 2022

Most likely you need to call FieldAsync rather than Field for some of your field definitions.

See: https://graphql-dotnet.github.io/docs/migrations/migration5#32-valuetask-execution-pipeline-support-changes

@Shane32
Copy link
Member

Shane32 commented Aug 10, 2022

I'd further guess that the field type in this case is an interface or union, and what happened is that IsTypeOf returned false for the Task<T> returned value, causing the type to be null. With this change here in proposed PR graphql-dotnet/graphql-dotnet#3270 : graphql-dotnet/graphql-dotnet#3270 (comment) the code will use the interface type and not cause a null reference exception. Then you would at least see a better exception than what is seen currently, but it would not solve your issue.

Without seeing more of your code, this is just a guess, of course.

@wtorricos
Copy link
Author

wtorricos commented Aug 10, 2022

Thank you for the reply.

Regarding FieldAsync, I don't think that's the problem as even some really simple types are failing, we have many like the one below:

public sealed class CharacterType : AsyncNodeGraphType<CharacterModel>
{
    public CharacterType()
    {
        Name = "Character";

        Id(characterModel => characterModel.Id);

        Field<NonNullGraphType<StringGraphType>>()
            .Name("name")
            .Resolve(context => context.Source.Name ?? "None");
    }

    public override async Task<CharacterModel> GetById(IResolveFieldContext<object> context, string id)
    {
        var characterService = context.RequestServices.GetRequiredService<CharacterService>();
        Guid characterId = Guid.Parse(id);
        return characterService.GetByIdAsync(characterId, context.CancellationToken);
    }
}

The workaround that is working at the moment is to extend NodeGraphType instead of AsyncNodeGraphType

@Shane32
Copy link
Member

Shane32 commented Aug 10, 2022

Right I see. AsyncNodeGraphType<T> (which is in the relay project) inherits from NodeGraphType<Task<T>>, and when you call Id it calls Field instead of FieldAsync. The relay project's AsyncNodeGraphType needs to be fixed so that it does not call Field but instead calls FieldAsync.

@Shane32 Shane32 transferred this issue from graphql-dotnet/graphql-dotnet Aug 10, 2022
@Shane32
Copy link
Member

Shane32 commented Aug 10, 2022

See faulty code here: (when used by the derived AsyncNodeGraphType -- the code is fine as NodeGraphType)

https://github.com/graphql-dotnet/relay/blob/5ad6b1c6367a2cfd83894a1f2f1f761a62c36b4d/src/GraphQL.Relay/Types/NodeGraphType.cs#L91..L98

There's nobody that actively works on this project, but if you'd like to open a pull request @tico321 I would be happy to review it, and when merged, issue an updated release.

@wtorricos
Copy link
Author

Thank you for the fast response, I'll try to take a look over the weekend

@sungam3r sungam3r added the bug Something isn't working label Aug 14, 2022
@wtorricos
Copy link
Author

Supporting NodeGraphType as well as AsyncNodeGraphtype seems to be a little bit trickier than what I thought, using FieldAsync wan't enough, but I found that awaiting the result in QueryGraphType fixes the problem.

return node.GetById(context, parts.Id);

Since I'm only using AsyncNodeGraphType I'm going to use the following class in my project until I can find a way to make it work with both Node and AsyncNode.

public class AsyncQueryGraphType : ObjectGraphType
{
    public AsyncQueryGraphType()
    {
        Name = "Query";

        Field<NodeInterface>()
            .Name("node")
            .Description("Fetches an object given its global Id")
            .Argument<NonNullGraphType<IdGraphType>>("id", "The global Id of the object")
            .ResolveAsync(ResolveObjectFromGlobalId);
    }

    private async Task<object> ResolveObjectFromGlobalId(IResolveFieldContext<object> context)
    {
        var globalId = context.GetArgument<string>("id");
        var parts = Node.FromGlobalId(globalId);
        var node = context.Schema.AllTypes[parts.Type];
        var getById = node!.GetType().GetMethod("GetById");

        dynamic awaitable = getById!.Invoke(node, new object[] { context, parts.Id });
        await awaitable!;
        return awaitable.GetAwaiter().GetResult();
    }
}

Having said that please let me know if you have any suggestions that may work and I'll take a look when I have some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants