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

Throw an error on asynchronous introspection query behavior. #1955

Merged
merged 1 commit into from
Nov 13, 2018

Conversation

abernix
Copy link
Member

@abernix abernix commented Nov 13, 2018

We expect introspection queries to behave in an synchronous manner since
they do not have any resolvers which return Promises. This expectation
seems to also be had by graphql-js which utilizes graphqlSync, rather
than graphql for execution of introspection queries. In fact, this may be
one of the entire reasons that graphqlSync exists: to fulfill a contract
for synchronous execution of server introspection. The introspection tests
within graphql-js seem to support this theory[0].

Utilities which wrap GraphQL resolvers should take care to maintain the
execution dynamics of what they are wrapping, or they should avoid wrapping
introspection types entirely by checking the type with the
isIntrospectionType predicate function from graphql/type[1].

Closes: #1935

We expect introspection queries to behave in an synchronous manner since
they do not have any resolvers which return Promises.  This expectation
seems to also be had by `graphql-js` which utilizes `graphqlSync`, rather
than `graphql` for execution of introspection queries.  In fact, this may be
one of the entire reasons that `graphqlSync` exists: to fulfill a contract
for synchronous execution of server introspection.  The introspection tests
within `graphql-js` seem to support this theory[[0]].

Utilities which wrap GraphQL resolvers should take care to maintain the
execution dynamics of what they are wrapping, or they should avoid wrapping
introspection types entirely by checking the type with the
`isIntrospectionType` predicate function from `graphql/type`[[1]].

[0]: https://github.com/graphql/graphql-js/blob/787422956c9554d12d063a41fe35705335ec6290/src/type/__tests__/introspection-test.js
[1]: https://github.com/graphql/graphql-js/blob/74d1e941/src/type/introspection.js#L484.
Closes: #1935
@abernix abernix merged commit c82cdec into master Nov 13, 2018
@abernix abernix deleted the abernix/error-on-async-introspection-execution branch November 13, 2018 10:12
abernix added a commit that referenced this pull request Nov 13, 2018
I foolishly used `O.p.hasOwnProperty` here which, while safe for checking
properties, is actually not correct since this `then` property is inherited
from `Promise.prototype.then`.

Checking if `then` is a function should be safe _enough_.

Follows-up on: #1955
abernix added a commit that referenced this pull request Nov 13, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generateSchemaHash doesn't await execution result
1 participant