JakeWharton/retrofit2-kotlin-coroutines-adapter

Is Deferred<Unit> supported?

Closed this issue · 16 comments

LouisCAD commented

I recall a snippet of a UnitConverterFactory you posted in an issue. Is is needed to support Deferred<Unit> for 204 No Content or ignored body responses?

JakeWharton commented

Yes. This is only a call adapter. If you want a converter for Unit as a body type, you'll need to install one separately.

LouisCAD commented

@JakeWharton Could be nice to post the snippet in the README since it's likely to be needed by users of this library

JakeWharton commented
  • 👍1
PaulWoitaschek commented

I don't understand why this is not provided out of the box. Or am I misunderstanding something?
When I have a POST or PATCH isn't Deferred<Unit> always what I want to return here?
When using retrofit directly I use a Call<Void>

  • 👍1
JakeWharton commented

This is an adapter which provides you Deferred<T>. If you want to use Unit for T, you want a converter.

PaulWoitaschek commented

And how is that possible? I try to write a moshi adapter like

class UnitJsonAdapter {

  @FromJson
  fun fromJson(value: String?): Unit = Unit

  @ToJson
  fun toJson(unit: Unit): String? = null
}

But it fails because the fromJson uses void in bytecode.

JakeWharton commented
PaulWoitaschek commented

Thanks! Also I ask myself if there is no better representation for that. Like RxJava has Completable. Maybe just allow returning a plain Job?

JakeWharton commented
PaulWoitaschek commented

With that Unit call adapter I have exceptions for Deferred<Unit> (which are printed to std?)

W/System.err: kotlin.KotlinNullPointerException
W/System.err:     at com.jakewharton.retrofit2.adapter.kotlin.coroutines.experimental.CoroutineCallAdapterFactory$BodyCallAdapter$adapt$2.onResponse(CoroutineCallAdapterFactory.kt:102)
        at retrofit2.OkHttpCall$1.onResponse(OkHttpCall.java:123)
W/System.err:     at okhttp3.RealCall$AsyncCall.execute(RealCall.java:153)
W/System.err:     at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
W/System.err:     at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
W/System.err:     at java.lang.Thread.run(Thread.java:764)
  • 👍6
LouisCAD commented

@PaulWoitaschek Try adding the UnitConverterFactory before other converter factories when you configure your Retrofit.

quentin41500 commented

I'm getting the same error @PaulWoitaschek, how did you fix it?

@LouisCAD, it is currently the first one on the list.

I'm just confused if I even still need the call adapter.

PaulWoitaschek commented

I fixed this by checking for the Unit type:

        override fun onResponse(call: Call<T>, response: Response<T>) {
          if (response.isSuccessful) {
            if (responseType == Unit::class.java) {
              @Suppress("UNCHECKED_CAST")
              deferred.complete(Unit as T)
            } else {
              deferred.complete(response.body()!!)
            }
          } else {
            deferred.completeExceptionally(HttpException(response))
          }
        }

@JakeWharton Will you accept a PR on this?

ashdavies commented

I'm getting the same error output as @PaulWoitaschek, with Retrofit 2.5.0, and including UnitConverterFactory before any others, using Deferred<Response<Unit>> alleviates it.

Is this still an issue? or is it just an incorrect configuration?

roelvandun commented

Also it would help debugging to write to Log.e instead of Log.w

branoduris commented

I'm getting the same error, with build in UnitResponseBodyConverter in Retrofit 2.6.0. I used @PaulWoitaschek fix on response function.