generateSchemaHash doesn't await execution result

[wmertens]

There's a problem here:

apollo-server/packages/apollo-server-core/src/utils/schemaHash.ts

Line 11 in 6bd73b1

const result = execute(schema, documentAST) as ExecutionResult;

in graphql-js, this can return a Promise:
https://github.com/graphql/graphql-js/blob/f529809f5408fb9a343e669ea4d4851add3df004/src/execution/execute.js#L141-L144

So generateSchemaHash has to always return a Promise, too.

This probably causes all the graphql-middleware issues, like #1934

[wmertens]

I can confirm that by awaiting the execution result, and doing a delayed storing of the result like this works:

apollo-server-core/dist/utils/schemaHash.js line 15:

async function generateSchemaHash(schema) {
	const introspectionQuery = utilities_1.getIntrospectionQuery()
	const documentAST = language_1.parse(introspectionQuery)
	const result = await execution_1.execute(schema, documentAST)

apollo-server-core/dist/ApolloServer.js: line 226

		this.schemaHash = ''
		schemaHash_1
			.generateSchemaHash(this.schema)
			.then(
				hash => (this.schemaHash = hash),
				err => console.error('Warning: Could not generated schema hash', err)
			)

[townmulti]

TypeError: utilities_1.getIntrospectionQuery is not a function

...is the error I'm getting.

[wmertens]

@martijnwalraven This is not fixed in 2.2.1, right?

[melkir]

The problem is still present in 2.2.1

[martijnwalraven]

Hmmm, this is surprising, because executing an introspection query is normally guaranteed to be synchronous. If graphql-middleware changes this to return an asynchronous result, that seems like a design issue with that library to me. But it would probably make sense to put a workaround in place to avoid errors.

[wmertens]

The workaround I describe above worked fine for me

[martijnwalraven]

Yep, I'm not saying that workaround doesn't work, but it's unfortunate it's required because introspection is meant to be synchronous.

[abernix]

Just to be clear, this problem only manifests itself if when graphql-middleware is used. The current contract for schemaHash necessitates that it be set synchronously. The work-around above fails to meet the requirements of this contract and provides no guarantee that it will be set by the time it's needed. For example, it will not be resolved by its use on this line:

apollo-server/packages/apollo-server-core/src/ApolloServer.ts

Line 317 in 26d6c73

schemaHash: this.schemaHash,

I'm happy to revisit this in the future but, as I've indicated in the commit message for #1955's 45bdcd9, the more correct solution here would be to maintain the synchronicity of introspection query execution by changing graphql-middleware to maintain the execution dynamics of the resolvers it is wrapping. I've made the error more clear via #1955, but I'm not currently willing to believe that an asynchronous introspection query is desired behavior and by working around this, we'd be encouraging that behavior.

[wmertens]

@abernix I'm encountering this too and I don't use graphql-middleware, I don't even have it in my node_modules.