First of all, Merry Christmas everyone :smile: !! ...

# coroutines

[gmariotti]

First of all, Merry Christmas everyone 😄 !! Quick question, did something change in Coroutines 1.1.0 in how exceptions are propagated? Because in the kotlin module for Vert.x, there’s a test like this

@Test
  fun `test failure event method`(testContext: TestContext) {
    GlobalScope.launch(vertx.dispatcher()) {
      val async = testContext.async()
      val cause = RuntimeException()
      try {
        awaitEvent<Any> { throw cause }
        testContext.fail()
      } catch (e: Exception) {
        testContext.assertEquals(cause, e)
      }
      async.complete()
    }
  }

with awaitEvent implemented in this way:

suspend fun <T> awaitEvent(block: (h: Handler<T>) -> Unit): T {
  return suspendCancellableCoroutine { cont: CancellableContinuation<T> ->
    try {
      block.invoke(Handler { t ->
        cont.resume(t)
      })
    } catch (e: Exception) {
      cont.resumeWithException(e)
    }
  }
}

and now this test pass only if I do testContext.assertEquals(cause, e.cause)

[gmariotti]

unfortunately, I didn’t find anything in the release notes that would make explain this change

[louiscad]

Missing a local coroutineScope { … } wrapping the code that can throw?

[elizarov]

This might be related to stack-trace recovery — it clones & recreates exceptions to add augmented stacktraces to them, so that is why equals check on exception can now fail. You can verify that by printing both original and caught exceptions with e.printStackTrace()

[gmariotti]

I'll have to debug it and check the stacktrace, thanks for the explanation @elizarov. Is it just a coincidence that the exception that I receive is still a RuntimeException, like the one I'm throwing, or is it done on purpose?

[elizarov]

This is done on purpose. The type and message of exception are preserved, so that your usual exception-handling logic can still work

[gmariotti]

Makes sense, thanks for the explanation

[pakoito]

@elizarov is this behavior documented somewhere? breaking identity of exceptions has caused issues in 2 open source libraries so far, many more closed source ones will be affected too

[pakoito]

The type and message of exception are preserved, so that your usual exception-handling logic can still work

Our logic was based off referential comparison, which is an approach other concurrency libraries respect. Comparing by type and message string provides weaker assurances about identity, on top of a perf hit.

Is this change strictly necessary for the library, or could it be triggered by a configuration flag, or any alternative solution?

[pakoito]

Here’s a similar discussion on Scala/cats: https://github.com/typelevel/cats-effect/pull/449 which in turn caused a revert in their approach: https://github.com/typelevel/cats-effect/pull/458

[elizarov]

Comparing by type and message string provides weaker assurances about identity, on top of a perf hit.

This is not true and, unfortunately, cannot be true. Exceptions on JVM are mutable and this prevents their reuse in concurrent setting. We use the similar logic of copying exceptions to that in the JVM’s CompletableFuture implementation.

[elizarov]

That is the only way to augment exceptions in concurrency setting. The goal of having human-readable exceptions of ease of debugging trumps all other considerations.

[pakoito]

Saying that you’re copying CompletableFuture doesn’t put me more at ease, there’s a reason why we’re reimplementing it at every turn hahahaha

The goal of having human-readable exceptions of ease of debugging trumps all other considerations.

I’m for that goal too, I’m just concerned about the implementation and I don’t think it’s the only possible option. I have several concerns: firstly, wouldn’t it hinder debuggability the fact that debug and production have different stacktraces? Secondly, in other concurrent frameworks like the ones mentioned above they’re using other APIs like Throwable#addSuppressed that don’t break referential equality and make this a problem only in the case where there are concurrent exceptions; instead of all of them. We have had the same problem when writing parallelMap and races for IO in Arrow, and that’s the approach we are currently considering.

[pakoito]

Following @Vsevolod Tolstopyatov [JB] advice I have added a github issue to follow the discussion: https://github.com/Kotlin/kotlinx.coroutines/issues/921