CORDA-540: Implementation of path in the serialization graph (#1484)

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CollectionSerializer.kt

@@ -61,13 +61,13 @@ class CollectionSerializer(val declaredType: ParameterizedType, factory: Seriali
 
     private val typeNotation: TypeNotation = RestrictedType(SerializerFactory.nameForType(declaredType), null, emptyList(), "list", Descriptor(typeDescriptor), emptyList())
 
-    override fun writeClassInfo(output: SerializationOutput) {
+    override fun writeClassInfo(output: SerializationOutput) = ifThrowsAppend({declaredType.typeName}) {
         if (output.writeTypeNotations(typeNotation)) {
             output.requireSerializer(declaredType.actualTypeArguments[0])
         }
     }
 
-    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) {
+    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) = ifThrowsAppend({declaredType.typeName}) {
         // Write described
         data.withDescribed(typeNotation.descriptor) {
             withList {
@@ -78,8 +78,8 @@ class CollectionSerializer(val declaredType: ParameterizedType, factory: Seriali
         }
     }
 
-    override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any {
+    override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any = ifThrowsAppend({declaredType.typeName})  {
         // TODO: Can we verify the entries in the list?
-        return concreteBuilder((obj as List<*>).map { input.readObjectOrNull(it, schema, declaredType.actualTypeArguments[0]) })
+        concreteBuilder((obj as List<*>).map { input.readObjectOrNull(it, schema, declaredType.actualTypeArguments[0]) })
     }
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/MapSerializer.kt

@@ -68,14 +68,14 @@ class MapSerializer(private val declaredType: ParameterizedType, factory: Serial
 
     private val typeNotation: TypeNotation = RestrictedType(SerializerFactory.nameForType(declaredType), null, emptyList(), "map", Descriptor(typeDescriptor), emptyList())
 
-    override fun writeClassInfo(output: SerializationOutput) {
+    override fun writeClassInfo(output: SerializationOutput) = ifThrowsAppend({declaredType.typeName}) {
         if (output.writeTypeNotations(typeNotation)) {
             output.requireSerializer(declaredType.actualTypeArguments[0])
             output.requireSerializer(declaredType.actualTypeArguments[1])
         }
     }
 
-    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) {
+    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) = ifThrowsAppend({declaredType.typeName}) {
         obj.javaClass.checkSupportedMapType()
         // Write described
         data.withDescribed(typeNotation.descriptor) {
@@ -90,10 +90,10 @@ class MapSerializer(private val declaredType: ParameterizedType, factory: Serial
         }
     }
 
-    override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any {
+    override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any = ifThrowsAppend({declaredType.typeName}) {
         // TODO: General generics question. Do we need to validate that entries in Maps and Collections match the generic type?  Is it a security hole?
         val entries: Iterable<Pair<Any?, Any?>> = (obj as Map<*, *>).map { readEntry(schema, input, it) }
-        return concreteBuilder(entries.toMap())
+        concreteBuilder(entries.toMap())
     }
 
     private fun readEntry(schema: Schema, input: DeserializationInput, entry: Map.Entry<Any?, Any?>) =

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt

@@ -41,7 +41,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS
         }
     }
 
-    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) {
+    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) = ifThrowsAppend({clazz.typeName}) {
         // Write described
         data.withDescribed(typeNotation.descriptor) {
             // Write list
@@ -53,11 +53,11 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS
         }
     }
 
-    override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any {
+    override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any = ifThrowsAppend({clazz.typeName}) {
         if (obj is List<*>) {
             if (obj.size > propertySerializers.size) throw NotSerializableException("Too many properties in described type $typeName")
             val params = obj.zip(propertySerializers).map { it.second.readProperty(it.first, schema, input) }
-            return construct(params)
+            construct(params)
         } else throw NotSerializableException("Body of described type is unexpected $obj")
     }
 
@@ -74,5 +74,4 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS
         return javaConstructor?.newInstance(*properties.toTypedArray()) ?:
                 throw NotSerializableException("Attempt to deserialize an interface: $clazz. Serialized form is invalid.")
     }
-
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/PropertySerializer.kt

@@ -76,19 +76,21 @@ sealed class PropertySerializer(val name: String, val readMethod: Method?, val r
         // This is lazy so we don't get an infinite loop when a method returns an instance of the class.
         private val typeSerializer: AMQPSerializer<*> by lazy { lazyTypeSerializer() }
 
-        override fun writeClassInfo(output: SerializationOutput) {
+        override fun writeClassInfo(output: SerializationOutput) = ifThrowsAppend({nameForDebug}) {
             if (resolvedType != Any::class.java) {
                 typeSerializer.writeClassInfo(output)
             }
         }
 
-        override fun readProperty(obj: Any?, schema: Schema, input: DeserializationInput): Any? {
+        override fun readProperty(obj: Any?, schema: Schema, input: DeserializationInput): Any? = ifThrowsAppend({nameForDebug}) {
-            return input.readObjectOrNull(obj, schema, resolvedType)
+            input.readObjectOrNull(obj, schema, resolvedType)
         }
 
-        override fun writeProperty(obj: Any?, data: Data, output: SerializationOutput) {
+        override fun writeProperty(obj: Any?, data: Data, output: SerializationOutput) = ifThrowsAppend({nameForDebug}) {
             output.writeObjectOrNull(readMethod!!.invoke(obj), data, resolvedType)
         }
+
+        private val nameForDebug = "$name(${resolvedType.typeName})"
     }
 
     /**

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt

@@ -238,3 +238,26 @@ internal fun suitableForObjectReference(type: Type): Boolean {
 internal enum class CommonPropertyNames {
     IncludeInternalInfo,
 }
+
+/**
+ * Utility function which helps tracking the path in the object graph when exceptions are thrown.
+ * Since there might be a chain of nested calls it is useful to record which part of the graph caused an issue.
+ * Path information is added to the message of the exception being thrown.
+ */
+internal inline fun <T> ifThrowsAppend(strToAppendFn: () -> String, block: () -> T): T {
+    try {
+        return block()
+    } catch (th: Throwable) {
+        th.setMessage("${strToAppendFn()} -> ${th.message}")
+        throw th
+    }
+}
+
+/**
+ * Not a public property so will have to use reflection
+ */
+private fun Throwable.setMessage(newMsg: String) {
+    val detailMessageField = Throwable::class.java.getDeclaredField("detailMessage")
+    detailMessageField.isAccessible = true
+    detailMessageField.set(this, newMsg)
+}

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/MapsSerializationTest.kt

@@ -7,12 +7,12 @@ import net.corda.core.serialization.serialize
 import net.corda.node.services.statemachine.SessionData
 import net.corda.testing.TestDependencyInjectionBase
 import net.corda.testing.amqpSpecific
+import net.corda.testing.kryoSpecific
 import org.assertj.core.api.Assertions
 import org.junit.Assert.assertArrayEquals
 import org.junit.Test
 import org.bouncycastle.asn1.x500.X500Name
 import java.io.ByteArrayOutputStream
-import java.io.NotSerializableException
 import java.util.*
 
 class MapsSerializationTest : TestDependencyInjectionBase() {
@@ -45,7 +45,8 @@ class MapsSerializationTest : TestDependencyInjectionBase() {
         val payload = HashMap<String, String>(smallMap)
         val wrongPayloadType = WrongPayloadType(payload)
         Assertions.assertThatThrownBy { wrongPayloadType.serialize() }
-                .isInstanceOf(NotSerializableException::class.java).hasMessageContaining("Cannot derive map type for declaredType")
+                .isInstanceOf(IllegalArgumentException::class.java).hasMessageContaining(
+                "Map type class java.util.HashMap is unstable under iteration. Suggested fix: use java.util.LinkedHashMap instead.")
     }
 
     @CordaSerializable
@@ -63,7 +64,7 @@ class MapsSerializationTest : TestDependencyInjectionBase() {
     }
 
     @Test
-    fun `check empty map serialises as Java emptytMap`() {
+    fun `check empty map serialises as Java emptyMap`() = kryoSpecific<MapsSerializationTest>("Specifically checks Kryo serialization") {
         val nameID = 0
         val serializedForm = emptyMap<Int, Int>().serialize()
         val output = ByteArrayOutputStream().apply {