graphql-python / graphql-server

This is the core package for using GraphQL in a custom server easily
MIT License
120 stars 72 forks source link

Wrong execution return type with syntax errors and return_promise=True #5

Closed ksonj closed 4 years ago

ksonj commented 6 years ago

In the presence of syntax errors in the graphql-query, graphql-server does not respect the return_promise execution setting.

The return_promise keyword argument is correctly passed down to graphql-core's graphql-function, such that the ExecutionResults are properly wrapped in a Promise. There was a bug in graphql-core where error-responses would still be unwrapped ExecutionResults, which is already fixed.

However, as the parsing of the query is separate from the actual execution, this does not cover errors due to invalid queries (e.g. because of syntax errors). graphql-server will wrap parsing errors in an ExecutionResult anyway though and return these as such, without looking at the return_promise keyword. I think it's debatable whether parsing errors should be an ExecutionResult at all (seeing as there has been no execution of anything), but as that's the way it is currently implemented at least those should be wrapped in a Promise if the user requested as much.

ksonj commented 6 years ago

I've prepared a pull request for this: https://github.com/graphql-python/graphql-server-core/pull/6

Cito commented 4 years ago

Fixed as proposed in #6 and added a test now.