-
Notifications
You must be signed in to change notification settings - Fork 129
[Question] Coroutine cancellation is not handled, right? #7
Comments
Oh, it seems it's already WIP in the jakew/suspend/2018-02-16 branch, using Retrofit 2.4.0 (SNAPSHOT in the WIP latest commit), and it seems it'd handle cancellation thanks to the |
@LouisCAD can you give a usage example? |
@Bolein Cancellation support is first class in Kotlin coroutines. |
@LouisCAD I asked for an example usage of the CallbackAdapter in jakew/suspend/2018-02-16 branch. I don't really understand the purpose of CallbackAdapters and how it is meant to support cancellations. |
@Bolein The Without this, the best you can get in regards to coroutines support is what is in this library: https://github.com/gildor/kotlin-coroutines-retrofit |
@LouisCAD that is still not quite what I asked. However, I found an answer myself and feel responsible to share it with anyone interested. This repository contains an adapter that enables the use of coroutines with Retrofit methods. The master branch contains a simple Usage example: @GET("weather")
fun getWeather() : Deferred<WeatherResponse> There is another way of executing Retrofit requests by using a @GET("weather")
fun getWeather(callback : Callback<WeatherResponse>) This approach to execution is limited, because it does not provide a way to cancel the request. However, this approach has something in common with the implementation of suspending functions on JVM. @GET("weather")
suspend fun getWeather() : WeatherResponse It turns out, that functions marked as @GET("weather")
@Nullable
Object getWeather(@NotNull Continuation<WeatherResponse> var1);
This allows us to mark Retrofit methods as suspending functions and never explicitly @JakeWharton please correct me if I got anything wrong. |
Currently, I can't find any signs of the cancellation support. @JakeWharton what is the status of this feature? I believe that there is no way to be notified of the Job cancellation through the |
Don't these |
They should, but since no CoroutineContext is passed, and since there's no
suspension point in the API interface methods where it could be retrieved
from the continuation, there's no way to cancel in practice.
…On Sun, Aug 5, 2018, 8:05 AM Matej Drobnič ***@***.***> wrote:
Don't these invokeOnCompletion calls
<https://github.com/JakeWharton/retrofit2-kotlin-coroutines-adapter/blob/d41c3198a3e909041d7245683b3cc9bc230dd8f6/src/main/java/com/jakewharton/retrofit2/adapter/kotlin/coroutines/CoroutineCallAdapterFactory.kt#L91>
handle cancellation?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGpvBTvJg6R47y-uak9qPoku-Ke0EZ3sks5uNoshgaJpZM4SsAWu>
.
|
Doesn't context get passed when you call |
No because the coroutine that originated the call is already launched in
another CoroutineContext that you don't control, and if you parallelize
requests, awaiting for `await()` calls to make it cancellable would be too
late anyway.
…On Sun, Aug 5, 2018, 9:49 AM Matej Drobnič ***@***.***> wrote:
Doesn't context get passed when you call await() method?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGpvBTQJpINgbwruqNezVWxQOzn_ildvks5uNqOOgaJpZM4SsAWu>
.
|
This seems like a pretty serious issue. Now when coroutines getting non-experimental it would be great to continue with the work and get CallbackAdapter to (stable) retrotif (release) to allow proper implementation for suspend and also include this lib into the retrofit itself eventually. |
Won't fix. |
I'm sorry for digging this up but as for today does it mean that Retrofit and Coroutines are unusable? Basically if I start a network call with a coroutine and cancel it when its ongoing (AAC ViewModel is being cleared) the app crashes with an uncaught exception from the CallAdapter. Anything I can do or just wait for the first-hand suspend support release? Cheers |
The coroutineContext is not shared with the calling coroutine, right?
Did anyone find a way to wrap these calls so
cancel()
is called on the returnedDeferred
instance if the calling coroutine is cancelled?Also, @JakeWharton, I remember you talking about an idea on how to directly support
suspend
functions in Retrofit in a Tweet. Would it handle cancellation and cancel any in progress request?The text was updated successfully, but these errors were encountered: