graphql-python / graphql-python/graphene-django

Middleware not registering correctly in GraphQLView()

Open
#1,384 4 comments 0 reactions 0 assignees View on GitHub
🐛bug
Dominant language
Python
Stars
4.4k
Forks
760
PR merge metrics
No merged PRs in 30d

Description

* **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
[]
```
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)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.