From e35c28dd0ecbf99460635059a39669683bb24cbf Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Tue, 14 May 2024 14:07:12 +0200 Subject: [PATCH] Refine exception messages in case of deserializing data from JsonElement. (#2648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Such a code path is often used when we cannot find type discriminator as a first key in Json (for example, if json input is invalid, and we got a string instead of an object). In such cases, we should display a nice error message. Also add tag stack — equivalent of a Json path — to most of the error messages. Note that it is far from an ideal, since changing between string and tree decoders (such happens in polymorphism) won't preserve stack or path correctly. Yet, it is the best we can do for now. Fixes #2630 Co-authored-by: Sergey Shanshin --- core/api/kotlinx-serialization-core.api | 1 + core/api/kotlinx-serialization-core.klib.api | 1 + .../kotlinx/serialization/internal/Tagged.kt | 9 +- .../json/JsonErrorMessagesTest.kt | 93 ++++++++++++++++++- .../json/internal/Polymorphic.kt | 4 +- .../json/internal/StreamingJsonDecoder.kt | 2 +- .../json/internal/TreeJsonDecoder.kt | 82 ++++++++-------- .../json/internal/TreeJsonEncoder.kt | 6 +- .../json/internal/DynamicDecoders.kt | 2 +- 9 files changed, 147 insertions(+), 53 deletions(-) diff --git a/core/api/kotlinx-serialization-core.api b/core/api/kotlinx-serialization-core.api index 6afa598060..be7625f3cd 100644 --- a/core/api/kotlinx-serialization-core.api +++ b/core/api/kotlinx-serialization-core.api @@ -908,6 +908,7 @@ public abstract class kotlinx/serialization/internal/NamedValueDecoder : kotlinx public synthetic fun getTag (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/Object; protected final fun getTag (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/String; protected final fun nested (Ljava/lang/String;)Ljava/lang/String; + protected final fun renderTagStack ()Ljava/lang/String; } public abstract class kotlinx/serialization/internal/NamedValueEncoder : kotlinx/serialization/internal/TaggedEncoder { diff --git a/core/api/kotlinx-serialization-core.klib.api b/core/api/kotlinx-serialization-core.klib.api index 9727580628..49b89f3d5e 100644 --- a/core/api/kotlinx-serialization-core.klib.api +++ b/core/api/kotlinx-serialization-core.klib.api @@ -212,6 +212,7 @@ abstract class kotlinx.serialization.internal/NamedValueDecoder : kotlinx.serial constructor () // kotlinx.serialization.internal/NamedValueDecoder.|(){}[0] final fun (kotlinx.serialization.descriptors/SerialDescriptor).getTag(kotlin/Int): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.getTag|getTag@kotlinx.serialization.descriptors.SerialDescriptor(kotlin.Int){}[0] final fun nested(kotlin/String): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.nested|nested(kotlin.String){}[0] + final fun renderTagStack(): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.renderTagStack|renderTagStack(){}[0] open fun composeName(kotlin/String, kotlin/String): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.composeName|composeName(kotlin.String;kotlin.String){}[0] open fun elementName(kotlinx.serialization.descriptors/SerialDescriptor, kotlin/Int): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.elementName|elementName(kotlinx.serialization.descriptors.SerialDescriptor;kotlin.Int){}[0] } diff --git a/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt b/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt index cf71388c3a..705cf4544c 100644 --- a/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt +++ b/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt @@ -299,7 +299,8 @@ public abstract class TaggedDecoder : Decoder, CompositeDecoder { return r } - private val tagStack = arrayListOf() + internal val tagStack: ArrayList = arrayListOf() + protected val currentTag: Tag get() = tagStack.last() protected val currentTagOrNull: Tag? @@ -331,4 +332,10 @@ public abstract class NamedValueDecoder : TaggedDecoder() { protected open fun elementName(descriptor: SerialDescriptor, index: Int): String = descriptor.getElementName(index) protected open fun composeName(parentName: String, childName: String): String = if (parentName.isEmpty()) childName else "$parentName.$childName" + + + protected fun renderTagStack(): String { + return if (tagStack.isEmpty()) "$" + else tagStack.joinToString(separator = ".", prefix = "$.") + } } diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt index 08d1eefd7d..da73bc3a21 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt @@ -6,12 +6,80 @@ package kotlinx.serialization.json import kotlinx.serialization.* +import kotlinx.serialization.builtins.* import kotlinx.serialization.test.* import kotlin.test.* class JsonErrorMessagesTest : JsonTestBase() { + @Serializable + @SerialName("app.Failure") + sealed interface Failure { + @Serializable + @SerialName("a") + data class A(val failure: Failure) : Failure + } + + @Test + fun testPolymorphicCastMessage() = parametrizedTest { mode -> + checkSerializationException({ + default.decodeFromString( + Failure.serializer(), + """{"type":"a", "failure":"wrong-input"}""", + mode + ) + }, { + assertContains( + it, + "Expected JsonObject, but had JsonLiteral as the serialized body of app.Failure at element: \$.failure" + ) + }) + } + + @Test + fun testPrimitiveInsteadOfObjectOrList() = parametrizedTest { mode -> + val input = """{"boxed": 42}""" + checkSerializationException({ + default.decodeFromString(Box.serializer(StringData.serializer()), input, mode) + }, { message -> + if (mode == JsonTestingMode.TREE) + assertContains(message, "Expected JsonObject, but had JsonLiteral as the serialized body of kotlinx.serialization.StringData at element: \$.boxed") + else + assertContains(message, "Unexpected JSON token at offset 10: Expected start of the object '{', but had '4' instead at path: \$.boxed") + }) + + checkSerializationException({ + default.decodeFromString(Box.serializer(ListSerializer(StringData.serializer())), input, mode) + }, { message -> + if (mode == JsonTestingMode.TREE) + assertContains(message, "Expected JsonArray, but had JsonLiteral as the serialized body of kotlin.collections.ArrayList at element: \$.boxed") + else + assertContains(message, "Unexpected JSON token at offset 10: Expected start of the array '[', but had '4' instead at path: \$.boxed") + }) + } + + @Test + fun testObjectOrListInsteadOfPrimitive() = parametrizedTest { mode -> + checkSerializationException({ + default.decodeFromString(Box.serializer(Int.serializer()), """{"boxed": [1,2]}""", mode) + }, { message -> + if (mode == JsonTestingMode.TREE) + assertContains(message, "Expected JsonPrimitive, but had JsonArray as the serialized body of int at element: \$.boxed") + else + assertContains(message, "Unexpected JSON token at offset 10: Expected numeric literal at path: \$.boxed") + }) + + checkSerializationException({ + default.decodeFromString(Box.serializer(String.serializer()), """{"boxed": {"x":"y"}}""", mode) + }, { message -> + if (mode == JsonTestingMode.TREE) + assertContains(message, "Expected JsonPrimitive, but had JsonObject as the serialized body of string at element: \$.boxed") + else + assertContains(message, "Unexpected JSON token at offset 10: Expected beginning of the string, but got { at path: \$.boxed") + }) + } + @Test fun testJsonTokensAreProperlyReported() = parametrizedTest { mode -> val input1 = """{"boxed":4}""" @@ -24,7 +92,7 @@ class JsonErrorMessagesTest : JsonTestBase() { default.decodeFromString(serString, input1, mode) }, { message -> if (mode == JsonTestingMode.TREE) - assertContains(message, "String literal for key 'boxed' should be quoted.") + assertContains(message, "String literal for key 'boxed' should be quoted at element: \$.boxed") else assertContains( message, @@ -42,7 +110,7 @@ class JsonErrorMessagesTest : JsonTestBase() { "Unexpected JSON token at offset 9: Unexpected symbol 's' in numeric literal at path: \$.boxed" ) else - assertContains(message, "Failed to parse literal as 'int' value") + assertContains(message, "Failed to parse literal '\"str\"' as an int value at element: \$.boxed") }) } @@ -116,7 +184,7 @@ class JsonErrorMessagesTest : JsonTestBase() { }, { message -> if (mode == JsonTestingMode.TREE) assertContains( message, - """String literal for key 'boxed' should be quoted.""" + "String literal for key 'boxed' should be quoted at element: ${'$'}.boxed" ) else assertContains( message, @@ -133,7 +201,7 @@ class JsonErrorMessagesTest : JsonTestBase() { default.decodeFromString(ser, input, mode) }, { message -> if (mode == JsonTestingMode.TREE) - assertContains(message, "Unexpected 'null' literal when non-nullable string was expected") + assertContains(message, "Expected string value for a non-null key 'boxed', got null literal instead at element: \$.boxed") else assertContains( message, @@ -142,6 +210,23 @@ class JsonErrorMessagesTest : JsonTestBase() { }) } + @Test + fun testNullLiteralForNotNullNumber() = parametrizedTest { mode -> + val input = """{"boxed":null}""" + val ser = serializer>() + checkSerializationException({ + default.decodeFromString(ser, input, mode) + }, { message -> + if (mode == JsonTestingMode.TREE) + assertContains(message, "Failed to parse literal 'null' as an int value at element: \$.boxed") + else + assertContains( + message, + "Unexpected JSON token at offset 9: Unexpected symbol 'n' in numeric literal at path: \$.boxed" + ) + }) + } + @Test fun testEof() = parametrizedTest { mode -> val input = """{"boxed":""" diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt index 6778f51c65..26cdbbcdd0 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt @@ -71,14 +71,14 @@ internal fun checkKind(kind: SerialKind) { if (kind is PolymorphicKind) error("Actual serializer for polymorphic cannot be polymorphic itself") } -internal fun JsonDecoder.decodeSerializableValuePolymorphic(deserializer: DeserializationStrategy): T { +internal inline fun JsonDecoder.decodeSerializableValuePolymorphic(deserializer: DeserializationStrategy, path: () -> String): T { // NB: changes in this method should be reflected in StreamingJsonDecoder#decodeSerializableValue if (deserializer !is AbstractPolymorphicSerializer<*> || json.configuration.useArrayPolymorphism) { return deserializer.deserialize(this) } val discriminator = deserializer.descriptor.classDiscriminator(json) - val jsonTree = cast(decodeJsonElement(), deserializer.descriptor) + val jsonTree = cast(decodeJsonElement(), deserializer.descriptor.serialName, path) val type = jsonTree[discriminator]?.jsonPrimitive?.contentOrNull // differentiate between `"type":"null"` and `"type":null`. @Suppress("UNCHECKED_CAST") val actualSerializer = diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt index bd0b9afa8f..ee813b314e 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt @@ -72,7 +72,7 @@ internal open class StreamingJsonDecoder( val discriminator = deserializer.descriptor.classDiscriminator(json) val type = lexer.peekLeadingMatchingValue(discriminator, configuration.isLenient) ?: // Fallback to slow path if we haven't found discriminator on first try - return decodeSerializableValuePolymorphic(deserializer as DeserializationStrategy) + return decodeSerializableValuePolymorphic(deserializer as DeserializationStrategy) { lexer.path.getPath() } @Suppress("UNCHECKED_CAST") val actualSerializer = try { diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt index 9f5844c80f..ec06db61f2 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt @@ -8,6 +8,7 @@ package kotlinx.serialization.json.internal import kotlinx.serialization.* +import kotlinx.serialization.builtins.* import kotlinx.serialization.descriptors.* import kotlinx.serialization.encoding.* import kotlinx.serialization.internal.* @@ -47,10 +48,12 @@ private sealed class AbstractJsonTreeDecoder( protected fun currentObject() = currentTagOrNull?.let { currentElement(it) } ?: value + fun renderTagStack(currentTag: String) = renderTagStack() + ".$currentTag" + override fun decodeJsonElement(): JsonElement = currentObject() override fun decodeSerializableValue(deserializer: DeserializationStrategy): T { - return decodeSerializableValuePolymorphic(deserializer) + return decodeSerializableValuePolymorphic(deserializer, ::renderTagStack) } override fun composeName(parentName: String, childName: String): String = childName @@ -68,95 +71,92 @@ private sealed class AbstractJsonTreeDecoder( } } + inline fun cast(value: JsonElement, descriptor: SerialDescriptor): T = cast(value, descriptor.serialName) { renderTagStack() } + inline fun cast(value: JsonElement, serialName: String, tag: String): T = cast(value, serialName) { renderTagStack(tag) } + override fun endStructure(descriptor: SerialDescriptor) { // Nothing } override fun decodeNotNullMark(): Boolean = currentObject() !is JsonNull - protected fun getPrimitiveValue(tag: String): JsonPrimitive { - val currentElement = currentElement(tag) - return currentElement as? JsonPrimitive ?: throw JsonDecodingException( - -1, - "Expected JsonPrimitive at $tag, found $currentElement", currentObject().toString() - ) + @Suppress("NOTHING_TO_INLINE") + protected inline fun getPrimitiveValue(tag: String, descriptor: SerialDescriptor): JsonPrimitive = + cast(currentElement(tag), descriptor.serialName, tag) + + private inline fun getPrimitiveValue(tag: String, primitiveName: String, convert: JsonPrimitive.() -> T?): T { + val literal = cast(currentElement(tag), primitiveName, tag) + try { + return literal.convert() ?: unparsedPrimitive(literal, primitiveName, tag) + } catch (e: IllegalArgumentException) { + // TODO: pass e as cause? (may conflict with #2590) + unparsedPrimitive(literal, primitiveName, tag) + } + } + + private fun unparsedPrimitive(literal: JsonPrimitive, primitive: String, tag: String): Nothing { + val type = if (primitive.startsWith("i")) "an $primitive" else "a $primitive" + throw JsonDecodingException(-1, "Failed to parse literal '$literal' as $type value at element: ${renderTagStack(tag)}", currentObject().toString()) } protected abstract fun currentElement(tag: String): JsonElement override fun decodeTaggedEnum(tag: String, enumDescriptor: SerialDescriptor): Int = - enumDescriptor.getJsonNameIndexOrThrow(json, getPrimitiveValue(tag).content) + enumDescriptor.getJsonNameIndexOrThrow(json, getPrimitiveValue(tag, enumDescriptor).content) override fun decodeTaggedNull(tag: String): Nothing? = null override fun decodeTaggedNotNullMark(tag: String): Boolean = currentElement(tag) !== JsonNull - override fun decodeTaggedBoolean(tag: String): Boolean { - return getPrimitiveValue(tag).primitive("boolean", JsonPrimitive::booleanOrNull) - } + override fun decodeTaggedBoolean(tag: String): Boolean = + getPrimitiveValue(tag, "boolean", JsonPrimitive::booleanOrNull) - override fun decodeTaggedByte(tag: String) = getPrimitiveValue(tag).primitive("byte") { + override fun decodeTaggedByte(tag: String) = getPrimitiveValue(tag, "byte") { val result = int if (result in Byte.MIN_VALUE..Byte.MAX_VALUE) result.toByte() else null } - override fun decodeTaggedShort(tag: String) = getPrimitiveValue(tag).primitive("short") { + override fun decodeTaggedShort(tag: String) = getPrimitiveValue(tag, "short") { val result = int if (result in Short.MIN_VALUE..Short.MAX_VALUE) result.toShort() else null } - override fun decodeTaggedInt(tag: String) = getPrimitiveValue(tag).primitive("int") { int } - override fun decodeTaggedLong(tag: String) = getPrimitiveValue(tag).primitive("long") { long } + override fun decodeTaggedInt(tag: String) = getPrimitiveValue(tag, "int") { int } + override fun decodeTaggedLong(tag: String) = getPrimitiveValue(tag, "long") { long } override fun decodeTaggedFloat(tag: String): Float { - val result = getPrimitiveValue(tag).primitive("float") { float } + val result = getPrimitiveValue(tag, "float") { float } val specialFp = json.configuration.allowSpecialFloatingPointValues if (specialFp || result.isFinite()) return result throw InvalidFloatingPointDecoded(result, tag, currentObject().toString()) } override fun decodeTaggedDouble(tag: String): Double { - val result = getPrimitiveValue(tag).primitive("double") { double } + val result = getPrimitiveValue(tag, "double") { double } val specialFp = json.configuration.allowSpecialFloatingPointValues if (specialFp || result.isFinite()) return result throw InvalidFloatingPointDecoded(result, tag, currentObject().toString()) } - override fun decodeTaggedChar(tag: String): Char = getPrimitiveValue(tag).primitive("char") { content.single() } - - private inline fun JsonPrimitive.primitive(primitive: String, block: JsonPrimitive.() -> T?): T { - try { - return block() ?: unparsedPrimitive(primitive) - } catch (e: IllegalArgumentException) { - unparsedPrimitive(primitive) - } - } - - private fun unparsedPrimitive(primitive: String): Nothing { - throw JsonDecodingException(-1, "Failed to parse literal as '$primitive' value", currentObject().toString()) - } + override fun decodeTaggedChar(tag: String): Char = getPrimitiveValue(tag, "char") { content.single() } override fun decodeTaggedString(tag: String): String { - val value = getPrimitiveValue(tag) - if (!json.configuration.isLenient) { - val literal = value.asLiteral("string") - if (!literal.isString) throw JsonDecodingException( - -1, "String literal for key '$tag' should be quoted.\n$lenientHint", currentObject().toString() + val value = cast(currentElement(tag), "string", tag) + if (value !is JsonLiteral) + throw JsonDecodingException(-1, "Expected string value for a non-null key '$tag', got null literal instead at element: ${renderTagStack(tag)}", currentObject().toString()) + if (!value.isString && !json.configuration.isLenient) { + throw JsonDecodingException( + -1, "String literal for key '$tag' should be quoted at element: ${renderTagStack(tag)}.\n$lenientHint", currentObject().toString() ) } - if (value is JsonNull) throw JsonDecodingException(-1, "Unexpected 'null' value instead of string literal", currentObject().toString()) return value.content } - private fun JsonPrimitive.asLiteral(type: String): JsonLiteral { - return this as? JsonLiteral ?: throw JsonDecodingException(-1, "Unexpected 'null' literal when non-nullable $type was expected") - } - override fun decodeTaggedInline(tag: String, inlineDescriptor: SerialDescriptor): Decoder { return if (inlineDescriptor.isUnsignedNumber) { - val lexer = StringJsonLexer(json, getPrimitiveValue(tag).content) + val lexer = StringJsonLexer(json, getPrimitiveValue(tag, inlineDescriptor).content) JsonDecoderForUnsignedTypes(lexer, json) } else super.decodeTaggedInline(tag, inlineDescriptor) } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonEncoder.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonEncoder.kt index 32ec571434..66b7f3c6e8 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonEncoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonEncoder.kt @@ -264,12 +264,12 @@ private class JsonTreeListEncoder(json: Json, nodeConsumer: (JsonElement) -> Uni override fun getCurrent(): JsonElement = JsonArray(array) } -@OptIn(ExperimentalSerializationApi::class) -internal inline fun cast(value: JsonElement, descriptor: SerialDescriptor): T { +internal inline fun cast(value: JsonElement, serialName: String, path: () -> String): T { if (value !is T) { throw JsonDecodingException( -1, - "Expected ${T::class} as the serialized body of ${descriptor.serialName}, but had ${value::class}" + "Expected ${T::class.simpleName}, but had ${value::class.simpleName} as the serialized body of $serialName at element: ${path()}", + value.toString() ) } return value diff --git a/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt b/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt index 1ff1e40daa..7ce0f709cc 100644 --- a/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt +++ b/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt @@ -68,7 +68,7 @@ private open class DynamicInput( } override fun decodeSerializableValue(deserializer: DeserializationStrategy): T { - return decodeSerializableValuePolymorphic(deserializer) + return decodeSerializableValuePolymorphic(deserializer, ::renderTagStack) } private fun coerceInputValue(descriptor: SerialDescriptor, index: Int, tag: String): Boolean =