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

Add extensions property to GraphQLError. #204

Closed
wants to merge 1 commit into from

Conversation

jckw
Copy link

@jckw jckw commented Aug 11, 2018

Allows an extensions property to be passed when creating a GraphQLError as described in graphql-python/graphql-core#203.

This property is included in graphql-js, which makes graphql-python more accurately reflect it.

@jckw
Copy link
Author

jckw commented Aug 11, 2018

No idea why some tests are failing. Any help?

@nikordaris
Copy link

Run tox locally and commit what it fixes. it will fix the formatting errors for you in the future if you install the pre-commit hook

@nikordaris
Copy link

Also, you should probably add extensions to the slots list.

@nikordaris
Copy link

Another problem with this is the format_error doesn't merge extensions from original_error. This is a common issue in my opinion with graphql-core. The executor wraps all raised errors in a GraphQLLocatedError which masks the original error in error reporting.

@dcosson
Copy link

dcosson commented Mar 6, 2019

@jckw @nikordaris any chance of getting this merged to keep this library up to date with the graphql protocol?

Copy link
Collaborator

@dan98765 dan98765 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had to add 'extensions' as an override in our codebase since it's not in graphql-core yet.. so I'm also keen to see this feature merged. I do not see any tests added or updated so far.
@jckw - could you please add/update test(s) that exercise the new code?

@nikordaris
Copy link

I think we should close this PR in favor of graphql-python/graphql-core#214 since it is passing the build and addresses the issues brought up in this PR

@jckw
Copy link
Author

jckw commented Mar 1, 2021

Closed in favour of #214

@jckw jckw closed this Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants