CancellableContinuation doesn't ignore resumeWithException

[paulblessing]

The docs for CancellableContinuation state:

kotlinx.coroutines/common/kotlinx-coroutines-core-common/src/CancellableContinuation.kt

Lines 31 to 32 in 52bfdab

	* Invocation of [resume] or [resumeWithException] in _resumed_ state produces [IllegalStateException]
	* but is ignored in _cancelled_ state.

The part about "ignored in cancelled state" doesn't seem to be true in practice. Running the below code allows me to catch the JobCancellationException but still prints a stack trace of the IOException thrown.

fun main(args: Array<String>) {
  val client = OkHttpClient()
  val request = Request.Builder()
      .url("http://httpbin.org/get")
      .build()
  val call = client.newCall(request)

  try {
    val inResponse = CountDownLatch(1)
    val continueReading = CountDownLatch(1)

    val job = GlobalScope.launch(Dispatchers.Default) {
      val result: String = try {
        call.await {
          inResponse.countDown()
          continueReading.await()
          throw IOException("IO")
        }
      } catch (e: Exception) {
        "${e.javaClass} was caught"
      }

      println("result is [$result]")
    }

    inResponse.await()
    job.cancel()
    continueReading.countDown()

    Thread.sleep(5000)
  } finally {
    client.dispatcher().executorService().shutdown()
  }
}

suspend fun <T> Call.await(responseConverter: (Response) -> T): T {
  return suspendCancellableCoroutine { continuation ->
    continuation.invokeOnCancellation {
      this@await.cancel()
    }

    enqueue(object : Callback {
      override fun onResponse(call: Call, response: Response) {
        try {
          val result = response.use(responseConverter)
          continuation.resume(result)
        } catch (e: Exception) {
          println("Calling resumeWithException with ${e.javaClass}")
          continuation.resumeWithException(e)
        }
      }

      override fun onFailure(call: Call, e: IOException) {
        continuation.resumeWithException(e)
      }
    })
  }
}

Following the path from resumeWithException leads to this code in AbstractContinuation:

kotlinx.coroutines/common/kotlinx-coroutines-core-common/src/AbstractContinuation.kt

Lines 160 to 171 in 52bfdab

	is CancelledContinuation -> {
	/*
	* Continuation is complete, invoke directly.
	* NOTE: multiple invokeOnCancellation calls with different handlers are allowed on cancelled continuation.
	* It's inconsistent with running continuation, but currently, we have no mechanism to check
	* whether any handler was registered during continuation lifecycle without additional overhead.
	* This may be changed in the future.
	*
	* :KLUDGE: We have to invoke a handler in platform-specific way via `invokeIt` extension,
	* because we play type tricks on Kotlin/JS and handler is not necessarily a function there
	*/
	handler.invokeIt((state as? CompletedExceptionally)?.cause)

Are the docs incorrect? Am I doing something wrong? I see in https://github.com/gildor/kotlin-coroutines-okhttp/blob/master/src/main/kotlin/ru/gildor/coroutines/okhttp/CallAwait.kt#L23 they check cancellation before calling resumeWithException. Is that something that always needs to be done?

[elizarov]

The docs are incorrect. Whether it has to be done depends on on what kind of API you are integrating with. In case of Retrofit, it invoke callback when you cancel it and reports IOException, so this check is needed. We'll improve docs and we also plan to provide some better ways to do in the future.