Properly recover exceptions when they are constructed from 'Throwable(cause)' constructor.

And restore the original behaviour. After #1631 this constructor's recovery mechanism was broken because 'Throwable(cause)' changes the message to be equal to 'cause.toString()' which isn't equal to the original message.

Also, make reflective constructor choice independable from source-code order

Fixes #3714

Author: qwwdfsad

.idea/codeStyles/Project.xml

@@ -32,13 +32,29 @@ internal fun <E : Throwable> tryCopyException(exception: E): E? {
 
 private fun <E : Throwable> createConstructor(clz: Class<E>): Ctor {
     val nullResult: Ctor = { null } // Pre-cache class
-    // Skip reflective copy if an exception has additional fields (that are usually populated in user-defined constructors)
+    // Skip reflective copy if an exception has additional fields (that are typically populated in user-defined constructors)
     if (throwableFields != clz.fieldsCountOrDefault(0)) return nullResult
     /*
-    * Try to reflectively find constructor(), constructor(message, cause), constructor(cause) or constructor(message).
-    * Exceptions are shared among coroutines, so we should copy exception before recovering current stacktrace.
-    */
-    val constructors = clz.constructors.sortedByDescending { it.parameterTypes.size }
+     * Try to reflectively find constructor(message, cause), constructor(message), constructor(cause), or constructor()
+     * Exceptions are shared among coroutines, so we should copy exception before recovering current stacktrace.
+     *
+     * By default, Java's reflection iterates over ctors in the source-code order and the sorting is stable,
+     * so in comparator we are picking the message ctor over the cause one to break such implcit dependency on the source code.
+     */
+    val constructors = clz.constructors.sortedByDescending {
+        // (m, c) -> 3
+        // (m) -> 2
+        // (c) -> 1
+        // () -> 0
+        val params = it.parameterTypes // cloned array
+        when (params.size) {
+            2 -> 3
+            1 -> if (params[0] == String::class.java) 2 else 1
+            0 -> 0
+            else -> -1
+        }
+
+    }
     for (constructor in constructors) {
         val result = createSafeConstructor(constructor)
         if (result != null) return result
@@ -66,8 +82,17 @@ private fun createSafeConstructor(constructor: Constructor<*>): Ctor? {
     }
 }
 
-private inline fun safeCtor(crossinline block: (Throwable) -> Throwable): Ctor =
-    { e -> runCatching { block(e) }.getOrNull() }
+private inline fun safeCtor(crossinline block: (Throwable) -> Throwable): Ctor = { e ->
+    runCatching {
+        val result = block(e)
+        /*
+         * Verify that the new exception has the same message as the original one (bail out if not, see #1631)
+         * or if the new message complies the contract from `Throwable(cause).message` contract.
+         */
+        if (e.message != result.message && result.message != e.toString()) null
+        else result
+    }.getOrNull()
+}
 
 private fun Class<*>.fieldsCountOrDefault(defaultValue: Int) =
     kotlin.runCatching { fieldsCount() }.getOrDefault(defaultValue)

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

@@ -84,9 +84,6 @@ private fun <E : Throwable> recoverFromStackFrame(exception: E, continuation: Co
 
 private fun <E : Throwable> tryCopyAndVerify(exception: E): E? {
     val newException = tryCopyException(exception) ?: return null
-    // Verify that the new exception has the same message as the original one (bail out if not, see #1631)
-    // CopyableThrowable has control over its message and thus can modify it the way it wants
-    if (exception !is CopyableThrowable<*> && newException.message != exception.message) return null
     return newException
 }
 
@@ -157,7 +154,6 @@ private fun mergeRecoveredTraces(recoveredStacktrace: Array<StackTraceElement>,
     }
 }
 
-@Suppress("NOTHING_TO_INLINE")
 internal actual suspend inline fun recoverAndThrow(exception: Throwable): Nothing {
     if (!RECOVER_STACK_TRACES) throw exception
     suspendCoroutineUninterceptedOrReturn<Nothing> {

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

@@ -87,6 +87,26 @@ class StackTraceRecoveryCustomExceptionsTest : TestBase() {
         assertEquals("Token OK", ex.message)
     }
 
+    @Test
+    fun testNestedExceptionWithCause() = runTest {
+        val result = runCatching {
+            coroutineScope<Unit> {
+                throw NestedException(IllegalStateException("ERROR"))
+            }
+        }
+        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)
+    }
+
+    class NestedException : RuntimeException {
+        constructor(cause: Throwable) : super(cause)
+        constructor() : super()
+    }
+
     @Test
     fun testWrongMessageExceptionInChannel() = runTest {
         val result = produce<Unit>(SupervisorJob() + Dispatchers.Unconfined) {