Is this the best way to cancel a Deferred? Or is t...

# coroutines

[Ahmed Ibrahim]

Is this the best way to cancel a Deferred? Or is there a better way without the need to access the current `CoroutineContext`'s Job?

suspend <T> fun executeQuery(query: T) {
   val deferred = client.queryCall(query).toDeferred()

 currentCoroutineContext()[Job]?.invokeOnCompletion {
       if (it is CancellationException) {
          deferred.cancel(it)
       }
   }
   deferred.await()
}

[streetsofboston]

Not sure what queryDeferred does nor toDeferred, but it seems that executeQuery does not do anything in parallel. Just waits for the result of queryDeferred.

suspend fun <T> executeQuery(query: T) {
    return queryDeferred(query).toDeferred().await()
}

When the calling scope (CoroutineScope) of executeQuery is cancelled, the Deferred returned by toDeferred will be cancelled too, because of Structured Concurrency (depending on how queryDeferred and toDeferred are implemented….)

[Ahmed Ibrahim]

For the sake of context.
toDeferred is from Apollo's GraphQL coroutine support.
https://github.com/apollographql/apollo-android/blob/master/apollo-coroutines-support/src/main/kotlin/com/apollographql/apollo/coroutines/CoroutinesExtensions.kt#L82

[Ahmed Ibrahim]

Also executeQuery is generally executed inside a flatMapLatest call
e.g.

queryGenerator.flatMapLatest { query -> 
    flowOf(executeQuery(query))
}.collect { }

[Ahmed Ibrahim]

So what I want to do, is that I want to cancel in flight requests when flatMapLatest cancels the old emission to handle the new emission.

[gildor]

Wow, it's a very bad api, I wouldn't recommend you to use toDeferred, it breaks structured concurrency

[gildor]

And causing exactly this problem which you have

[gildor]

Honestly all those extensions are bad, except flow

[gildor]

They should provide await() function, it will not have all those problems

[gildor]

I would recommend report an issue, they should remove all of them :( and provide one single .await() function

[Ahmed Ibrahim]

Thanks for your answer! I'll probably do that, however since that bug will take sometime to get fixed, do you think I should use toFlow().first() or I should roll my own solution until it gets fixed?

[streetsofboston]

In our project we use this code:

private suspend fun <T> ApolloCall<T>.await(): Response<T> =
        suspendCancellableCoroutine { cont ->
            enqueue(
                object : Callback<T>() {
                    override fun onResponse(response: Response<T>) {
                        cont.resume(response)
                    }

                    override fun onFailure(e: ApolloException) {
                        cont.resumeWithException(e)
                    }
                }
            )
        }

basically, the await() function that Andrey was referring to.

[Ahmed Ibrahim]

@streetsofboston Do you mind if I mentioned you in the issue?

[Ahmed Ibrahim]

since I'm going to suggest your solution.

[streetsofboston]

No, I don’t mind.
Be aware that our await() implementation assumes that onResponse is called at most once! For a more general solution, you’d probably want to add some code around the cont.resume(response) to guard for this.

[Ahmed Ibrahim]

@streetsofboston I'm not sure I get this, ain't toDeferred is designed for a single response and the await solution is intended to replace it?
AFAIK we have toFlow() to handle multiple values.

[streetsofboston]

Depending on how the ApolloCall<T> is created, when enqueueing the Callback, its onResponse m_ay_ get called more than once. In our app, we know that will never happen.

But, in principle, the onResponse can be called more than once for a given `ApolloCall`… you’d need to guard against that, since cont.resume(value) can be called at most once.