Skip to content

Commit

Permalink
Refine exception messages in case of deserializing data from JsonElem…
Browse files Browse the repository at this point in the history
…ent. (#2648)

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 <sergey.shanshin@jetbrains.com>
  • Loading branch information
sandwwraith and shanshin authored May 14, 2024
1 parent b1dd800 commit e35c28d
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 53 deletions.
1 change: 1 addition & 0 deletions core/api/kotlinx-serialization-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions core/api/kotlinx-serialization-core.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ abstract class kotlinx.serialization.internal/NamedValueDecoder : kotlinx.serial
constructor <init>() // kotlinx.serialization.internal/NamedValueDecoder.<init>|<init>(){}[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]
}
Expand Down
9 changes: 8 additions & 1 deletion core/commonMain/src/kotlinx/serialization/internal/Tagged.kt
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ public abstract class TaggedDecoder<Tag : Any?> : Decoder, CompositeDecoder {
return r
}

private val tagStack = arrayListOf<Tag>()
internal val tagStack: ArrayList<Tag> = arrayListOf()

protected val currentTag: Tag
get() = tagStack.last()
protected val currentTagOrNull: Tag?
Expand Down Expand Up @@ -331,4 +332,10 @@ public abstract class NamedValueDecoder : TaggedDecoder<String>() {
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 = "$.")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}"""
Expand All @@ -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,
Expand All @@ -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")
})
}

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -142,6 +210,23 @@ class JsonErrorMessagesTest : JsonTestBase() {
})
}

@Test
fun testNullLiteralForNotNullNumber() = parametrizedTest { mode ->
val input = """{"boxed":null}"""
val ser = serializer<Box<Int>>()
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":"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,14 @@ internal fun checkKind(kind: SerialKind) {
if (kind is PolymorphicKind) error("Actual serializer for polymorphic cannot be polymorphic itself")
}

internal fun <T> JsonDecoder.decodeSerializableValuePolymorphic(deserializer: DeserializationStrategy<T>): T {
internal inline fun <T> JsonDecoder.decodeSerializableValuePolymorphic(deserializer: DeserializationStrategy<T>, 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<JsonObject>(decodeJsonElement(), deserializer.descriptor)
val jsonTree = cast<JsonObject>(decodeJsonElement(), deserializer.descriptor.serialName, path)
val type = jsonTree[discriminator]?.jsonPrimitive?.contentOrNull // differentiate between `"type":"null"` and `"type":null`.
@Suppress("UNCHECKED_CAST")
val actualSerializer =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(deserializer as DeserializationStrategy<T>)
return decodeSerializableValuePolymorphic<T>(deserializer as DeserializationStrategy<T>) { lexer.path.getPath() }

@Suppress("UNCHECKED_CAST")
val actualSerializer = try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down Expand Up @@ -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 <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T {
return decodeSerializableValuePolymorphic(deserializer)
return decodeSerializableValuePolymorphic(deserializer, ::renderTagStack)
}

override fun composeName(parentName: String, childName: String): String = childName
Expand All @@ -68,95 +71,92 @@ private sealed class AbstractJsonTreeDecoder(
}
}

inline fun <reified T : JsonElement> cast(value: JsonElement, descriptor: SerialDescriptor): T = cast(value, descriptor.serialName) { renderTagStack() }
inline fun <reified T : JsonElement> 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 <T : Any> getPrimitiveValue(tag: String, primitiveName: String, convert: JsonPrimitive.() -> T?): T {
val literal = cast<JsonPrimitive>(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 <T : Any> 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<JsonPrimitive>(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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,12 @@ private class JsonTreeListEncoder(json: Json, nodeConsumer: (JsonElement) -> Uni
override fun getCurrent(): JsonElement = JsonArray(array)
}

@OptIn(ExperimentalSerializationApi::class)
internal inline fun <reified T : JsonElement> cast(value: JsonElement, descriptor: SerialDescriptor): T {
internal inline fun <reified T : JsonElement> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private open class DynamicInput(
}

override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T {
return decodeSerializableValuePolymorphic(deserializer)
return decodeSerializableValuePolymorphic(deserializer, ::renderTagStack)
}

private fun coerceInputValue(descriptor: SerialDescriptor, index: Int, tag: String): Boolean =
Expand Down

0 comments on commit e35c28d

Please sign in to comment.