ExpediaGroup/graphql-kotlin

Can not call `kotlinx.coroutines.future.await` on output from DataFetchingEnvironment::getValueFromDataLoader after updating to 7.1.x

terminalnode opened this issue · 2 comments

Library Version
Tried on 7.1.1 and 7.1.4. Worked previously on 7.0.2.

Describe the bug
kotlinx.coroutines.core has a function kotlinx.coroutines.future.await for converting CompletableFutures to coroutines. In previous versions of this library, calling await() on the output of DataFetchingEnvironment::getValueFromDataLoader was something we used extensively in our code, but now it causes all requests doing this to timeout.

To Reproduce
I found one part in our code where I can easily fix/break it with only this await differing between the two.

This is the working variant, which is not using await()

suspend fun stuff(
	env: DataFetchingEnvironment,
	pagination: PaginationOptionsInputGql? = null,
	filter: FilterInput? = null,
): CompletableFuture<MyConnectionGql> = env.getValueFromDataLoader<MyKey, MyConnectionGql>(
	ActivityByMyKeyDataLoader::class.simpleName!!,
	MyKey(pagination, filter),
)

This is the broken variant which causes timeouts:

import kotlinx.coroutines.future.await

suspend fun stuff(
	env: DataFetchingEnvironment,
	pagination: PaginationOptionsInputGql? = null,
	filter: FilterInput? = null,
): MyConnectionGql = env.getValueFromDataLoader<MyKey, MyConnectionGql>(
	ActivityByMyKeyDataLoader::class.simpleName!!,
	MyKey(pagination, filter),
).await()

The only difference here is removal of CompletableFuture in the return type and addition of .await()

Expected behavior
The two approaches should have the same effect.

please provide a repo link/zip file with repro steps.
Out of curiosity for your "working variant" why would you mark your function as suspend if you are not executing any suspendable function or launching a coroutine inside ?

The working variant only has the suspend marker to keep the breaking changes as small as possible. It's not required at all here, but highlights that only adding the .await() is enough to break it.

As for repo to reproduce, I'll see what I can do.