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

GraphQLError double wrapping returning result #106

Closed
Checho3388 opened this issue Sep 4, 2020 · 6 comments
Closed

GraphQLError double wrapping returning result #106

Checho3388 opened this issue Sep 4, 2020 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Checho3388
Copy link

I found out something confusing and wanted to know if maybe this could be a bug or if I'm missing something and this is an expected behavior.

This is my example.py file. It only has one resolver that raises an error.

from graphql import (GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString, graphql_sync, )


def resolve_fail(*args, **kwargs):
    raise ValueError("Some error")


schema = GraphQLSchema(
    query=GraphQLObjectType(
        name='RootQueryType',
        fields={
            'hello': GraphQLField(
                GraphQLString,
                resolve=resolve_fail)
        }))

query = '{ hello }'

result = graphql_sync(schema, query)

I'm aware that result.errors[0] is a GraphQLError exception. But, I was expecting result.errors[0].original_error to be ValueError.

However, I can see that result.errors[0].original_error is a GraphQLError and result.errors[0].original_error.original_error is ValueError.

Is this ok?

>>> print(type(result.errors[0]))
<class 'graphql.error.graphql_error.GraphQLError'>
>>> print(type(result.errors[0].original_error))
<class 'graphql.error.graphql_error.GraphQLError'>
>>> print(type(result.errors[0].original_error.original_error))
<class 'ValueError'>
@Cito
Copy link
Member

Cito commented Sep 4, 2020

Thanks you for reporting this, @Checho3388.

The reason for the double-wrapping is that GraphQL-core wraps the ValueError into a GraphQLError in the method resolve_field_value_or_error() and then wraps that in a located_error again in the method handle_field_error().

The problem is with the first wrapping. GraphQL-core simply tries to do too much here. I think the idea was to mimic the asErrorInstance() function in GraphQL.js, but that function only wraps a non-Error-object into an Error object - it does not create a GraphQLError. Since the case that we get a non-Error-object can be ignored in modern Python, we should simply leave the error as it is in Python and not wrap it.

I will work on a fix and regression test for that.

@Cito Cito self-assigned this Sep 4, 2020
@Cito Cito added the bug Something isn't working label Sep 4, 2020
@Cito Cito added this to the v3.1 milestone Sep 4, 2020
@Cito Cito closed this as completed in 3093b5b Sep 5, 2020
@Checho3388
Copy link
Author

Glad to know it was fixed. Thanks for your quick response!

@patrick91
Copy link
Member

@Cito was this ever released? I'm encountering the same issue :)

@Cito
Copy link
Member

Cito commented Feb 3, 2021

@patrick91 Not yet, sorry. Will try to work on a new release this weekend.

@patrick91
Copy link
Member

@Cito awesome, thanks for the prompt response, let me know if you need help :)

@Cito
Copy link
Member

Cito commented Feb 8, 2021

Version 3.1.3 with this fix has now been released.

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