Skip to content

Commit e35c28d

Browse files
sandwwraithshanshin
sandwwraith
and
shanshin
authored
Refine exception messages in case of deserializing data from JsonElement. (#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>
1 parent b1dd800 commit e35c28d

File tree

9 files changed

+147
-53
lines changed

9 files changed

+147
-53
lines changed

‎core/api/kotlinx-serialization-core.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,7 @@ public abstract class kotlinx/serialization/internal/NamedValueDecoder : kotlinx
908908
public synthetic fun getTag (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/Object;
909909
protected final fun getTag (Lkotlinx/serialization/descriptors/SerialDescriptor;I)Ljava/lang/String;
910910
protected final fun nested (Ljava/lang/String;)Ljava/lang/String;
911+
protected final fun renderTagStack ()Ljava/lang/String;
911912
}
912913

913914
public abstract class kotlinx/serialization/internal/NamedValueEncoder : kotlinx/serialization/internal/TaggedEncoder {

‎core/api/kotlinx-serialization-core.klib.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ abstract class kotlinx.serialization.internal/NamedValueDecoder : kotlinx.serial
212212
constructor <init>() // kotlinx.serialization.internal/NamedValueDecoder.<init>|<init>(){}[0]
213213
final fun (kotlinx.serialization.descriptors/SerialDescriptor).getTag(kotlin/Int): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.getTag|getTag@kotlinx.serialization.descriptors.SerialDescriptor(kotlin.Int){}[0]
214214
final fun nested(kotlin/String): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.nested|nested(kotlin.String){}[0]
215+
final fun renderTagStack(): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.renderTagStack|renderTagStack(){}[0]
215216
open fun composeName(kotlin/String, kotlin/String): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.composeName|composeName(kotlin.String;kotlin.String){}[0]
216217
open fun elementName(kotlinx.serialization.descriptors/SerialDescriptor, kotlin/Int): kotlin/String // kotlinx.serialization.internal/NamedValueDecoder.elementName|elementName(kotlinx.serialization.descriptors.SerialDescriptor;kotlin.Int){}[0]
217218
}

‎core/commonMain/src/kotlinx/serialization/internal/Tagged.kt‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,8 @@ public abstract class TaggedDecoder<Tag : Any?> : Decoder, CompositeDecoder {
299299
return r
300300
}
301301

302-
private val tagStack = arrayListOf<Tag>()
302+
internal val tagStack: ArrayList<Tag> = arrayListOf()
303+
303304
protected val currentTag: Tag
304305
get() = tagStack.last()
305306
protected val currentTagOrNull: Tag?
@@ -331,4 +332,10 @@ public abstract class NamedValueDecoder : TaggedDecoder<String>() {
331332
protected open fun elementName(descriptor: SerialDescriptor, index: Int): String = descriptor.getElementName(index)
332333
protected open fun composeName(parentName: String, childName: String): String =
333334
if (parentName.isEmpty()) childName else "$parentName.$childName"
335+
336+
337+
protected fun renderTagStack(): String {
338+
return if (tagStack.isEmpty()) "$"
339+
else tagStack.joinToString(separator = ".", prefix = "$.")
340+
}
334341
}

‎formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt‎

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,80 @@
66
package kotlinx.serialization.json
77

88
import kotlinx.serialization.*
9+
import kotlinx.serialization.builtins.*
910
import kotlinx.serialization.test.*
1011
import kotlin.test.*
1112

1213

1314
class JsonErrorMessagesTest : JsonTestBase() {
1415

16+
@Serializable
17+
@SerialName("app.Failure")
18+
sealed interface Failure {
19+
@Serializable
20+
@SerialName("a")
21+
data class A(val failure: Failure) : Failure
22+
}
23+
24+
@Test
25+
fun testPolymorphicCastMessage() = parametrizedTest { mode ->
26+
checkSerializationException({
27+
default.decodeFromString(
28+
Failure.serializer(),
29+
"""{"type":"a", "failure":"wrong-input"}""",
30+
mode
31+
)
32+
}, {
33+
assertContains(
34+
it,
35+
"Expected JsonObject, but had JsonLiteral as the serialized body of app.Failure at element: \$.failure"
36+
)
37+
})
38+
}
39+
40+
@Test
41+
fun testPrimitiveInsteadOfObjectOrList() = parametrizedTest { mode ->
42+
val input = """{"boxed": 42}"""
43+
checkSerializationException({
44+
default.decodeFromString(Box.serializer(StringData.serializer()), input, mode)
45+
}, { message ->
46+
if (mode == JsonTestingMode.TREE)
47+
assertContains(message, "Expected JsonObject, but had JsonLiteral as the serialized body of kotlinx.serialization.StringData at element: \$.boxed")
48+
else
49+
assertContains(message, "Unexpected JSON token at offset 10: Expected start of the object '{', but had '4' instead at path: \$.boxed")
50+
})
51+
52+
checkSerializationException({
53+
default.decodeFromString(Box.serializer(ListSerializer(StringData.serializer())), input, mode)
54+
}, { message ->
55+
if (mode == JsonTestingMode.TREE)
56+
assertContains(message, "Expected JsonArray, but had JsonLiteral as the serialized body of kotlin.collections.ArrayList at element: \$.boxed")
57+
else
58+
assertContains(message, "Unexpected JSON token at offset 10: Expected start of the array '[', but had '4' instead at path: \$.boxed")
59+
})
60+
}
61+
62+
@Test
63+
fun testObjectOrListInsteadOfPrimitive() = parametrizedTest { mode ->
64+
checkSerializationException({
65+
default.decodeFromString(Box.serializer(Int.serializer()), """{"boxed": [1,2]}""", mode)
66+
}, { message ->
67+
if (mode == JsonTestingMode.TREE)
68+
assertContains(message, "Expected JsonPrimitive, but had JsonArray as the serialized body of int at element: \$.boxed")
69+
else
70+
assertContains(message, "Unexpected JSON token at offset 10: Expected numeric literal at path: \$.boxed")
71+
})
72+
73+
checkSerializationException({
74+
default.decodeFromString(Box.serializer(String.serializer()), """{"boxed": {"x":"y"}}""", mode)
75+
}, { message ->
76+
if (mode == JsonTestingMode.TREE)
77+
assertContains(message, "Expected JsonPrimitive, but had JsonObject as the serialized body of string at element: \$.boxed")
78+
else
79+
assertContains(message, "Unexpected JSON token at offset 10: Expected beginning of the string, but got { at path: \$.boxed")
80+
})
81+
}
82+
1583
@Test
1684
fun testJsonTokensAreProperlyReported() = parametrizedTest { mode ->
1785
val input1 = """{"boxed":4}"""
@@ -24,7 +92,7 @@ class JsonErrorMessagesTest : JsonTestBase() {
2492
default.decodeFromString(serString, input1, mode)
2593
}, { message ->
2694
if (mode == JsonTestingMode.TREE)
27-
assertContains(message, "String literal for key 'boxed' should be quoted.")
95+
assertContains(message, "String literal for key 'boxed' should be quoted at element: \$.boxed")
2896
else
2997
assertContains(
3098
message,
@@ -42,7 +110,7 @@ class JsonErrorMessagesTest : JsonTestBase() {
42110
"Unexpected JSON token at offset 9: Unexpected symbol 's' in numeric literal at path: \$.boxed"
43111
)
44112
else
45-
assertContains(message, "Failed to parse literal as 'int' value")
113+
assertContains(message, "Failed to parse literal '\"str\"' as an int value at element: \$.boxed")
46114
})
47115
}
48116

@@ -116,7 +184,7 @@ class JsonErrorMessagesTest : JsonTestBase() {
116184
}, { message ->
117185
if (mode == JsonTestingMode.TREE) assertContains(
118186
message,
119-
"""String literal for key 'boxed' should be quoted."""
187+
"String literal for key 'boxed' should be quoted at element: ${'$'}.boxed"
120188
)
121189
else assertContains(
122190
message,
@@ -133,7 +201,7 @@ class JsonErrorMessagesTest : JsonTestBase() {
133201
default.decodeFromString(ser, input, mode)
134202
}, { message ->
135203
if (mode == JsonTestingMode.TREE)
136-
assertContains(message, "Unexpected 'null' literal when non-nullable string was expected")
204+
assertContains(message, "Expected string value for a non-null key 'boxed', got null literal instead at element: \$.boxed")
137205
else
138206
assertContains(
139207
message,
@@ -142,6 +210,23 @@ class JsonErrorMessagesTest : JsonTestBase() {
142210
})
143211
}
144212

213+
@Test
214+
fun testNullLiteralForNotNullNumber() = parametrizedTest { mode ->
215+
val input = """{"boxed":null}"""
216+
val ser = serializer<Box<Int>>()
217+
checkSerializationException({
218+
default.decodeFromString(ser, input, mode)
219+
}, { message ->
220+
if (mode == JsonTestingMode.TREE)
221+
assertContains(message, "Failed to parse literal 'null' as an int value at element: \$.boxed")
222+
else
223+
assertContains(
224+
message,
225+
"Unexpected JSON token at offset 9: Unexpected symbol 'n' in numeric literal at path: \$.boxed"
226+
)
227+
})
228+
}
229+
145230
@Test
146231
fun testEof() = parametrizedTest { mode ->
147232
val input = """{"boxed":"""

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,14 @@ internal fun checkKind(kind: SerialKind) {
7171
if (kind is PolymorphicKind) error("Actual serializer for polymorphic cannot be polymorphic itself")
7272
}
7373

74-
internal fun <T> JsonDecoder.decodeSerializableValuePolymorphic(deserializer: DeserializationStrategy<T>): T {
74+
internal inline fun <T> JsonDecoder.decodeSerializableValuePolymorphic(deserializer: DeserializationStrategy<T>, path: () -> String): T {
7575
// NB: changes in this method should be reflected in StreamingJsonDecoder#decodeSerializableValue
7676
if (deserializer !is AbstractPolymorphicSerializer<*> || json.configuration.useArrayPolymorphism) {
7777
return deserializer.deserialize(this)
7878
}
7979
val discriminator = deserializer.descriptor.classDiscriminator(json)
8080

81-
val jsonTree = cast<JsonObject>(decodeJsonElement(), deserializer.descriptor)
81+
val jsonTree = cast<JsonObject>(decodeJsonElement(), deserializer.descriptor.serialName, path)
8282
val type = jsonTree[discriminator]?.jsonPrimitive?.contentOrNull // differentiate between `"type":"null"` and `"type":null`.
8383
@Suppress("UNCHECKED_CAST")
8484
val actualSerializer =

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ internal open class StreamingJsonDecoder(
7272
val discriminator = deserializer.descriptor.classDiscriminator(json)
7373
val type = lexer.peekLeadingMatchingValue(discriminator, configuration.isLenient)
7474
?: // Fallback to slow path if we haven't found discriminator on first try
75-
return decodeSerializableValuePolymorphic<T>(deserializer as DeserializationStrategy<T>)
75+
return decodeSerializableValuePolymorphic<T>(deserializer as DeserializationStrategy<T>) { lexer.path.getPath() }
7676

7777
@Suppress("UNCHECKED_CAST")
7878
val actualSerializer = try {

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt‎

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package kotlinx.serialization.json.internal
99

1010
import kotlinx.serialization.*
11+
import kotlinx.serialization.builtins.*
1112
import kotlinx.serialization.descriptors.*
1213
import kotlinx.serialization.encoding.*
1314
import kotlinx.serialization.internal.*
@@ -47,10 +48,12 @@ private sealed class AbstractJsonTreeDecoder(
4748

4849
protected fun currentObject() = currentTagOrNull?.let { currentElement(it) } ?: value
4950

51+
fun renderTagStack(currentTag: String) = renderTagStack() + ".$currentTag"
52+
5053
override fun decodeJsonElement(): JsonElement = currentObject()
5154

5255
override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T {
53-
return decodeSerializableValuePolymorphic(deserializer)
56+
return decodeSerializableValuePolymorphic(deserializer, ::renderTagStack)
5457
}
5558

5659
override fun composeName(parentName: String, childName: String): String = childName
@@ -68,95 +71,92 @@ private sealed class AbstractJsonTreeDecoder(
6871
}
6972
}
7073

74+
inline fun <reified T : JsonElement> cast(value: JsonElement, descriptor: SerialDescriptor): T = cast(value, descriptor.serialName) { renderTagStack() }
75+
inline fun <reified T : JsonElement> cast(value: JsonElement, serialName: String, tag: String): T = cast(value, serialName) { renderTagStack(tag) }
76+
7177
override fun endStructure(descriptor: SerialDescriptor) {
7278
// Nothing
7379
}
7480

7581
override fun decodeNotNullMark(): Boolean = currentObject() !is JsonNull
7682

77-
protected fun getPrimitiveValue(tag: String): JsonPrimitive {
78-
val currentElement = currentElement(tag)
79-
return currentElement as? JsonPrimitive ?: throw JsonDecodingException(
80-
-1,
81-
"Expected JsonPrimitive at $tag, found $currentElement", currentObject().toString()
82-
)
83+
@Suppress("NOTHING_TO_INLINE")
84+
protected inline fun getPrimitiveValue(tag: String, descriptor: SerialDescriptor): JsonPrimitive =
85+
cast(currentElement(tag), descriptor.serialName, tag)
86+
87+
private inline fun <T : Any> getPrimitiveValue(tag: String, primitiveName: String, convert: JsonPrimitive.() -> T?): T {
88+
val literal = cast<JsonPrimitive>(currentElement(tag), primitiveName, tag)
89+
try {
90+
return literal.convert() ?: unparsedPrimitive(literal, primitiveName, tag)
91+
} catch (e: IllegalArgumentException) {
92+
// TODO: pass e as cause? (may conflict with #2590)
93+
unparsedPrimitive(literal, primitiveName, tag)
94+
}
95+
}
96+
97+
private fun unparsedPrimitive(literal: JsonPrimitive, primitive: String, tag: String): Nothing {
98+
val type = if (primitive.startsWith("i")) "an $primitive" else "a $primitive"
99+
throw JsonDecodingException(-1, "Failed to parse literal '$literal' as $type value at element: ${renderTagStack(tag)}", currentObject().toString())
83100
}
84101

85102
protected abstract fun currentElement(tag: String): JsonElement
86103

87104
override fun decodeTaggedEnum(tag: String, enumDescriptor: SerialDescriptor): Int =
88-
enumDescriptor.getJsonNameIndexOrThrow(json, getPrimitiveValue(tag).content)
105+
enumDescriptor.getJsonNameIndexOrThrow(json, getPrimitiveValue(tag, enumDescriptor).content)
89106

90107
override fun decodeTaggedNull(tag: String): Nothing? = null
91108

92109
override fun decodeTaggedNotNullMark(tag: String): Boolean = currentElement(tag) !== JsonNull
93110

94-
override fun decodeTaggedBoolean(tag: String): Boolean {
95-
return getPrimitiveValue(tag).primitive("boolean", JsonPrimitive::booleanOrNull)
96-
}
111+
override fun decodeTaggedBoolean(tag: String): Boolean =
112+
getPrimitiveValue(tag, "boolean", JsonPrimitive::booleanOrNull)
97113

98-
override fun decodeTaggedByte(tag: String) = getPrimitiveValue(tag).primitive("byte") {
114+
override fun decodeTaggedByte(tag: String) = getPrimitiveValue(tag, "byte") {
99115
val result = int
100116
if (result in Byte.MIN_VALUE..Byte.MAX_VALUE) result.toByte()
101117
else null
102118
}
103119

104-
override fun decodeTaggedShort(tag: String) = getPrimitiveValue(tag).primitive("short") {
120+
override fun decodeTaggedShort(tag: String) = getPrimitiveValue(tag, "short") {
105121
val result = int
106122
if (result in Short.MIN_VALUE..Short.MAX_VALUE) result.toShort()
107123
else null
108124
}
109125

110-
override fun decodeTaggedInt(tag: String) = getPrimitiveValue(tag).primitive("int") { int }
111-
override fun decodeTaggedLong(tag: String) = getPrimitiveValue(tag).primitive("long") { long }
126+
override fun decodeTaggedInt(tag: String) = getPrimitiveValue(tag, "int") { int }
127+
override fun decodeTaggedLong(tag: String) = getPrimitiveValue(tag, "long") { long }
112128

113129
override fun decodeTaggedFloat(tag: String): Float {
114-
val result = getPrimitiveValue(tag).primitive("float") { float }
130+
val result = getPrimitiveValue(tag, "float") { float }
115131
val specialFp = json.configuration.allowSpecialFloatingPointValues
116132
if (specialFp || result.isFinite()) return result
117133
throw InvalidFloatingPointDecoded(result, tag, currentObject().toString())
118134
}
119135

120136
override fun decodeTaggedDouble(tag: String): Double {
121-
val result = getPrimitiveValue(tag).primitive("double") { double }
137+
val result = getPrimitiveValue(tag, "double") { double }
122138
val specialFp = json.configuration.allowSpecialFloatingPointValues
123139
if (specialFp || result.isFinite()) return result
124140
throw InvalidFloatingPointDecoded(result, tag, currentObject().toString())
125141
}
126142

127-
override fun decodeTaggedChar(tag: String): Char = getPrimitiveValue(tag).primitive("char") { content.single() }
128-
129-
private inline fun <T : Any> JsonPrimitive.primitive(primitive: String, block: JsonPrimitive.() -> T?): T {
130-
try {
131-
return block() ?: unparsedPrimitive(primitive)
132-
} catch (e: IllegalArgumentException) {
133-
unparsedPrimitive(primitive)
134-
}
135-
}
136-
137-
private fun unparsedPrimitive(primitive: String): Nothing {
138-
throw JsonDecodingException(-1, "Failed to parse literal as '$primitive' value", currentObject().toString())
139-
}
143+
override fun decodeTaggedChar(tag: String): Char = getPrimitiveValue(tag, "char") { content.single() }
140144

141145
override fun decodeTaggedString(tag: String): String {
142-
val value = getPrimitiveValue(tag)
143-
if (!json.configuration.isLenient) {
144-
val literal = value.asLiteral("string")
145-
if (!literal.isString) throw JsonDecodingException(
146-
-1, "String literal for key '$tag' should be quoted.\n$lenientHint", currentObject().toString()
146+
val value = cast<JsonPrimitive>(currentElement(tag), "string", tag)
147+
if (value !is JsonLiteral)
148+
throw JsonDecodingException(-1, "Expected string value for a non-null key '$tag', got null literal instead at element: ${renderTagStack(tag)}", currentObject().toString())
149+
if (!value.isString && !json.configuration.isLenient) {
150+
throw JsonDecodingException(
151+
-1, "String literal for key '$tag' should be quoted at element: ${renderTagStack(tag)}.\n$lenientHint", currentObject().toString()
147152
)
148153
}
149-
if (value is JsonNull) throw JsonDecodingException(-1, "Unexpected 'null' value instead of string literal", currentObject().toString())
150154
return value.content
151155
}
152156

153-
private fun JsonPrimitive.asLiteral(type: String): JsonLiteral {
154-
return this as? JsonLiteral ?: throw JsonDecodingException(-1, "Unexpected 'null' literal when non-nullable $type was expected")
155-
}
156-
157157
override fun decodeTaggedInline(tag: String, inlineDescriptor: SerialDescriptor): Decoder {
158158
return if (inlineDescriptor.isUnsignedNumber) {
159-
val lexer = StringJsonLexer(json, getPrimitiveValue(tag).content)
159+
val lexer = StringJsonLexer(json, getPrimitiveValue(tag, inlineDescriptor).content)
160160
JsonDecoderForUnsignedTypes(lexer, json)
161161
} else super.decodeTaggedInline(tag, inlineDescriptor)
162162
}

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonEncoder.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,12 +264,12 @@ private class JsonTreeListEncoder(json: Json, nodeConsumer: (JsonElement) -> Uni
264264
override fun getCurrent(): JsonElement = JsonArray(array)
265265
}
266266

267-
@OptIn(ExperimentalSerializationApi::class)
268-
internal inline fun <reified T : JsonElement> cast(value: JsonElement, descriptor: SerialDescriptor): T {
267+
internal inline fun <reified T : JsonElement> cast(value: JsonElement, serialName: String, path: () -> String): T {
269268
if (value !is T) {
270269
throw JsonDecodingException(
271270
-1,
272-
"Expected ${T::class} as the serialized body of ${descriptor.serialName}, but had ${value::class}"
271+
"Expected ${T::class.simpleName}, but had ${value::class.simpleName} as the serialized body of $serialName at element: ${path()}",
272+
value.toString()
273273
)
274274
}
275275
return value

‎formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ private open class DynamicInput(
6868
}
6969

7070
override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T {
71-
return decodeSerializableValuePolymorphic(deserializer)
71+
return decodeSerializableValuePolymorphic(deserializer, ::renderTagStack)
7272
}
7373

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

0 commit comments

Comments
 (0)