No longer able to catch exceptions from other coroutines while within runTest after 1.7.0

[lucas-livefront]

Describe the bug
Pre-1.7.0 we were doing the correct thing by using the Thread ’s uncaught exception handler for exceptions thrown in coroutines that are encapsulated in a tested function like so:

// An example function to test
private fun somethingThatBuildsACoroutineScopeAndThrows() {
    CoroutineScope(Dispatchers.Unconfined).launch {
        throw IllegalStateException("Something is happening")
    }
}

// The utility for testing it correctly before 1.7.0.
fun <T : Throwable> assertThrowsInCoroutine(
    expectedThrowable: Class<T>,
    block: () -> Unit,
): T {
    val originalHandler = Thread.getDefaultUncaughtExceptionHandler()
    var throwable: Throwable? = null
    Thread.setDefaultUncaughtExceptionHandler { _, t -> throwable = t }

    try {
        block()
    } catch (t: Throwable) {
        // Manually catch this here just in case the exception is not in a coroutine
        throwable = t
    }

    Thread.setDefaultUncaughtExceptionHandler(originalHandler)

    return assertThrows(expectedThrowable) {
        throwable?.let { throw it }
    }
}

// An example test
@Test
fun `happy test`() = runTest {
   // Do some other suspend function work... thus requiring the `runTest` wrapper.
   
    assertThrowsInCoroutine(IllegalStateException::class.java) {
        somethingThatBuildsACoroutineScopeAndThrows()
    }
}

But as of 1.7.0 if we try this, the exception goes uncaught and the test bombs. In this PR it seems this behavior was explicitly removed so that users who aren't handling failures in other coroutines experience test failure. I would like to know how we can still correctly catch these exceptions.

Additional Note
We can work around this work around by just wrapping our test function in runTest. This is not ideal, because as I understand it, we shouldn't nest runTests.

@Test
fun `wrap in runTest for success`() = runTest {
   // Do some other suspend function work... thus requiring the `runTest` wrapper.

    assertThrowsInCoroutine(IllegalStateException::class.java) {
        runTest {
            somethingThatBuildsACoroutineScopeAndThrows()
        }
    }
}

[dkhalanskyjb]

Could you give an example of a useful test that uses this? Why would one ever want to intentionally throw exceptions in a way that they are passed to the uncaught exception handler?

[lucas-livefront]

The current use case for this is within our ViewModels that use the actor pattern. The ViewModel builds an actor with CoroutineScope.actor(), so the actor is initialized from within the coroutine scope. We have scenarios where we expect something to be in the SavedStateMap that we have passed to our actor. In our tests we want to assert that if that value isn't within the SavedStateMap, an exception is thrown.

If more information is required, I can certainly get it for you.

[dkhalanskyjb]

ViewModel means that you have an Android codebase, and uncaught exceptions crash the application on Android. Do you actually have tests that check that if a value isn't present, the app has to crash? Or do you handle exceptions some other way in production?

[lucas-livefront]

Your inference is correct. The app would also crash in production. Is your hypothesis that there is no case where an exception would be thrown from a coroutine outside of the hierarchy of the test scope?

[dkhalanskyjb]

So, do I get it right that you want to ensure that your app crashes if the conditions are right? That's a very unusual requirement. Typically, people try to avoid writing code that crashes the app and test that it does not crash under any circumstances.

[brian-livefront]

@dkhalanskyjb The idea here is to "fail fast" during development so that we catch any cases where required input data are missing and fix them before they hit production. We want to test that our "fail fast" logic is working. This is useful for us to both know and test. This used to be possible to do and with recent changes it has become much more cumbersome and awkward. If the recommendation here is to wrap the call in runTest and catch the exceptions that it throws, then please let us know.

[brian-livefront]

@dkhalanskyjb If it helps to have a standard reference, here is an example from the Now In Android Google sample project :

internal class TopicArgs(val topicId: String) {
    constructor(savedStateHandle: SavedStateHandle) :
        this(URLDecoder.decode(checkNotNull(savedStateHandle[topicIdArg]), URL_CHARACTER_ENCODING))
}

That checkNotNull will crash in the ViewModel if that data is missing:

@HiltViewModel
class TopicViewModel @Inject constructor(
    savedStateHandle: SavedStateHandle,
    private val userDataRepository: UserDataRepository,
    topicsRepository: TopicsRepository,
    userNewsResourceRepository: UserNewsResourceRepository,
) : ViewModel() {

    private val topicArgs: TopicArgs = TopicArgs(savedStateHandle) // <---- Crash happens here if the "topicId" data is missing.

    val topicId = topicArgs.topicId

That is the same as what's going on in our case (an exception would be thrown if its missing); the only difference here is that they would get the exception when constructing the VM and we will get in inside a coroutine just based on how our VMs get constructed. This kind of "developer exception" use case is not unusual, but now we are looking for guidance on how to test it given these recent changes to runTest.

[dkhalanskyjb]

I see. Yes, in your case, nesting runTest is fine. It only becomes a problem when dealing with multiplatform code, and I see that your problem is Android-specific, where you can't supply additional coroutine context to the ViewModelScope.

[brian-livefront]

Sounds good, thanks @dkhalanskyjb! We just wanted to make sure those nested runTest calls wouldn't cause us any unforeseen issues so it's good to hear that we should be safe. Thanks for taking a look at this!

[lucas-livefront]

Actually, @dkhalanskyjb this solution doesn't work as we expected. 🤦 To be clear here I will use slightly different tests to illustrate the issues here.

As a reminder, here is the method we want to assert throws. This is a simple mock up, but in practice we would test some core app functionality that is wrapped up in a coroutine scope that it creates itself.

// An example function to test
private fun somethingThatBuildsACoroutineScopeAndThrows() {
    CoroutineScope(Dispatchers.Unconfined).launch {
        throw IllegalStateException("Something is happening")
    }
}

Using TestScope.runTest

The first one uses there TestScope.runTest in an attempt to catch the uncaught exception.

@Test
fun `non-TestScope runTest test`() {
    val block: TestScope.() -> Unit = {
        runTest { somethingThatBuildsACoroutineScopeAndThrows() }
    }
    runTest {
        val throwable = assertThrows(IllegalStateException::class.java) {
            block()
        }
    }
}

This throws an exception that happens to be an IllegalStateException which makes the test appear to succeed. However, if we update the test to test the exception's message we will see it fail. This was the flaw we made in the code we claimed to be a workaround in the initial post.

@Test
fun `non-TestScope runTest test`() {
    val block: TestScope.() -> Unit = {
        runTest { somethingThatBuildsACoroutineScopeAndThrows() }
    }
    runTest {
        val throwable = assertThrows(IllegalStateException::class.java) {
            block()
        }
        assertEquals(
            throwable.message, "Something is happening"
        )
    }
}

expected: <Only a single call to runTest can be performed during one test.> but was:
Expected :Only a single call to runTest can be performed during one test.
Actual :Something is happening

So this is not an option for catching uncaught exceptions.

Using the Unscoped runTest

If we use a regular runTest, the assertions will succeed but the outermost runTest will also throw IllegalStateException("Something is happening").

@Test
fun `non-TestScope runTest test`() {
    val block: TestScope.() -> Unit = {
        runTest { somethingThatBuildsACoroutineScopeAndThrows() }
    }
    runTest {
        val throwable = assertThrows(IllegalStateException::class.java) {
            block()
        }
        // Added print statements to prove that assertions are succeeding
        println("Throwable type is correct: ${throwable is IllegalStateException}")
        println("Throwable message is correct: ${throwable.message == "Something is happening"}")
        assertEquals(
            throwable.message, "Something is happening"
        )
    }
}

Throwable type is correct: true
Throwable message is correct: true

Something is happening
java.lang.IllegalStateException: Something is happening
...

This, also, is a non-option for catching uncaught exceptions.

Conclusion

Ultimately, we don't have a solution for handling uncaught exceptions from with a runTest invocation. What do you suggest we do from here and is there anything else you might need from me?

[dkhalanskyjb]

Only a single call to runTest can be performed during one test

You can work around that with

    runTest {
        TestScope().runTest {
            println("this doesn't fail")
        }
    }

[lucas-livefront]

@dkhalanskyjb In this case we see the same error as the "unscoped runTest" case that I described above.

    @Test
    fun `building new TestScope runTest test`() {
        val block: () -> Unit = {
            TestScope().runTest { somethingThatBuildsACoroutineScopeAndThrows() }
        }
        runTest {
            val throwable = assertThrows(IllegalStateException::class.java) {
                block()
            }
        // Added print statements to prove that assertions are succeeding
        println("Throwable type is correct: ${throwable is IllegalStateException}")
        println("Throwable message is correct: ${throwable.message == "Something is happening"}")
            assertEquals(
                throwable.message, "Something is happening"
            )
        }
    }

Throwable type is correct: true
Throwable message is correct: true

Something is happening
java.lang.IllegalStateException: Something is happening
...

[dkhalanskyjb]

Ok, I see the problem: both runTest calls subscribe to notifications about exceptions, so the outer runTest also throws.

Will this work for you?

    val block: () -> Unit = {
        TestScope().runTest { CoroutineScope(Dispatchers.Unconfined).launch { throw IllegalStateException("Something is happening") } }
    }
    runBlocking {
        try {
            block()
        } catch (throwable: Exception) {
            println("Caught exception: $throwable")
            // Added print statements to prove that assertions are succeeding
            println("Throwable type is correct: ${throwable is IllegalStateException}")
            println("Throwable message is correct: ${throwable.message == "Something is happening"}")
        }
    }

[lucas-livefront]

@dkhalanskyjb So is it officially recommended that I not use runTest to run suspending code whenever I also need to catch uncaught exceptions? If it is, I can see there being lingering issues here. This seems like something that would easily be done incorrectly and is difficult to debug.