Kotlin Coroutines + OkHttp + Retrofit throw errors on 204 responses

[luiges90]

What kind of issue is this?

• Question. This issue tracker is not the place for questions. If you want to ask how to do
  something, or to understand why something isn't working the way you expect it to, use Stack
  Overflow. https://stackoverflow.com/questions/tagged/retrofit

• Bug report. If you’ve found a bug, spend the time to write a failing test. Bugs with tests
  get fixed. Here’s an example: https://gist.github.com/swankjesse/6608b4713ad80988cdc9

• Feature Request. Start by telling us what problem you’re trying to solve. Often a solution
  already exists! Don’t send pull requests to implement new features without first getting our
  support. Sometimes we leave features out on purpose to keep the project small.

Retrofit version: Tested on 2.7.0 and 2.9.0 and getting the same issue.

Expected: No error.

Actual:

Exception in thread "main" kotlin.KotlinNullPointerException: Response from com.example.retrofit_okhttp_kotlin.WebInterface.logout was null but response body type was declared as non-null
	at retrofit2.KotlinExtensions$await$2$2.onResponse(KotlinExtensions.kt:43)
	at retrofit2.OkHttpCall$1.onResponse(OkHttpCall.java:161)
	at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Reproduce code:

package com.example.retrofit_okhttp_kotlin

import kotlinx.coroutines.runBlocking
import okhttp3.OkHttpClient
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
import retrofit2.Retrofit
import retrofit2.http.GET

fun main() = runBlocking {
    val server = MockWebServer()
    server.enqueue(MockResponse().setResponseCode(204))
    server.start()

    val okhttp = OkHttpClient.Builder()
        .build()
    val retrofit = Retrofit.Builder()
        .baseUrl(server.url("/user/revoke/"))
        .client(okhttp)
        .build()
        .create(WebInterface::class.java)

    retrofit.logout()
}

interface WebInterface {
    @GET("user/revoke")
    suspend fun logout()
}

Other notes:

Tried all solutions in #1554 but no avail because in retrofit2.OkHttpCall#parseResponse it is hardcoded to return null body upon receiving 204 or 205 response codes and skipped any response converters that I would have added.

 Response<T> parseResponse(okhttp3.Response rawResponse) throws IOException {
    ResponseBody rawBody = rawResponse.body();

    // Remove the body's source (the only stateful object) so we can pass the response along.
    rawResponse = rawResponse.newBuilder()
        .body(new NoContentResponseBody(rawBody.contentType(), rawBody.contentLength()))
        .build();

    int code = rawResponse.code();
    if (code < 200 || code >= 300) {
      try {
        // Buffer the entire body to avoid future I/O.
        ResponseBody bufferedBody = Utils.buffer(rawBody);
        return Response.error(bufferedBody, rawResponse);
      } finally {
        rawBody.close();
      }
    }

    if (code == 204 || code == 205) { // <-- entered this branch
      rawBody.close();
      return Response.success(null, rawResponse); // <-- returns here
    }

    ExceptionCatchingResponseBody catchingBody = new ExceptionCatchingResponseBody(rawBody);
    try {
      T body = responseConverter.convert(catchingBody); // <-- All converters I would ever add would be here, which is never run in this case
      return Response.success(body, rawResponse);
    } catch (RuntimeException e) {
      // If the underlying source threw an exception, propagate that rather than indicating it was
      // a runtime exception.
      catchingBody.throwIfCaught();
      throw e;
    }
  }

This response is promptly sent to retrofit2.KotlinExtensions which hardcoded an error case with a null body.

override fun onResponse(call: Call<T>, response: Response<T>) {
        if (response.isSuccessful) {
          val body = response.body()
          if (body == null) { // <-- entered this branch
            val invocation = call.request().tag(Invocation::class.java)!!
            val method = invocation.method()
            val e = KotlinNullPointerException("Response from " +
                method.declaringClass.name +
                '.' +
                method.name +
                " was null but response body type was declared as non-null")
            continuation.resumeWithException(e) // <-- error thrown here
          } else {
            continuation.resume(body)
          }
        } else {
          continuation.resumeWithException(HttpException(response))
        }
      }

Also checked #2494 but the reporter was not using Kotlin
Update: May be related to #3075

[akka81]

No news about this issue?

[lukaszkalnik]

You might want to take a look at a Retrofit adapter for Either (currently in PR): arrow-kt/arrow#2621
It lets you define your API as returning Unit in case of success and then accepts null response body:

suspend fun postSomething(): Either<CallError, Unit> // accepts null response body

[victor-munoz]

Is using arrow-kt/arrow#2621 the only solution for this issue?

[lukaszkalnik]

Actually there is an even simpler workaround: change the return type of your API call to Response<Unit>:

import retrofit2.Response

interface WebInterface {

    @GET("user/revoke")
    suspend fun logout(): Response<Unit>
}

However in this case if you get an error response like 4xx or 5xx then your method won't throw an exception, as described in #3075 (comment)

[victor-munoz]

Thanks lukaszkalnik, Sadly, I need to perform specific actions in case of 4xx or 5xx responses, so do not throw an exception will be a problem.

Currently I am replacing the 204 code with a 200 code using a NetworkInterceptor to avoid trowing a KotlinNullPointerException

          OkHttpClient.Builder().addNetworkInterceptor { chain ->
                val request = chain.request()
                val response: Response = chain.proceed(request)
                if(response.code() == 204) response.newBuilder().code(200).build()
                else response
            }.build().run {
                Retrofit.Builder()
                    .baseUrl(YOUR_URL)
                    .addConverterFactory(GsonConverterFactory.create(GsonBuilder().create()))
                    .addCallAdapterFactory(CoroutineCallAdapterFactory())
                    .client(this)
                    .build()
                    .create(YourService::class.java)
            }

[mtwolak]

We had a production incident because of this bug. Sadly we also had to make some manual workarounds.