graphql-core icon indicating copy to clipboard operation
graphql-core copied to clipboard

Able to return `None` when you are not supposed to??

Open dcreekp opened this issue 1 year ago • 2 comments

Reporting issues with GraphQL-core 3

I came across an issue that may be due to Graphene but the problem surfaces here.

I was writing a mutation which would raise an Error or return:

{"data": {"someMutationThatReturnsNone": None}

instead of something like:

{"data": {"someMutationThatWillAlwaysBeTrueIfNoErrorRaised": {"success": True}}}

I was happy that this worked:

class SomeMutationThatReturnsNone(graphene.Mutation):

    class Arguments:
        ...

    Output = CustomNullTypeScalar

    def mutate(self, info: graphene.ResolveInfo, **kwargs: Any) -> None:
        ...
        return None

until I realised that this would also work:

class SomeMutationThatReturnsNone(graphene.Mutation):

    class Arguments:
        ...

    Output = graphene.Date(required=True)

    def mutate(self, info: graphene.ResolveInfo, **kwargs: Any) -> None:
        ...
        return None

currently, because the line of code linked above is before the check if is_leaf_type(return_type), so long as Output = someScalarOrEnumType the mutation will return {"data": {"someMutationThatReturnsNone": None}. Which I don't think is right.

Would appreciate your thoughts.

dcreekp avatar Oct 07 '24 15:10 dcreekp

Ok I just realised that this :

class SomeMutationThatReturnsNone(graphene.Mutation):

    class Arguments:
        ...

    Output = graphene.NonNull(graphene.Date)

    def mutate(self, info: graphene.ResolveInfo, **kwargs: Any) -> None:
        ...
        return None

errors as expected. Because the if is_non_null_type(return_type): will catch it first.

So maybe it should be enforced that Output class attributes are wrapped by graphene.NonNull in the Graphene library here.

But I guess that would mean that I will not be able to do what I want: create a CustomNullTypeScalar when I want a mutation to return None. There is nothing in the specs that I can find that prohibits returning something like: {"data": {"someMutationThatReturnsNone": None}.

Again would be interested in your thoughts @Cito 🙏

dcreekp avatar Oct 07 '24 16:10 dcreekp

Sorry for not responding earlier, I currently have very limited capacity.

If you still think this is not only a Graphene issue, but something should be changed in GraphQL-core, please make it more concrete, and provide an example or test case that does not use Graphene.

Cito avatar Jan 25 '25 19:01 Cito

Closing this since there was no more feedback. Feel free to re-open with a more concrete GraphQL-core specific test case or suggestion.

Cito avatar Nov 01 '25 16:11 Cito