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

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

Open
terminalnode opened this issue Jul 15, 2024 · 2 comments
Labels
type: bug Something isn't working

Comments

@terminalnode
Copy link

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.

@terminalnode terminalnode added the type: bug Something isn't working label Jul 15, 2024
@samuelAndalon
Copy link
Contributor

samuelAndalon commented Jul 15, 2024

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 ?

@terminalnode
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Development

No branches or pull requests

2 participants