recoverStackTrace causes nested exceptions

[iwismer]

Describe the bug

When throwing an exception in a coroutine scope it sometimes becomes nested one level too deep in the enclosing exception.

For example, if I throw a: TestException(IllegalStateException("test")) (a TestException with a cause of IllegalStateException with a message), then the coroutine scope will throw an exception that is: TestException(TestException(IllegalStateException("test"))).

Provide a Reproducer

The following test will fail as a result of this bug:

@Test
fun testNestedException() = runTest {
    val result = runCatching {
        coroutineScope<Unit> {
            throw NestedException(IllegalStateException("ERROR"))
        }
    }
    val ex = result.exceptionOrNull() ?: error("Expected to fail")
    assertTrue(ex is NestedException)
    assertTrue(ex.cause is IllegalStateException)
    assertEquals("ERROR", (ex.cause as IllegalStateException).message)
}

class NestedException : RuntimeException {
    constructor(message: String) : super(message)
    constructor(cause: Throwable) : super(cause)
    constructor() : super()
}

I can get this test to not fail by changing the order of the constructors in the class to:

class NestedException : RuntimeException {
    constructor(cause: Throwable) : super(cause)
    constructor(message: String) : super(message)
    constructor() : super()
}

But I don't think this is actually a good solution as it shouldn't matter the order of the constructors in my exception, it should work the same way regardless.

I think the issue is at this line where the wrong constructor is being selected (the string constructor instead of the cause constructor).

[dkhalanskyjb]

Thank you for the report! The actual problem is in this line:

kotlinx.coroutines/kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstructor.kt

Line 44 in 3fc1c20

if (result != null) return result

Instead of ordering the constructors from best to worst, we exit when we find the first one that matches any viable signature.

[qwwdfsad]

These two constructors should be identical, something is going completely wrong here

[iwismer]

The test that was implemented in #3731 seems to have some incorrect asserts that are actually testing that the error is still happening.

The asserts in the test are:

val ex = result.exceptionOrNull() ?: error("Expected to fail")
assertIs<NestedException>(ex)
assertIs<NestedException>(ex.cause)
val originalCause = ex.cause?.cause
assertIs<IllegalStateException>(originalCause)
assertEquals("ERROR", originalCause.message)

which asserts that the error is: NestedException(NestedException(IllegalStateException()))

But what the thrown error should actually be is: NestedException(IllegalStateException())
So the asserts should be changed to:

val ex = result.exceptionOrNull() ?: error("Expected to fail")
assertIs<NestedException>(ex)
val originalCause = ex.cause
assertIs<IllegalStateException>(originalCause)
assertEquals("ERROR", originalCause.message)

So it seems like this issue is not yet solved, and I'm still running into this issue in my dependant code after updating to 1.7.0.