Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsider and/or better document handling of exceptions thrown by await() #3937

Closed
OliverO2 opened this issue Nov 10, 2023 · 6 comments
Closed
Labels
docs KDoc and API reference

Comments

@OliverO2
Copy link
Contributor

An exception thrown by an await() call can be caught, but will then be re-thrown again in the enclosing coroutine scope, apparently from out of nowhere. Re-throwing an exception which has just been caught breaks normal exception behavior and contradicts user expectations.

Example:

import kotlinx.coroutines.*

private class TestException(override val message: String) : Throwable()

fun main(): Unit = runBlocking {
    suspend fun reportCatching(name: String, block: suspend () -> Unit) {
        try {
            block()
            println("$name: block finished without exception")
        } catch (throwable: Throwable) {
            println("$name: block threw $throwable")
            throwable.cause?.let { println("   cause: ${throwable.cause}") }
        }
    }

    reportCatching("outer") {
        coroutineScope {
            reportCatching("inner, wrapped in coroutineScope") {
                coroutineScope {
                    async {
                        throw TestException("boom 1")
                    }.await()
                }
            }

            reportCatching("inner, bare") {
                async {
                    throw TestException("boom 2")
                }.await()
            }
        }
    }
}

This would print:

inner, wrapped in coroutineScope: block threw TestException: boom 1
inner, bare: block threw TestException: boom 2
outer: block threw TestException: boom 2

While this issue has been raised before (in #2147), I don't think it has been properly addressed.

Currently, correct usage would rely on users being aware of the implications regarding structured concurrency, the Job hierarchy and the interwoven timing between launching and completion of jobs. In particular, the following snippet from the section on Cancellation and exceptions could be interpreted as implying the above behavior (emphasis added):

If a coroutine encounters an exception other than CancellationException, it cancels its parent with that exception. This behaviour cannot be overridden

However, the async/await situation is not mentioned and there is no explanation of the underlying mechanism and its effect on await(). While with normal coroutine operations, exception handling is consistent with sequential operations, in case of await() the situation causes misinterpretations and errors.

Discussion: https://kotlinlang.slack.com/archives/C1CFAFJSK/p1696873014170159

Two solutions seem possible:

  1. Make an await() call throw a special exception like DeferredResultUnavailable, wrapping the original one.
  2. Let the docs clearly state that catching an exception from an await() call is discouraged.

In any case, the docs should explicitly mention this case.

@dkhalanskyjb dkhalanskyjb added docs KDoc and API reference and removed design labels Nov 14, 2023
@dkhalanskyjb
Copy link
Contributor

I don't think I understand the problem or the solutions. Let's first make sure we're on the same page.

https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/async.html states:

it cancels the parent job (or outer scope) on failure to enforce structured concurrency paradigm. To change that behaviour, supervising parent (SupervisorJob or supervisorScope) can be used.

https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-deferred/await.html states:

returning the resulting value or throwing the corresponding exception if the deferred was cancelled.

and

This suspending function is cancellable. If the Job of the current coroutine is cancelled or completed while this suspending function is waiting, this function immediately resumes with CancellationException.

On async { throw TestException("") }.await(), two things happen:

  • The Deferred gets cancelled with a TestException;
  • Because Deferred got cancelled, the parent also gets cancelled to ensure structured concurrency.

This behaviour can also be easily observed like this:

fun main() = runTest {
    async { throw TestException() }
    try {
        delay(10.milliseconds) // will throw an exception, because the parent coroutine got cancelled in the meantime
    } catch (e: Exception) {
        println("delay threw")
    }
}

Semantically, you can think of async as a launch that returns not just a Job but also a value. Or you can think of launch with its job.join() as an async that returns a Unit.

Occasionally, the Deferred is used in a scope other than the one it was created in. A simplified example (pseudocode):

class ApiAccessor(scope: CoroutineScope) {
  fun getUser(id: Int): Deferred<User> = scope.async {
    if (!checkId(id)) throw IllegalStateException("wrong id")
    downloadUserData(id)
  }
}

fun main() {
  val apiScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
  val api = ApiAccessor(apiScope)
  runBlocking {
    launch {
      delay(1.seconds)
      api.cancel() // disable the API access after one second of work
    }
    repeat(1000) {
      val userDeferred = api.getUser(1000)
      val user = runCatching { userDeferred.await() } // waits for the user, failure does not cancel the scope
    }
  }
}

The link to the discussion doesn't open for me for some reason, unfortunately.

Does this answer your concerns?

@OliverO2
Copy link
Contributor Author

Here's a link to the Slack discussion on Linen: https://slack-chats.kotlinlang.org/t/15971244/after-catching-an-async-call-s-exception-why-does-it-re-thro

First, I agree with everything you wrote. The differences are in the details which are implied in the docs rather than explicitly stated. For example, does "cancel" refer to a cancellation via

  • throwing a special cancellation exception (CancellationException, JobCancellationException), or
  • throwing the original exception, or
  • "marking" the current job as inactive by just invoking cancel()?

https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/async.html states:

it cancels the parent job (or outer scope) on failure to enforce structured concurrency paradigm.

Yes, but how is the parent job cancelled? Via a CancellationException or not? Here, it doesn't tell.

But there is https://kotlinlang.org/docs/exception-handling.html#cancellation-and-exceptions, which states:

If a coroutine encounters an exception other than CancellationException, it cancels its parent with that exception.

So we can assume in this case that the parent job is cancelled with a TestException (but we have to consult multiple places in the docs).

Continuing to quote from the previous post:

https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-deferred/await.html states:

returning the resulting value or throwing the corresponding exception if the deferred was cancelled.

Now, in "inner, bare", async and await are in the same coroutine scope. When TestException is explicitly thrown, the async coroutine cancels its parent job with that TestException. await is suspending at the time. What happens next?

https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/await.html says:

If the Job of the current coroutine is cancelled or completed while this suspending function is waiting, this function stops waiting for the promise and immediately resumes with CancellationException.

So we might think that await will throw a CancellationException. But in case of "inner, bare", it does not: await will throw the same TestException that is just in the process of cancelling the coroutine which await belongs to.

Now we have two active TestExceptions at the same time in the same coroutine: One from async, another one from await. If we catch the one from await the one from async is still there. And that's unusual given the exception handling background learned in sequential contexts.

This behaviour can also be easily observed like this:

fun main() = runTest {
    async { throw TestException() }
    try {
        delay(10.milliseconds) // will throw an exception, because the parent coroutine got cancelled in the meantime
    } catch (e: Exception) {
        println("delay threw")
    }
}

Yes, the difference being that println("delay threw $e") would reveal

delay threw kotlinx.coroutines.JobCancellationException: BlockingCoroutine is cancelling; job=BlockingCoroutine{Cancelling}@50675690

Afterward, the parent's cancellation with the original exception would appear:

Exception in thread "main" TestException: 

Here we have async cancelling its parent runTest with TestException, and then delay being cancelled with a JobCancellationException. So no double-throw of TestException here. All fine.

Semantically, you can think of async as a launch that returns not just a Job but also a value. Or you can think of launch with its job.join() as an async that returns a Unit.

Yes, I have always looked at it that way.

Occasionally, the Deferred is used in a scope other than the one it was created in. A simplified example (pseudocode):

If I'm not overlooking something, that scenario would also be fine as there would not be multiple active exceptions in the same scope.

The original problem just comes up with async launching a child of the coroutine which await belongs to.

Does that better explain the problem?

@dkhalanskyjb
Copy link
Contributor

Does that better explain the problem?

I don't think so, sorry.

I see a couple of suspicious turns of phrase on your side, though. Let's clarify them to make sure there is no misunderstanding.

From the discussion you linked:

two active exceptions in parallel

From the issue text:

Now we have two active TestExceptions at the same time in the same coroutine: One from async, another one from await. If we catch the one from await the one from async is still there. And that's unusual given the exception handling background learned in sequential contexts.

and

So no double-throw of TestException here.

I simply don't understand what an "active exception" or a "double-throw" is. There's no such thing as several exceptions in flight simultaneously, the language doesn't permit that.

Let's manually recreate what happens, without using async.

fun main1() = runTest {
    val deferred: Deferred<Int> = async {
        throw TestException()
    }
    testScheduler.advanceUntilIdle() // make sure the work does finish and the parent gets cancelled
    println("The exception from the spawned-off coroutine: ${deferred.getCompletionExceptionOrNull()}")
    try {
        delay(1000)
    } catch (e: Exception) {
        println("Structured concurrency returned an error: $e")
    }
}

fun main2() = runTest {
    var result: Result<Int>? = null
    launch {
        val exception = TestException()
        result = Result.failure(exception)
        throw exception
    }
    testScheduler.advanceUntilIdle() // make sure the work does finish and the parent gets cancelled
    println("The exception from the spawned-off coroutine: ${result!!.exceptionOrNull()}")
    try {
        delay(1000)
    } catch (e: Exception) {
        println("Structured concurrency returned an error: $e")
    }
}

Here, the combination of advanceUntilIdle and deferred.getCompletionExceptionOrNull behave the same way as await(), only instead of throwing the exception inside of it, we get to observe it safely.

In both cases, we observe three exceptions:

  • The one from Result/Deferred (we avoid throwing it, instead just printing it);
  • The one from delay (caused by the thrown exception propagating to the parent via structured concurrency: no new suspensions are permitted once the parent coroutine is cancelled);
  • The one from runTest (also due to structured concurrency: the coroutine in which the test body was running failed).

Do you think the behavior of launch is also problematic in this case? After all, even though we "handled" the exception written to result, it still gets rethrown on delay and when runTest exits. If we replaced printing with a throw + try/catch, it would be almost exactly as what happens in the async case.

@OliverO2
Copy link
Contributor Author

There's no such thing as several exceptions in flight simultaneously, the language doesn't permit that.

Agreed. And you are right that "in-flight simultaneously" or (as I phrased it) "in parallel" does not accurate describe it. What happens in my example is that the same exception (by identity), although caught and not explicitly rethrown, is recycled and thrown a second time.

Let's go through your examples first.

Running main1():

The exception from the spawned-off coroutine: TestException: 1
Structured concurrency returned an error: kotlinx.coroutines.JobCancellationException: TestScopeImpl is cancelling; job=TestScope[test started]
Exception in thread "main" TestException: 1

We're seeing

  1. TestException is thrown in the child coroutine.
  2. TestException is extracted in the parent coroutine from deferred and printed. Execution continues (though the parent coroutine is now in a cancelled state).
  3. A JobCancellationException is thrown by invoking a suspend function (delay) on a cancelled coroutine.
  4. TestException, having never been caught, continues to bubble up at the end of the parent's coroutine.

Two exceptions, each thrown once and caught once: JobCancellationException, then TestException.

Exactly the same occurs in main2().

Do you think the behavior of launch is also problematic in this case?

No. Both cases look perfectly fine.

After all, even though we "handled" the exception written to result,

The difference being "handled". Which in this case is not catching, but storing the exception.

it still gets rethrown on delay

It does not get rethrown on delay. delay throws a different one: JobCancellationException.

and when runTest exits.

Semantically, it does not (explicitly) get "rethrown" but just continues to bubble up. (All code from testScheduler.advanceUntilIdle() until the final } of main1() could be imagined as executing in one big finally clause of the parent coroutine.)

If we replaced printing with a throw + try/catch, it would be almost exactly as what happens in the async case.

Yes, and the term "almost" is what breaks the expectations learned from non-concurrent exception handling: Once an exception is caught and not explicitly rethrown, it is "dead". No implicit mechanism will interfere and try to revive (throw once more) that "dead" exception. (Throwing a different exception for other reasons would be another thing.)

Let's illustrate it by this example, which catches from await:

fun main() = runTest {
    var awaitException: Throwable? = null
    try {
        coroutineScope {
            val deferred = async {
                throw TestException("boom")
            }
            testScheduler.advanceUntilIdle() // make sure the work does finish and the parent gets cancelled
            try {
                deferred.await()
            } catch (e: Throwable) {
                println("caught from await: $e")
                awaitException = e
            }
        }
    } catch (e: Throwable) {
        println("caught from outer scope: $e.")
        println("Are the exceptions caught from 'outer scope' and 'await' identical? ${e === awaitException}")
    }
}

Running it:

caught from await: TestException: boom
caught from outer scope: TestException: boom.
Are the exceptions caught from 'outer scope' and 'await' identical? true

So that just seems strange and that special case would – in my opinion – either better be explicitly illustrated in the docs or be made explicit by a different kind of (wrapper) exception.

Why do I think this needs clarification? I would have probably never come across that case if it were just me. Mainly, because I'm doing event-based stuff and almost don't use async at all (everything is just Channels and Flows). But in the context of Kotest users coming up with problems related to testing, I'd like to be able to easily explain the behavior of code, which might be a bit unusual or even non-idiomatic.

@dkhalanskyjb
Copy link
Contributor

Ok, thank you, I think I'm starting to get it. Just to make sure: is this also strange?

fun main() = runTest {
    val deferred = CompletableDeferred<Int>()
    deferred.completeExceptionally(TestException())
    val exception1: Exception
    try {
        deferred.await()
        return@runTest
    } catch (e: Exception) {
        exception1 = e
    }
    val exception2: Exception
    try {
        deferred.await()
        return@runTest
    } catch (e: Exception) {
        exception2 = e
    }
    if (exception1 === exception2) {
        println("got the same exception $exception1 twice")
    }
}

Here, we process an exception once, but then we can obtain exactly the same exception a second time, even though it should be "dead."

@OliverO2
Copy link
Contributor Author

That's an interesting example. I'm taking it that deferred.completeExceptionally(TestException()) is equivalent to actually throwing the exception within the Deferred. So the example would contradict the concept of throw once / catch once.

This is also somewhat strange. The user might ask: Who throws initially?

If it is the Deferred (async job) throwing, one might conclude that the exception was somehow tunneled through await() and finally consumed via the first catch, so it is "dead" afterward. In that case, the second invocation would have to behave differently (throw some other exception).

Actually, it is await() (re-)throwing the exception, which would be OK if illuminated in the docs, hinting at the potential multiplicity of throws. So something like

await

Awaits for completion of this value without blocking a thread and resumes when the deferred computation is complete. It returns the resulting value or throwing the corresponding exception if the deferred was cancelled.

In the latter case, await throws the deferred's exception on each invocation. This occurs in addition to the same exception being thrown to cancel the deferred's parent.

...

dkhalanskyjb added a commit that referenced this issue Feb 12, 2024
Fixes two issues:
* It is surprising for some users that the same exception can be
  thrown several times. Clarified this point explicitly.
* Due to #3658, `await` can throw `CancellationException` in
  several cases: when the `await` call is cancelled, or when the
  `Deferred` is cancelled. This is clarified with an example of
  how to handle this.

Fixes #3937
dkhalanskyjb added a commit that referenced this issue Feb 13, 2024
Fixes two issues:
* It is surprising for some users that the same exception can be
  thrown several times. Clarified this point explicitly.
* Due to #3658, `await` can throw `CancellationException` in
  several cases: when the `await` call is cancelled, or when the
  `Deferred` is cancelled. This is clarified with an example of
  how to handle this.

Fixes #3937
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs KDoc and API reference
Projects
None yet
Development

No branches or pull requests

2 participants