Middleware not registering correctly in GraphQLView()

[jamestsayre]

• What is the current behavior?
  When an exception is raised within a mutate() function, the response to the mutation query is an empty JSON, and the raised exception is swallowed within GraphQLView().execute_graphql_request(). Even in the case of something like this:

class mutationExample(graphene.Mutation)
    status=graphene.String()
    @authorized
    def mutate(root, info, user, **kwargs):
        raise ValueError("Should see this in 'errors' field of JSON reponse!")

the result of e.g. GraphQLView().execute_graphql_request(*args, **kwargs) is something like

ExecutionResult(data={'mutationQuery': {'status': None}}, errors=None)

and the json response is:

{
    "data": {
        "mutationExample": {
            "status": null
        }
    }
}

From working on apps using older versions of graphene-django, I recall that exceptions raised anywhere within the schema definition code would end up propagating to the JSON responses. In any case, the desired behavior here is to have the mutation response include information on any exceptions that were raised.

One suggestion I found indicated that perhaps DjangoDebugMiddleware needed to be included in order to achieve the desired outcome. My understanding is that it is included using something like the following in the Django settings file:

GRAPHENE = {
    'SCHEMA': 'core.graphql.index.schema',
    'MIDDLEWARE': ['graphene_django.debug.middleware.DjangoDebugMiddleware',]
}

Even with the above, the behavior is the same (that is, with and without the 'MIDDLEWARE' entry). With that entry included, I see:

from graphene_django.views import GraphQLView
GraphQLView().middleware
[<graphene_django.debug.middleware.DjangoDebugMiddleware at 0x7f7384595810>]

So it seems that the middleware is indeed registered, but doesn't alter the described behavior. What DOES work is wrapping the middleware argument in an extra list, via something like:

class GQLView(GraphQLView):
   def __init__(self, *args, **kwargs):
        kwargs.update({'middleware':[graphene_settings.MIDDLEWARE] }) #note the extra list
        super().__init__(*args, **kwargs)

and then making sure in urls.py that the above class is what's handling the queries.

Note again that the difference is just ensuring that GraphQLView().middleware ends up being a nested list, e.g. [['graphene_django.debug.middleware.DjangoDebugMiddleware']].

It's not clear to me why wrapping the middleware location in an extra list is yielding the desired behavior.

• What is the expected behavior?
  The expected behavior is for the response JSON to be something like:

{
    "errors": [
        "Should see this in 'errors' field of JSON reponse!"
    ],
    "data": {
        "mutationExample": null
    }
}

• What is the motivation / use case for changing the behavior?

If indeed it's necessary to invoke DjangoDebugMiddleware to avoid vanishing exceptions, the standard way of installing middleware (e.g. in the Django settings file in the GRAPHENE dict) -- which requires that the class path be in a list like the example above -- then requiring a hack to wrap the class path in a second list is impossible to achieve.

• Please tell us about your environment:

  • Version:
  • -python: 3.11.1
  • • Django: 4.1.5
  • • graphene_django: 3.0.0
  • -graphql: 3.2.5
  • Platform:
  • • Linux

• Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow)

[EverWinter23]

Hi @firaskafri @jamestsayre,

The graphql mutation-specification outlines the following:

• Let errors be any field errors produced while executing the selection set

The expected behavior will align with the specification set. I checked out mutation test cases and DjangorFormMutation aligns with the specification.

class DjangoFormMutation(BaseDjangoFormMutation):
    class Meta:
        abstract = True

    errors = graphene.List(ErrorType)
...

For example, this specific test case:

    result = schema.execute(
        """ mutation MyMutation {
            myMutation(input: { text: "INVALID_INPUT" }) {
                errors {
                    field
                    messages
                }
                text
            }
        }
        """
    )

    assert result.errors is None
    assert result.data["myMutation"]["errors"] == [
        {"field": "text", "messages": ["Invalid input"]}
    ]

In case of base field graphene.Mutation, although the result.errors attribute contains the errors, they are not surfaced because the graphene.Mutation does not have a errors field and a resolver.

I would like to take up this issue. Could you suggest a direction I should pursue? Should we rely on the middleware to align with the specification?

[firaskafri]

Hi @firaskafri @jamestsayre,

The graphql mutation-specification outlines the following:

* Let `errors` be any field errors produced while executing the selection set

The expected behavior will align with the specification set. I checked out mutation test cases and DjangorFormMutation aligns with the specification.

class DjangoFormMutation(BaseDjangoFormMutation):
    class Meta:
        abstract = True

    errors = graphene.List(ErrorType)
...

For example, this specific test case:

    result = schema.execute(
        """ mutation MyMutation {
            myMutation(input: { text: "INVALID_INPUT" }) {
                errors {
                    field
                    messages
                }
                text
            }
        }
        """
    )

    assert result.errors is None
    assert result.data["myMutation"]["errors"] == [
        {"field": "text", "messages": ["Invalid input"]}
    ]

In case of base field graphene.Mutation, although the result.errors attribute contains the errors, they are not surfaced because the graphene.Mutation does not have a errors field and a resolver.

I would like to take up this issue. Could you suggest a direction I should pursue? Should we rely on the middleware to align with the specification?

This is great! Are you going to start a PR?

[jaw9c]

For the base issue here the errors are getting consumed by an issue with the Debug toolbar. @jamestsayre could you try with the library version pinned to the current main?

[EverWinter23]

The underlying issue lies with the graphene library as it contains the base class graphene.Mutation which should surface errors as implemented in DjangoFormMutation.