https://kotlinlang.org logo
Title
g

gmariotti

12/25/2018, 9:24 AM
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)
unfortunately, I didn’t find anything in the release notes that would make explain this change
l

louiscad

12/25/2018, 11:13 AM
Missing a local
coroutineScope { … }
wrapping the code that can throw?
e

elizarov

12/25/2018, 11:19 AM
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()
g

gmariotti

12/26/2018, 9:36 AM
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?
e

elizarov

12/26/2018, 9:45 AM
This is done on purpose. The type and message of exception are preserved, so that your usual exception-handling logic can still work
g

gmariotti

12/26/2018, 10:17 AM
Makes sense, thanks for the explanation
p

pakoito

12/27/2018, 10:37 PM
@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
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?
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
e

elizarov

12/28/2018, 10:20 AM
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.
👍 1
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.
p

pakoito

12/28/2018, 9:47 PM
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.
Following @Vsevolod Tolstopyatov [JB] advice I have added a github issue to follow the discussion: https://github.com/Kotlin/kotlinx.coroutines/issues/921