Exception Deserialization (#170)

Mr3zee · web-flow · commit 07d7e88ac3ae · 2024-08-19T17:14:49.000+02:00
diff --git a/core/src/commonMain/kotlin/kotlinx/rpc/internal/ExceptionUtils.kt b/core/src/commonMain/kotlin/kotlinx/rpc/internal/ExceptionUtils.kt
@@ -12,7 +12,7 @@ public fun serializeException(cause: Throwable): SerializedException {
     val message = cause.message ?: "Unknown exception"
     val stacktrace = cause.stackElements()
     val serializedCause = cause.cause?.let { serializeException(it) }
-    val className = cause::class.qualifiedClassNameOrNull ?: ""
+    val className = cause::class.typeName ?: ""
 
     return SerializedException(cause.toString(), message, stacktrace, serializedCause, className)
 }
diff --git a/core/src/commonMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.kt b/core/src/commonMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.kt
@@ -21,6 +21,9 @@ public fun internalError(message: String): Nothing {
     error("Internal kotlinx.rpc error: $message")
 }
 
+@InternalRPCApi
+public expect val KClass<*>.typeName: String?
+
 @InternalRPCApi
 public expect val KClass<*>.qualifiedClassNameOrNull: String?
 
diff --git a/core/src/jsMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.js.kt b/core/src/jsMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.js.kt
@@ -11,3 +11,7 @@ import kotlin.reflect.KClass
 @InternalRPCApi
 public actual val KClass<*>.qualifiedClassNameOrNull: String?
     get() = toString()
+
+@InternalRPCApi
+public actual val KClass<*>.typeName: String?
+    get() = qualifiedClassNameOrNull
diff --git a/core/src/jvmMain/kotlin/kotlinx/rpc/internal/ExceptionUtils.jvm.kt b/core/src/jvmMain/kotlin/kotlinx/rpc/internal/ExceptionUtils.jvm.kt
@@ -62,21 +62,44 @@ public actual fun SerializedException.deserialize(): Throwable {
 private fun Class<*>.fieldsCountOrDefault(defaultValue: Int) =
     kotlin.runCatching { fieldsCount() }.getOrDefault(defaultValue)
 
-private tailrec fun Class<*>.fieldsCount(accumulator: Int = 0): Int {
-    val fieldsCount = declaredFields.count { !Modifier.isStatic(it.modifiers) }
-    val totalFields = accumulator + fieldsCount
-    val superClass = superclass ?: return totalFields
+private tailrec fun Class<*>.fieldsCount(accumulator: Set<String> = emptySet()): Int {
+    val fields = declaredFields
+        .filter { !Modifier.isStatic(it.modifiers) }
+        .map { it.name }
+    val totalFields = (accumulator + fields)
+    val superClass = superclass ?: run {
+        var messageField = false
+        return totalFields.count {
+            // Throwable has a private field 'detailMessage', but a public open `getMessage`,
+            // which are the same in custom Kotlin exceptions
+            if (it == "message" || it == "detailMessage") {
+                if (messageField) {
+                    return@count false
+                }
+                messageField = true
+            }
+            true
+        }
+    }
     return superClass.fieldsCount(totalFields)
 }
 
+private fun Class<*>.isExceptionClass(): Boolean = Throwable::class.java.isAssignableFrom(this)
+
 private fun tryCreateException(constructor: Constructor<*>, serialized: SerializedException): Throwable? {
     val parameters = constructor.parameterTypes
 
     val result = when (parameters.size) {
-        2 -> constructor.newInstance(serialized.message, serialized.cause?.deserialize())
-        1 -> when (parameters[0]) {
-            Throwable::class.java -> constructor.newInstance(serialized.cause?.deserialize())
-            String::class.java -> constructor.newInstance(serialized.message)
+        2 -> when {
+            parameters[0] == String::class.java && parameters[1].isExceptionClass() ->
+                constructor.newInstance(serialized.message, serialized.cause?.deserialize())
+
+            else -> null
+        }
+
+        1 -> when {
+            parameters[0].isExceptionClass() -> constructor.newInstance(serialized.cause?.deserialize())
+            parameters[0] == String::class.java -> constructor.newInstance(serialized.message)
             else -> null
         }
 
diff --git a/core/src/jvmMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.jvm.kt b/core/src/jvmMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.jvm.kt
@@ -11,3 +11,7 @@ import kotlin.reflect.KClass
 @InternalRPCApi
 public actual val KClass<*>.qualifiedClassNameOrNull: String?
     get() = qualifiedName
+
+@InternalRPCApi
+public actual val KClass<*>.typeName: String?
+    get() = java.typeName
diff --git a/core/src/nativeMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.native.kt b/core/src/nativeMain/kotlin/kotlinx/rpc/internal/ReflectionUtils.native.kt
@@ -11,3 +11,7 @@ import kotlin.reflect.KClass
 @InternalRPCApi
 public actual val KClass<*>.qualifiedClassNameOrNull: String?
     get() = qualifiedName
+
+@InternalRPCApi
+public actual val KClass<*>.typeName: String?
+    get() = qualifiedClassNameOrNull
diff --git a/gradle-conventions-settings/src/main/kotlin/util/PublicationUtils.kt b/gradle-conventions-settings/src/main/kotlin/util/PublicationUtils.kt
@@ -34,17 +34,19 @@ fun MavenPublication.fixModuleMetadata(project: Project) {
     val upperCaseName = Character.toUpperCase(publicationName.first()) + publicationName.drop(1)
     val patchTaskName = "patchModuleJsonFor$upperCaseName"
     val generateMetadataTaskName = "generateMetadataFileFor${upperCaseName}Publication"
+    val projectName = project.name
+    val projectVersion = project.version.toString()
+    val moduleJsonFile = project.layout.buildDirectory.file("publications/$publicationName/module.json").get().asFile
 
     val patch = project.tasks.register(patchTaskName) {
         group = "patch"
 
         doLast {
-            val file = project.layout.buildDirectory.file("publications/$publicationName/module.json").get().asFile
-            if (!file.exists()) {
+            if (!moduleJsonFile.exists()) {
                 return@doLast
             }
 
-            file.fixMetadata(project)
+            moduleJsonFile.fixMetadata(projectName, projectVersion)
 
             logger.info("Updated metadata for $publicationName publication")
         }
@@ -55,13 +57,13 @@ fun MavenPublication.fixModuleMetadata(project: Project) {
     }
 }
 
-private fun File.fixMetadata(project: Project) {
+private fun File.fixMetadata(projectName: String, projectVersion: String) {
     val text = readText()
 
     val newText = text
-        .updateFilesNameField(project.name)
-        .updateAvailableAtModuleField(project.name)
-        .updateAvailableAtUrlField(project.name, project.version.toString())
+        .updateFilesNameField(projectName)
+        .updateAvailableAtModuleField(projectName)
+        .updateAvailableAtUrlField(projectName, projectVersion)
 
     writeText(newText)
 }
diff --git a/krpc/krpc-test/src/jvmMain/kotlin/kotlinx/rpc/test/KRPCTestService.kt b/krpc/krpc-test/src/jvmMain/kotlin/kotlinx/rpc/test/KRPCTestService.kt
@@ -55,6 +55,7 @@ interface KRPCTestService : RPC {
     suspend fun nullableBytes(byteArray: ByteArray?)
 
     suspend fun throwsIllegalArgument(message: String)
+    suspend fun throwsSerializableWithMessageAndCause(message: String)
     suspend fun throwsThrowable(message: String)
     suspend fun throwsUNSTOPPABLEThrowable(message: String)
 
diff --git a/krpc/krpc-test/src/jvmMain/kotlin/kotlinx/rpc/test/KRPCTestServiceBackend.kt b/krpc/krpc-test/src/jvmMain/kotlin/kotlinx/rpc/test/KRPCTestServiceBackend.kt
@@ -6,6 +6,7 @@ package kotlinx.rpc.test
 
 import kotlinx.coroutines.*
 import kotlinx.coroutines.flow.*
+import kotlinx.serialization.Serializable
 import kotlin.coroutines.CoroutineContext
 import kotlin.coroutines.resumeWithException
 import kotlin.test.assertEquals
@@ -184,6 +185,16 @@ class KRPCTestServiceBackend(override val coroutineContext: CoroutineContext) :
         throw IllegalArgumentException(message)
     }
 
+    @Serializable
+    class SerializableTestException(
+        override val message: String?,
+        override val cause: SerializableTestException? = null,
+    ) : Exception()
+
+    override suspend fun throwsSerializableWithMessageAndCause(message: String) {
+        throw SerializableTestException(message, SerializableTestException("cause: $message"))
+    }
+
     @Suppress("detekt.TooGenericExceptionThrown")
     override suspend fun throwsThrowable(message: String) {
         throw Throwable(message)
diff --git a/krpc/krpc-test/src/jvmMain/kotlin/kotlinx/rpc/test/KRPCTransportTestBase.kt b/krpc/krpc-test/src/jvmMain/kotlin/kotlinx/rpc/test/KRPCTransportTestBase.kt
@@ -12,6 +12,7 @@ import kotlinx.coroutines.flow.*
 import kotlinx.rpc.*
 import kotlinx.rpc.serialization.RPCSerialFormatConfiguration
 import kotlinx.rpc.server.KRPCServer
+import kotlinx.rpc.test.KRPCTestServiceBackend.SerializableTestException
 import org.junit.Assert.assertEquals
 import org.junit.Rule
 import org.junit.rules.Timeout
@@ -370,16 +371,21 @@ abstract class KRPCTransportTestBase {
         }
     }
 
-    @OptIn(ExperimentalCoroutinesApi::class)
     @Test
     @Suppress("detekt.TooGenericExceptionCaught")
-    fun testException() {
+    fun testExceptionSerializationAndPropagating() {
         runBlocking {
             try {
                 client.throwsIllegalArgument("me")
             } catch (e: IllegalArgumentException) {
                 assertEquals("me", e.message)
             }
+            try {
+                client.throwsSerializableWithMessageAndCause("me")
+            } catch (e: SerializableTestException) {
+                assertEquals("me", e.message)
+                assertEquals("cause: me", e.cause?.message)
+            }
             try {
                 client.throwsThrowable("me")
             } catch (e: Throwable) {
diff --git a/publishLocal.sh b/publishLocal.sh
@@ -6,7 +6,7 @@
 
 set -euxo pipefail
 
-./gradlew publishAllPublicationsToBuildRepoRepository \
-  :compiler-plugin:publishAllPublicationsToBuildRepoRepository \
-  :ksp-plugin:publishAllPublicationsToBuildRepoRepository \
-  :gradle-plugin:publishAllPublicationsToBuildRepoRepository
+./gradlew publishAllPublicationsToBuildRepoRepository
+./gradlew -p compiler-plugin publishAllPublicationsToBuildRepoRepository
+./gradlew -p ksp-plugin publishAllPublicationsToBuildRepoRepository
+./gradlew -p gradle-plugin publishAllPublicationsToBuildRepoRepository

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ public fun serializeException(cause: Throwable): SerializedException {`
`12`	`12`	`val message = cause.message ?: "Unknown exception"`
`13`	`13`	`val stacktrace = cause.stackElements()`
`14`	`14`	`val serializedCause = cause.cause?.let { serializeException(it) }`
`15`		`- val className = cause::class.qualifiedClassNameOrNull ?: ""`
	`15`	`+ val className = cause::class.typeName ?: ""`
`16`	`16`
`17`	`17`	`return SerializedException(cause.toString(), message, stacktrace, serializedCause, className)`
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,9 @@ public fun internalError(message: String): Nothing {`
`21`	`21`	`error("Internal kotlinx.rpc error: $message")`
`22`	`22`	`}`
`23`	`23`
	`24`	`+@InternalRPCApi`
	`25`	`+public expect val KClass<*>.typeName: String?`
	`26`	`+`
`24`	`27`	`@InternalRPCApi`
`25`	`28`	`public expect val KClass<*>.qualifiedClassNameOrNull: String?`
`26`	`29`