Add first-party Kotlin coroutine suspend support #2886

JakeWharton · 2018-09-12T03:37:09Z

No description provided.

pom.xml

swankjesse · 2018-09-16T14:57:37Z

retrofit/src/main/java/retrofit2/HttpServiceMethod.java

+    if (requestFactory.isKotlinSuspendFunction) {
+      Type[] parameterTypes = method.getGenericParameterTypes();
+      Type continuationType = parameterTypes[parameterTypes.length - 1];
+      Type innerType = Utils.getParameterLowerBound(0, (ParameterizedType) continuationType);


I don't love this name, just cause inner often means nested and not parameter. Maybe call it asyncResponseType?

Fixed by just using responseType which cleaned this section up a bit

swankjesse · 2018-09-16T15:01:12Z

retrofit/src/main/java/retrofit2/HttpServiceMethod.java

@@ -32,8 +35,28 @@
   */
  static <ResponseT, ReturnT> HttpServiceMethod<ResponseT, ReturnT> parseAnnotations(
      Retrofit retrofit, Method method, RequestFactory requestFactory) {
-    CallAdapter<ResponseT, ReturnT> callAdapter = createCallAdapter(retrofit, method);
-    Type responseType = callAdapter.responseType();
+    CallAdapter<ResponseT, ReturnT> callAdapter = null;


Just a wrinkle: asymmetry on how this is set up feels awkward. No suggestions on how to do it better.

Yeah I'm willing to muck up the internals instead of inventing some elaborate abstraction that will only be used by coroutines. If we ever need to generalize we can, but I don't want to jump that gun.

I've refined this a bit by using subtypes. This method is still gnarly because of all the shared logic, but the created classes are small and focused and don't involve crazy conditionals

This code actually reminds me of Retrofit internals circa 2012-2013... it can be cleaned up it's just a matter of effort but since it's isolated here for now I'm not worried about it.

swankjesse · 2018-09-16T15:03:38Z

retrofit/src/main/java/retrofit2/HttpServiceMethod.java

+    Object continuation = args[args.length - 1];
+    if (continuationWantsResponse) {
+      //noinspection unchecked Guaranteed by parseAnnotations above.
+      return (ReturnT) KotlinExtensions.awaitResponse(call,


This function name looks like it’ll be blocking but my assumption is that it'll return a promise. I gotta read more.

It actually returns a union type of COROUTINE_SUSPENDED | T which is either a marker value indication suspension to unwind the stack or the result if it was able to be computed synchronously. Since we're invoking Call.enqueue it will always return COROUTINE_SUSPENDED.

swankjesse · 2018-09-16T15:05:07Z

retrofit/src/main/java/retrofit2/KotlinExtensions.kt

 inline fun <reified T> Retrofit.create(): T = create(T::class.java)
+
+suspend fun <T : Any> Call<T>.await(): T {


This is neat.

And public API for people to use if they already have Calls! Makes it a win-win really.

swankjesse · 2018-09-16T15:07:26Z

retrofit/src/main/java/retrofit2/KotlinExtensions.kt

+    enqueue(object : Callback<T> {
+      override fun onResponse(call: Call<T>, response: Response<T>) {
+        if (response.isSuccessful) {
+          // TODO handle nullability


Presumably the callers T can itself be nullable or not – like the parameter will be String or String? but to this code it's all just T.

Which is a real problem in practice since an endpoint declared as suspend fun foo(): String can get an HTTP 204 which produces a null body and leaks that into an otherwise non-null type.

We need something like a hand-written Kotlin metadata parser for return type nullability that doesn't pull in any deps or support anything except looking for exactly what we need. That way it's 1 method and not 10,000.

swankjesse · 2018-09-16T15:11:46Z

retrofit/src/main/java/retrofit2/RequestFactory.java

@@ -98,6 +101,10 @@ static RequestFactory parseAnnotations(Retrofit retrofit, Method method) {
          + ") doesn't match expected count (" + handlers.length + ")");
    }

+    if (isKotlinSuspendFunction) {
+      // The Continuation is the last parameter and the handlers array contains null at that index.


Wondering aloud: could you symmetrically synthesize the return type from this argument? It’s how the coroutine and non-coroutine flows could fit together.

interface ResponseReturner<>T { void success (Response<T> response); void failure (Throwable t); }

I guess where I'm going is to imagine coroutines is actually a more general case and we can model regular returns within the coroutine machinery. Though this would only be for API symmetry.

What you're describing sounds like the callback factory that I originally modeled. It seems like a lot of work for something I expect no one except coroutines to use.

Works for me! None of it leaks into the public API anyway so it doesn’t much matter.

One other way of potentially looking at this:

The coroutines support synthesizes a return type of Call<T> or Call<Response<T>> and just calls await() for you

swankjesse · 2018-09-16T15:14:17Z

retrofit/src/main/java/retrofit2/RequestFactory.java

+              isKotlinSuspendFunction = true;
+              return null;
+            }
+          } catch (NoClassDefFoundError ignored) {


retrofit/src/main/java/retrofit2/Utils.java

swankjesse · 2018-09-16T15:16:16Z

retrofit/src/test/java/retrofit2/KotlinSuspendRawTest.java

+
+/**
+ * This code path can only be tested from Java because Kotlin does not allow you specify a raw
+ * Response type. Win! We still test this codepath for completeness.


swankjesse

This is wonderful.

Is there an easy way to test that Retrofit works without the optional dependencies? Would help to defend against accidental introductions of Kotlin dependencies.

swankjesse · 2018-09-16T15:19:38Z

retrofit/src/test/java/retrofit2/KotlinSuspendTest.kt

+
+    server.enqueue(MockResponse().setResponseCode(404))
+
+    try {


kotlin.test.assertFailsWith !

Requires an extra dependency. Maybe i'll upgrade to JUnit 4.13 beta in another PR which has assertThrows

retrofit/src/test/java/retrofit2/KotlinSuspendTest.kt

JakeWharton · 2018-09-16T17:33:34Z

The optional dependencies are implicitly tested by the other modules which do not include them. For example, if the instanceof check for Continuation wasn't behind a try/catch you would see it throw NCDFE in all of the converter and adapter code.

swankjesse · 2018-09-16T17:56:32Z

Thumbs up. The implicit coverage is good but I still sort of want to add something small and explicit – a test that asserts Kotlin isn't on the classpath and then does some Retrofit stuff. That way if/when we add indirect Kotlin dependencies (such as via Okio 2!) then we have something to catch if we regress.

codefromthecrypt

fantastic

FabianTerhorst · 2018-10-29T16:10:50Z

kotlin 1.3 is now released with coroutines 1.0

JakeWharton · 2018-10-29T16:18:47Z

Yes. It will still be a few weeks before this lands, but it's at least unblocked.

matejdro · 2018-11-14T07:02:12Z

From what I see, this MR completely bypasses call adapters when using suspend functions.

I'm currently using custom call adapters for centralising parsing of error body (and then throwing appropriate exceptions), smilarly to the official retrofit2 sample.

Any chance we get alternative to this, some kind of adapter that is injected between here?

retrofit/src/main/java/retrofit2/KotlinExtensions.kt

cbruegg · 2018-11-14T19:27:23Z

Would it be possible to implement this in such a way that stacktraces are properly augmented with actual call sites? There is WIP being done on this in kotlinx.coroutines: Kotlin/kotlinx.coroutines#792

This would apparently require using either of async, launch, coroutineScope, supervisorScope, withContext. See also this motivation.

retrofit/src/main/java/retrofit2/HttpServiceMethod.java

swankjesse · 2019-02-15T19:46:43Z

retrofit/src/main/java/retrofit2/KotlinExtensions.kt

+suspend fun <T : Any> Call<T>.await(): T {
+  return suspendCancellableCoroutine { continuation ->
+    continuation.invokeOnCancellation {
+      cancel()


Should probably test this. Can do in follow-up.

retrofit/src/test/java/retrofit2/KotlinSuspendTest.kt

TurKurT656 · 2019-02-27T09:57:21Z

fantastic work. so do we need to add a call adapter for this to work? i didn't see any documentation about this.

JakeWharton · 2019-02-27T13:44:52Z

No

…

On Wed, Feb 27, 2019 at 4:57 AM Saman Sattari ***@***.***> wrote: fantastic work. so do we need to add a call adapter for this to work? i didn't see any documentation about this. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#2886 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEUtmrsyMoFaVJHR_vBVwHrzJQBOFks5vRlaDgaJpZM4Wkl1S> .

rodion-m · 2019-02-27T13:50:13Z

JakeWharton, many thanks for your great work! May you help us how to implement suspend in retrofit?

droidluv · 2019-03-17T22:03:52Z

I am not exactly sure what to do here, because cancelling a deferred job of retrofit doesn't cancel the network call, the api call seems to move to completion whether I cancelled it or not, is there a newer version of retrofit I can use which has this feature? Currently I am on retrofit 2.5.0.

bradynpoulsen · 2019-03-17T22:42:19Z

@droidluv this seems unrelated because 2.5.0 doesn't contain this PR. This PR is available on master (2.5.1-SNAPSHOT). Perhaps open new issues for any problems you experience

droidluv · 2019-03-18T00:07:24Z

@bradynpoulsen I apologise if I was unclear and vague, I was brought here from here, I realised the fix was on 2.5.1-SNAPSHOT
I have my gradle configured for
maven { url "https://oss.sonatype.org/content/repositories/snapshots" }
and has com.squareup.retrofit2:retrofit:2.5.1-SNAPSHOT as my dependency but it fails to resolve, and I am unsure on how to move forward

Draptol · 2019-03-21T00:49:10Z

I have a question about retrofit 2.5.1-SNAPSHOT and canceling request. I downloaded this version of retrofit from the repository
"https://oss.sonatype.org/content/repositories/snapshots" and my project was set up properly. Next, I tried to implement suspending method or await call like in this tutorial but after canceling parent job nothing happens. My request didn’t cancel and I got a response (I checked logs while the application was working). For tests, I use https://httpstat.us/ with delay after which I am getting the response from a server. So my question is how to implement canceling the request in the new retrofit version (which should support cancellation properly ). Could you give me any code snippet? In my scenario, I have a few requests which are running simultaneously using async and I need to cancel all of them by canceling the parent job.

JakeWharton · 2019-03-21T00:53:04Z

A cancelation test was added in #3029 which verifies that canceling the coroutine will propagate to the underlying Call.

Draptol · 2019-03-21T11:46:30Z

@JakeWharton I saw this test.
Maybe I am doing something wrong, but I have a problem with this cancellation.

Here you have my project's environment.

Gradle:

 // Retrofit 2.0
    implementation 'com.squareup.retrofit2:retrofit:2.5.1-SNAPSHOT'
    implementation 'com.squareup.retrofit2:converter-gson:2.5.1-SNAPSHOT'
    implementation 'com.squareup.okhttp3:logging-interceptor:3.11.0'
    implementation 'com.squareup.okhttp3:okhttp:3.12.0'

Routes:

interface Routes {
    @GET("/200?sleep=5000")
    suspend fun test(): ResponseBody

    @GET("/200?sleep=5000")
    fun test2(): Call<ResponseBody>
}

Server:

class NetworkService() {
    val routes: Routes

    init {
        val gson = GsonBuilder().setDateFormat(DateFormat.LONG).create()
        val retrofit = Retrofit.Builder()
            .addConverterFactory(GsonConverterFactory.create(gson))
            .client(
                createOkHttpClient(
                    connectionTimeout = 10 * 1000,
                    readTimeout = 10 * 1000,
                    writeTimeout = 10 * 1000
                )
            )
            .baseUrl("https://httpstat.us")
            .build()
        routes = retrofit.create(Routes::class.java)
    }

    private fun createOkHttpClient(
        connectionTimeout: Long,
        readTimeout: Long,
        writeTimeout: Long
    ): OkHttpClient {
        val debugInterceptor = HttpLoggingInterceptor().apply {
            level = HttpLoggingInterceptor.Level.BODY
        }

        return OkHttpClient.Builder()
            .addInterceptor(debugInterceptor)
            .connectTimeout(connectionTimeout, TimeUnit.MILLISECONDS)
            .readTimeout(readTimeout, TimeUnit.MILLISECONDS)
            .writeTimeout(writeTimeout, TimeUnit.MILLISECONDS)
            .build()
    }
}

Main Activity:

override fun onCreate(savedInstanceState: Bundle?) {
       super.onCreate(savedInstanceState)
       setContentView(R.layout.activity_main)

       val routes = NetworkService().routes
       runBlocking {
           val job = GlobalScope.launch {
               Log.d("NETWORK_DEBUG_TAG", "start request")
               routes.test()
           }
           Log.d("NETWORK_DEBUG_TAG", "before delay")
           delay(1500)
           Log.d("NETWORK_DEBUG_TAG", "cancel")
           job.cancel()
           Log.d("NETWORK_DEBUG_TAG", "job isCancelled = ${job.isCancelled}")
           job.join()
           Log.d("NETWORK_DEBUG_TAG", "after join")
       }
   }

After invoking above code I got these logs:

2019-03-21 11:46:50.665 21884-21884/com.test.testapp D/NETWORK_DEBUG_TAG: before delay
2019-03-21 11:46:50.666 21884-22069/com.test.testapp D/NETWORK_DEBUG_TAG: start request
2019-03-21 11:46:50.717 21884-22072/com.test.testapp D/OkHttp: --> GET https://httpstat.us/200?sleep=5000
2019-03-21 11:46:50.717 21884-22072/com.test.testapp D/OkHttp: --> END GET
2019-03-21 11:46:52.170 21884-21884/com.test.testapp D/NETWORK_DEBUG_TAG: cancel
2019-03-21 11:46:52.171 21884-21884/com.test.testapp D/NETWORK_DEBUG_TAG: job isCancelled = true
2019-03-21 11:46:57.102 21884-22072/com.test.testapp D/OkHttp: <-- 200 OK https://httpstat.us/200?sleep=5000 (6383ms)
2019-03-21 11:46:57.103 21884-22072/com.test.testapp D/OkHttp: Cache-Control: private
2019-03-21 11:46:57.103 21884-22072/com.test.testapp D/OkHttp: Server: Microsoft-IIS/10.0
2019-03-21 11:46:57.103 21884-22072/com.test.testapp D/OkHttp: X-AspNetMvc-Version: 5.1
2019-03-21 11:46:57.103 21884-22072/com.test.testapp D/OkHttp: Access-Control-Allow-Origin: *
2019-03-21 11:46:57.104 21884-22072/com.test.testapp D/OkHttp: X-AspNet-Version: 4.0.30319
2019-03-21 11:46:57.104 21884-22072/com.test.testapp D/OkHttp: X-Powered-By: ASP.NET
2019-03-21 11:46:57.104 21884-22072/com.test.testapp D/OkHttp: Set-Cookie: ARRAffinity=beec3692495883b8df6195e900c12f49514e054d865a22ad2951c84f51dbaf93;Path=/;HttpOnly;Domain=httpstat.us
2019-03-21 11:46:57.104 21884-22072/com.test.testapp D/OkHttp: Date: Thu, 21 Mar 2019 10:46:56 GMT
2019-03-21 11:46:57.106 21884-22072/com.test.testapp D/OkHttp: Content-Length: 0
2019-03-21 11:46:57.108 21884-22072/com.test.testapp D/OkHttp: <-- END HTTP (0-byte body)
2019-03-21 11:46:57.121 21884-21884/com.test.testapp D/NETWORK_DEBUG_TAG: after join

Test scenario:
Firstly, I make request to server which is to respond after 5 seconds (look at query param). After 1,5 second I cancel the job so I assume that the call would be Immediately cancelled. But it doesn't heppen. I look at logs and I see that request was completed although I had cancelled the job after 1,5 second.

So could you tell me what could be the problem?

Jesus13 · 2019-03-21T13:24:28Z

Hey, I want to use the suspend function in my production ready project, but the suspend function is not yet in the stable branch. When do you plan to release 2.5.1? What issues are blocking the release?

wlara · 2019-04-28T16:22:06Z

@JakeWharton Any update on when should we expect a stable release of this feature?

JakeWharton · 2019-04-28T16:24:05Z

Nope. Releases aren't done based on dates.

…

On Sun, Apr 28, 2019, 12:22 PM wlara ***@***.***> wrote: @JakeWharton <https://github.com/JakeWharton> Any update on when should we expect a stable release of this feature? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2886 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAQIEJC5ROJZC3CUC4NTT3PSXFLBANCNFSM4FUSLVJA> .

comm1x · 2019-04-28T22:23:54Z

I noticed, that in 2.5.1 in cases with suspend CallAdapters does not calling. ConverterFactory calls directly. So if you want to use your custom CallAdapters, for example for Result:

suspend getUser(): Result<User>

your code will fails with message, that your ConverterFactory cannot deserialize Result object (without creating any CallAdapter).

JakeWharton · 2019-04-28T22:39:21Z

Result is an RxJava adapter concept for now

…

On Sun, Apr 28, 2019, 6:23 PM Pavel Shorokhov ***@***.***> wrote: I also noticed, that in 2.5.1 in cases with suspend CallAdapters does not calling. ConverterFactory calls directly. So if you want to use you custom CallAdapters, for example for Result: suspend getUser(): Result<User> your code will fails with message, that your ConverterFactory cannot deserialize Result object (without creating any CallAdapter). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2886 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAQIEOLJBKKXHOLAE2NWGDPSYPXZANCNFSM4FUSLVJA> .

onraz · 2019-05-12T19:58:29Z

Thanks @JakeWharton for this awesome work! Would you consider a milestone release? It may promote more testing and inclusion in some non-critical projects.

JakeWharton force-pushed the jakew/suspend/2018-09-11 branch from 2a7db7a to 3fd655b Compare September 13, 2018 17:02

swankjesse approved these changes Sep 16, 2018

View reviewed changes

JakeWharton force-pushed the jakew/suspend/2018-09-11 branch 2 times, most recently from d325a9e to 77af488 Compare September 16, 2018 17:55

JakeWharton force-pushed the jakew/suspend/2018-09-11 branch from 77af488 to 0692026 Compare September 16, 2018 18:09

This was referenced Sep 21, 2018

New 'Invocation' class for tracking metrics on Retrofit calls #2899

Merged

Expose static factory for Invocation #2902

Merged

JakeWharton force-pushed the jakew/suspend/2018-09-11 branch 3 times, most recently from 3486d8f to c395666 Compare September 24, 2018 15:02

vadymhimself approved these changes Oct 19, 2018

View reviewed changes

codefromthecrypt reviewed Oct 20, 2018

View reviewed changes

zsmb13 mentioned this pull request Oct 29, 2018

Update to Kotlin 1.3.11 and coroutines 1.0.1 JakeWharton/retrofit2-kotlin-coroutines-adapter#34

Open

LouisCAD mentioned this pull request Oct 31, 2018

Update to Kotlin 1.3 and use stdlib Result class gildor/kotlin-coroutines-retrofit#45

Closed

louis993546 mentioned this pull request Nov 11, 2018

Use first party retrofit + coroutine instead of adapter louis993546/Loft-Android#29

Open

codefromthecrypt mentioned this pull request Nov 14, 2018

Kotlin coroutine context propagation openzipkin/brave#820

Open

plastiv reviewed Nov 14, 2018

View reviewed changes

retrofit/src/main/java/retrofit2/KotlinExtensions.kt Show resolved Hide resolved

nenick reviewed Jan 12, 2019

View reviewed changes

retrofit/src/main/java/retrofit2/HttpServiceMethod.java Show resolved Hide resolved

JakeWharton mentioned this pull request Jan 23, 2019

Retrofit 2.5 Android build error: Task transformClassesWithAndroidGradleClassShrinkerForRelease failed #2992

Open

JakeWharton force-pushed the jakew/suspend/2018-09-11 branch 2 times, most recently from 5f161d6 to 09c8f2c Compare February 15, 2019 18:00

swankjesse approved these changes Feb 15, 2019

View reviewed changes

Add first-party Kotlin coroutine suspend support

b761518

JakeWharton force-pushed the jakew/suspend/2018-09-11 branch from 09c8f2c to b761518 Compare February 15, 2019 20:35

JakeWharton merged commit 3ee4950 into master Feb 15, 2019

JakeWharton deleted the jakew/suspend/2018-09-11 branch February 15, 2019 20:46

matejdro mentioned this pull request Feb 27, 2019

Ability to add custom adapter for suspend methods #3037

Closed

mag0716 mentioned this pull request Mar 6, 2019

[Retrofit] 2.5.1-SNAPSHOT mag0716/Android#85

Merged

Draptol mentioned this pull request Mar 24, 2019

Snapshot doesn't cancel call after cancelling coroutine #3055

Closed

JakeWharton mentioned this pull request Apr 1, 2019

[Feature request] first class support for suspend functions #3060

Closed

Dong-wan referenced this pull request in OmarBradley/MovieSearchApplication May 20, 2019

기능 완료, 아직 Spek / KotlinTest 로 되어진 UnitTest는 나중에 작성해야함

c46cbef

jzbrooks mentioned this pull request Aug 5, 2024

Consider interacting with the network off the calling thread connectrpc/connect-kotlin#313

Closed

		inline fun <reified T> Retrofit.create(): T = create(T::class.java)

		suspend fun <T : Any> Call<T>.await(): T {

Add first-party Kotlin coroutine suspend support #2886

Add first-party Kotlin coroutine suspend support #2886

Conversation

JakeWharton commented Sep 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swankjesse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JakeWharton commented Sep 16, 2018

swankjesse commented Sep 16, 2018

codefromthecrypt left a comment

Choose a reason for hiding this comment

FabianTerhorst commented Oct 29, 2018

JakeWharton commented Oct 29, 2018

matejdro commented Nov 14, 2018 • edited Loading

cbruegg commented Nov 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurKurT656 commented Feb 27, 2019

JakeWharton commented Feb 27, 2019 via email

rodion-m commented Feb 27, 2019 • edited Loading

droidluv commented Mar 17, 2019

bradynpoulsen commented Mar 17, 2019

droidluv commented Mar 18, 2019

Draptol commented Mar 21, 2019

JakeWharton commented Mar 21, 2019

Draptol commented Mar 21, 2019

Jesus13 commented Mar 21, 2019

wlara commented Apr 28, 2019

JakeWharton commented Apr 28, 2019 via email

comm1x commented Apr 28, 2019 • edited Loading

JakeWharton commented Apr 28, 2019 via email

onraz commented May 12, 2019

JakeWharton commented Sep 12, 2018 •

edited

Loading

matejdro commented Nov 14, 2018 •

edited

Loading

rodion-m commented Feb 27, 2019 •

edited

Loading

comm1x commented Apr 28, 2019 •

edited

Loading