Skip to content

Question: Document how to raise exceptions to get an "errors" list in response #1469

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

Closed
khink opened this issue Oct 28, 2022 · 11 comments
Closed

Comments

@khink
Copy link

khink commented Oct 28, 2022

First off, this is a question. I apologize for creating a ticket here. As suggested in the "bug report" issue template, i asked on StackOverflow first, but unfortunately there are no responses yet. I realize this does not give me the right to create an issue . I'm hoping my question might benefit the documentation in some way.

I don't know how to raise an exception in such a way that it results in an "errors" list in the response data.

In #1410 (comment), @erikwrede wrote that we can just raise a GraphQLError. I don't think that's documented yet. All i found about GraphQLError usage is with regard to validators: https://docs.graphene-python.org/en/latest/execution/queryvalidation/#implementing-custom-validators There's nothing there that says that the response will have an "errors" list.

The reason is ask is that this used to work for us with older Graphene versions (graphene==3.0, graphene-django==3.0.0b7, graphql-core==3.1.7) but stopped working after we upgrade (graphene==3.1.1, graphene-django==3.0.0 and graphql-core==3.2.3).

This used to work:

class DeleteOrderlineMutation(graphene.Mutation):
    Output = graphene.ID()

    class Arguments:
        id = graphene.UUID()
        order_id = graphene.ID()

    @classmethod
    def mutate(cls, root, info, id, order_id):
        user = info.context.user
        order = Order.objects.for_user(user).get(pk=order_id)
        if not order.is_editable_by(user):
            raise GraphQLError(
                "Order mutation not allowed, Orderline can not be deleted."
            )

It would result in a response like this:

{
  ...
  "errors": [
      {"message": "Order mutation not allowed, Orderline can not be deleted.", ...}
  ]
}

However, after upgrade this gives

{
  'data': {
    'deleteOrderline': "<Promise at 0x7f899900d5b0 rejected with GraphQLError('Order mutation not allowed, Orderline can not be deleted.')>"
  }
}

I've read through the changelog of graphene and graphene-django. As this "broke" (for us) in graphene-django 3.0.0b8, i thought graphql-python/graphene-django#1327 might have something to do with it. But then, that also requires a newer version of graphene.

Thanks for taking the time to read this.

@erikwrede
Copy link
Member

I believe that this problem is caused by graphene-django rather than graphene. Usually, raising a graphql error should add an entry to the errors list for the specific resolver (identified by the path) rather than return a string. I'm usually on a non-django stack and it works exactly like you'd expect. I'll search through the code and get back to you!

@erikwrede
Copy link
Member

hoi @khink, somehow my comment on this was not posted. It's been some time since then so I'll need to look again, sorry!

@edwinvandeven
Copy link

@erikwrede
Would also be interested in your findings, I'm running into the same as @khink

@payamt007
Copy link

I have same issue, please help!

@Satcheel
Copy link

+1 for this issue. I have been breaking my head for a day around this.

There is little to no documentation around error handling and things also seem to be broken.
For queries, I get the above 'Promise returned the error' whereas for mutations no error or anything - just the fields are null.

Will appreciate a fix for the issue and would love to see some proper documentation with custom handlers etc.

@erikwrede
Copy link
Member

Is anyone able to post a minimal reproduction as a gist (including django setup)? I'm no longer able to reproduce the behavior on a simple django app using the newest versions.

@khink
Copy link
Author

khink commented Jan 31, 2023

Reproduced it with a minimal setup: https://github.com/khink/graphene-issue-1469/

@erikwrede
Copy link
Member

erikwrede commented Jan 31, 2023

@khink I see why my reproduction didn't work anymore. I had debug mode disabled. Setting Debug = False in settings.py fixes the error for me.

The problem is in the DjangoDebugMiddleware of graphene-django. Whenever Debug=True, it wraps all calls to resolvers (like your mutation resolver). On error, it uses the old promise library which graphql-core>2 no longer supports and does Promise.reject(error):

https://github.com/graphql-python/graphene-django/blob/d18cab8aa4b49a0314e7d26b877e88b7eb30a3d0/graphene_django/debug/middleware.py#L66-L69

https://github.com/graphql-python/graphene-django/blob/d18cab8aa4b49a0314e7d26b877e88b7eb30a3d0/graphene_django/debug/middleware.py#L23-L26

The correct way to handle exceptions in graphql core 3 is to raise them. Promises are treated like a valid response object. In your case, the promise can be converted to the ID type which is an implicit str, so the promise is converted to a string representation and treated like a response instead of an exception.

To solve the issue, you can try disabling the middleware or send a PR to the graphene-django team that removes the outdated promise code. I hope this helps, sorry it took me so long to get to it.

You can disable the middleware by removing "graphene_django.debug.DjangoDebugMiddleware" from settings.Middleware after initializing graphene-django. It is added here:
https://github.com/graphql-python/graphene-django/blob/d18cab8aa4b49a0314e7d26b877e88b7eb30a3d0/graphene_django/settings.py#L48-L49

@khink
Copy link
Author

khink commented Feb 1, 2023

Thanks, i'd never have found that. Thanks especially since the reason is not within this project. I'll think about whether i want to report this at graphene-django, for now i'm happy that i was able to get everything working.

Edit: There's already a ticket, i think graphql-python/graphene-django#1367 covers exactly this.

@khink khink closed this as completed Feb 1, 2023
@erikwrede
Copy link
Member

Great to hear I could help! I've forwarded the issue to @firaskafri - hopefully it will be resolved soon! 🙂

@firaskafri
Copy link
Contributor

Hello! Solved in graphql-python/graphene-django#1388

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

No branches or pull requests

6 participants