Skip to content

Commit 0b5145c

Browse files
xiaozhikang0916xiaozhikang0916
authored
Improve readability of protobuf decoding exception messages (#2768)
- Wrap definition of wire types with enum class ProtoWireType to show both name and id in message, - Catch and rethrow any ProtobufDecodingException in most decodeXXX functions, with proto number and type name in new exception message.
1 parent b931598 commit 0b5145c

File tree

8 files changed

+530
-160
lines changed

8 files changed

+530
-160
lines changed

‎formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/Helpers.kt‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,29 @@ import kotlinx.serialization.modules.*
1212
import kotlinx.serialization.protobuf.*
1313

1414
internal typealias ProtoDesc = Long
15-
internal const val VARINT = 0
16-
internal const val i64 = 1
17-
internal const val SIZE_DELIMITED = 2
18-
internal const val i32 = 5
15+
16+
internal enum class ProtoWireType(val typeId: Int) {
17+
INVALID(-1),
18+
VARINT(0),
19+
i64(1),
20+
SIZE_DELIMITED(2),
21+
i32(5),
22+
;
23+
24+
companion object {
25+
fun from(typeId: Int): ProtoWireType {
26+
return ProtoWireType.entries.find { it.typeId == typeId } ?: INVALID
27+
}
28+
}
29+
30+
fun wireIntWithTag(tag: Int): Int {
31+
return ((tag shl 3) or typeId)
32+
}
33+
34+
override fun toString(): String {
35+
return "${this.name}($typeId)"
36+
}
37+
}
1938

2039
internal const val ID_HOLDER_ONE_OF = -2
2140

@@ -104,7 +123,7 @@ internal fun extractProtoId(descriptor: SerialDescriptor, index: Int, zeroBasedD
104123
return result
105124
}
106125

107-
internal class ProtobufDecodingException(message: String) : SerializationException(message)
126+
internal class ProtobufDecodingException(message: String, e: Throwable? = null) : SerializationException(message, e)
108127

109128
internal expect fun Int.reverseBytes(): Int
110129
internal expect fun Long.reverseBytes(): Long

‎formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/ProtobufDecoding.kt‎

Lines changed: 150 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -122,41 +122,53 @@ internal open class ProtobufDecoder(
122122
}
123123

124124
override fun beginStructure(descriptor: SerialDescriptor): CompositeDecoder {
125-
return when (descriptor.kind) {
126-
StructureKind.LIST -> {
127-
val tag = currentTagOrDefault
128-
return if (this.descriptor.kind == StructureKind.LIST && tag != MISSING_TAG && this.descriptor != descriptor) {
129-
val reader = makeDelimited(reader, tag)
130-
// repeated decoder expects the first tag to be read already
131-
reader.readTag()
132-
// all elements always have id = 1
133-
RepeatedDecoder(proto, reader, ProtoDesc(1, ProtoIntegerType.DEFAULT), descriptor)
134-
135-
} else if (reader.currentType == SIZE_DELIMITED && descriptor.getElementDescriptor(0).isPackable) {
136-
val sliceReader = ProtobufReader(reader.objectInput())
137-
PackedArrayDecoder(proto, sliceReader, descriptor)
138-
139-
} else {
140-
RepeatedDecoder(proto, reader, tag, descriptor)
125+
return try {
126+
when (descriptor.kind) {
127+
StructureKind.LIST -> {
128+
val tag = currentTagOrDefault
129+
return if (this.descriptor.kind == StructureKind.LIST && tag != MISSING_TAG && this.descriptor != descriptor) {
130+
val reader = makeDelimited(reader, tag)
131+
// repeated decoder expects the first tag to be read already
132+
reader.readTag()
133+
// all elements always have id = 1
134+
RepeatedDecoder(proto, reader, ProtoDesc(1, ProtoIntegerType.DEFAULT), descriptor)
135+
136+
} else if (reader.currentType == ProtoWireType.SIZE_DELIMITED && descriptor.getElementDescriptor(0).isPackable) {
137+
val sliceReader = ProtobufReader(reader.objectInput())
138+
PackedArrayDecoder(proto, sliceReader, descriptor)
139+
140+
} else {
141+
RepeatedDecoder(proto, reader, tag, descriptor)
142+
}
141143
}
142-
}
143-
StructureKind.CLASS, StructureKind.OBJECT, is PolymorphicKind -> {
144-
val tag = currentTagOrDefault
145-
// Do not create redundant copy
146-
if (tag == MISSING_TAG && this.descriptor == descriptor) return this
147-
if (tag.isOneOf) {
148-
// If a tag is annotated as oneof
149-
// [tag.protoId] here is overwritten with index-based default id in
150-
// [kotlinx.serialization.protobuf.internal.HelpersKt.extractParameters]
151-
// and restored the real id from index2IdMap, set by [decodeElementIndex]
152-
val rawIndex = tag.protoId - 1
153-
val restoredTag = index2IdMap?.get(rawIndex)?.let { tag.overrideId(it) } ?: tag
154-
return OneOfPolymorphicReader(proto, reader, restoredTag, descriptor)
144+
145+
StructureKind.CLASS, StructureKind.OBJECT, is PolymorphicKind -> {
146+
val tag = currentTagOrDefault
147+
// Do not create redundant copy
148+
if (tag == MISSING_TAG && this.descriptor == descriptor) return this
149+
if (tag.isOneOf) {
150+
// If a tag is annotated as oneof
151+
// [tag.protoId] here is overwritten with index-based default id in
152+
// [kotlinx.serialization.protobuf.internal.HelpersKt.extractParameters]
153+
// and restored the real id from index2IdMap, set by [decodeElementIndex]
154+
val rawIndex = tag.protoId - 1
155+
val restoredTag = index2IdMap?.get(rawIndex)?.let { tag.overrideId(it) } ?: tag
156+
return OneOfPolymorphicReader(proto, reader, restoredTag, descriptor)
157+
}
158+
return ProtobufDecoder(proto, makeDelimited(reader, tag), descriptor)
155159
}
156-
return ProtobufDecoder(proto, makeDelimited(reader, tag), descriptor)
160+
161+
StructureKind.MAP -> MapEntryReader(
162+
proto,
163+
makeDelimitedForced(reader, currentTagOrDefault),
164+
currentTagOrDefault,
165+
descriptor
166+
)
167+
168+
else -> throw SerializationException("Primitives are not supported at top-level")
157169
}
158-
StructureKind.MAP -> MapEntryReader(proto, makeDelimitedForced(reader, currentTagOrDefault), currentTagOrDefault, descriptor)
159-
else -> throw SerializationException("Primitives are not supported at top-level")
170+
} catch (e: ProtobufDecodingException) {
171+
throw ProtobufDecodingException("Fail to begin structure for ${descriptor.serialName} in ${this.descriptor.serialName} at proto number ${currentTagOrDefault.protoId}", e)
160172
}
161173
}
162174

@@ -173,41 +185,51 @@ internal open class ProtobufDecoder(
173185
override fun decodeTaggedByte(tag: ProtoDesc): Byte = decodeTaggedInt(tag).toByte()
174186
override fun decodeTaggedShort(tag: ProtoDesc): Short = decodeTaggedInt(tag).toShort()
175187
override fun decodeTaggedInt(tag: ProtoDesc): Int {
176-
return if (tag == MISSING_TAG) {
177-
reader.readInt32NoTag()
178-
} else {
179-
reader.readInt(tag.integerType)
188+
return decodeOrThrow(tag) {
189+
if (tag == MISSING_TAG) {
190+
reader.readInt32NoTag()
191+
} else {
192+
reader.readInt(tag.integerType)
193+
}
180194
}
181195
}
182196
override fun decodeTaggedLong(tag: ProtoDesc): Long {
183-
return if (tag == MISSING_TAG) {
184-
reader.readLongNoTag()
185-
} else {
186-
reader.readLong(tag.integerType)
197+
return decodeOrThrow(tag) {
198+
if (tag == MISSING_TAG) {
199+
reader.readLongNoTag()
200+
} else {
201+
reader.readLong(tag.integerType)
202+
}
187203
}
188204
}
189205

190206
override fun decodeTaggedFloat(tag: ProtoDesc): Float {
191-
return if (tag == MISSING_TAG) {
192-
reader.readFloatNoTag()
193-
} else {
194-
reader.readFloat()
207+
return decodeOrThrow(tag) {
208+
if (tag == MISSING_TAG) {
209+
reader.readFloatNoTag()
210+
} else {
211+
reader.readFloat()
212+
}
195213
}
196214
}
197215
override fun decodeTaggedDouble(tag: ProtoDesc): Double {
198-
return if (tag == MISSING_TAG) {
199-
reader.readDoubleNoTag()
200-
} else {
201-
reader.readDouble()
216+
return decodeOrThrow(tag) {
217+
if (tag == MISSING_TAG) {
218+
reader.readDoubleNoTag()
219+
} else {
220+
reader.readDouble()
221+
}
202222
}
203223
}
204224
override fun decodeTaggedChar(tag: ProtoDesc): Char = decodeTaggedInt(tag).toChar()
205225

206226
override fun decodeTaggedString(tag: ProtoDesc): String {
207-
return if (tag == MISSING_TAG) {
208-
reader.readStringNoTag()
209-
} else {
210-
reader.readString()
227+
return decodeOrThrow(tag) {
228+
if (tag == MISSING_TAG) {
229+
reader.readStringNoTag()
230+
} else {
231+
reader.readString()
232+
}
211233
}
212234
}
213235

@@ -218,22 +240,49 @@ internal open class ProtobufDecoder(
218240
override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T = decodeSerializableValue(deserializer, null)
219241

220242
@Suppress("UNCHECKED_CAST")
221-
override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>, previousValue: T?): T = when {
222-
deserializer is MapLikeSerializer<*, *, *, *> -> {
223-
deserializeMap(deserializer as DeserializationStrategy<T>, previousValue)
243+
override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>, previousValue: T?): T = try {
244+
when {
245+
deserializer is MapLikeSerializer<*, *, *, *> -> {
246+
deserializeMap(deserializer as DeserializationStrategy<T>, previousValue)
247+
}
248+
249+
deserializer.descriptor == ByteArraySerializer().descriptor -> deserializeByteArray(previousValue as ByteArray?) as T
250+
deserializer is AbstractCollectionSerializer<*, *, *> ->
251+
(deserializer as AbstractCollectionSerializer<*, T, *>).merge(this, previousValue)
252+
253+
else -> deserializer.deserialize(this)
254+
}
255+
} catch (e: ProtobufDecodingException) {
256+
val currentTag = currentTagOrDefault
257+
val msg = if (descriptor != deserializer.descriptor) {
258+
// Decoding child element
259+
if (descriptor.kind == StructureKind.LIST && deserializer.descriptor.kind != StructureKind.MAP) {
260+
// Decoding repeated field
261+
"Error while decoding index ${currentTag.protoId - 1} in repeated field of ${deserializer.descriptor.serialName}"
262+
} else if (descriptor.kind == StructureKind.MAP) {
263+
// Decoding map field
264+
val index = (currentTag.protoId - 1) / 2
265+
val field = if ((currentTag.protoId - 1) % 2 == 0) { "key" } else "value"
266+
"Error while decoding $field of index $index in map field of ${deserializer.descriptor.serialName}"
267+
} else {
268+
// Decoding common class
269+
"Error while decoding ${deserializer.descriptor.serialName} at proto number ${currentTag.protoId} of ${descriptor.serialName}"
270+
}
271+
} else {
272+
// Decoding self
273+
"Error while decoding ${descriptor.serialName}"
224274
}
225-
deserializer.descriptor == ByteArraySerializer().descriptor -> deserializeByteArray(previousValue as ByteArray?) as T
226-
deserializer is AbstractCollectionSerializer<*, *, *> ->
227-
(deserializer as AbstractCollectionSerializer<*, T, *>).merge(this, previousValue)
228-
else -> deserializer.deserialize(this)
275+
throw ProtobufDecodingException(msg, e)
229276
}
230277

231278
private fun deserializeByteArray(previousValue: ByteArray?): ByteArray {
232279
val tag = currentTagOrDefault
233-
val array = if (tag == MISSING_TAG) {
234-
reader.readByteArrayNoTag()
235-
} else {
236-
reader.readByteArray()
280+
val array = decodeOrThrow(tag) {
281+
if (tag == MISSING_TAG) {
282+
reader.readByteArrayNoTag()
283+
} else {
284+
reader.readByteArray()
285+
}
237286
}
238287
return if (previousValue == null) array else previousValue + array
239288
}
@@ -252,29 +301,33 @@ internal open class ProtobufDecoder(
252301
override fun SerialDescriptor.getTag(index: Int) = extractParameters(index)
253302

254303
override fun decodeElementIndex(descriptor: SerialDescriptor): Int {
255-
while (true) {
256-
val protoId = reader.readTag()
257-
if (protoId == -1) { // EOF
258-
return elementMarker.nextUnmarkedIndex()
259-
}
260-
val index = getIndexByNum(protoId)
261-
if (index == -1) { // not found
262-
reader.skipElement()
263-
} else {
264-
if (descriptor.extractParameters(index).isOneOf) {
265-
/**
266-
* While decoding message with one-of field,
267-
* the proto id read from wire data cannot be easily found
268-
* in the properties of this type,
269-
* So the index of this one-of property and the id read from the wire
270-
* are saved in this map, then restored in [beginStructure]
271-
* and passed to [OneOfPolymorphicReader] to get the actual deserializer.
272-
*/
273-
index2IdMap?.put(index, protoId)
304+
try {
305+
while (true) {
306+
val protoId = reader.readTag()
307+
if (protoId == -1) { // EOF
308+
return elementMarker.nextUnmarkedIndex()
309+
}
310+
val index = getIndexByNum(protoId)
311+
if (index == -1) { // not found
312+
reader.skipElement()
313+
} else {
314+
if (descriptor.extractParameters(index).isOneOf) {
315+
/**
316+
* While decoding message with one-of field,
317+
* the proto id read from wire data cannot be easily found
318+
* in the properties of this type,
319+
* So the index of this one-of property and the id read from the wire
320+
* are saved in this map, then restored in [beginStructure]
321+
* and passed to [OneOfPolymorphicReader] to get the actual deserializer.
322+
*/
323+
index2IdMap?.put(index, protoId)
324+
}
325+
elementMarker.mark(index)
326+
return index
274327
}
275-
elementMarker.mark(index)
276-
return index
277328
}
329+
} catch (e: ProtobufDecodingException) {
330+
throw ProtobufDecodingException("Fail to get element index for ${descriptor.serialName} in ${this.descriptor.serialName}", e)
278331
}
279332
}
280333

@@ -296,6 +349,19 @@ internal open class ProtobufDecoder(
296349
}
297350
return false
298351
}
352+
353+
private inline fun <T> decodeOrThrow(tag: ProtoDesc, action: (tag: ProtoDesc) -> T): T {
354+
try {
355+
return action(tag)
356+
} catch (e: ProtobufDecodingException) {
357+
rethrowException(tag, e)
358+
}
359+
}
360+
361+
@Suppress("NOTHING_TO_INLINE")
362+
private inline fun rethrowException(tag: ProtoDesc, e: ProtobufDecodingException): Nothing {
363+
throw ProtobufDecodingException("Error while decoding proto number ${tag.protoId} of ${descriptor.serialName}", e)
364+
}
299365
}
300366

301367
private class RepeatedDecoder(

0 commit comments

Comments
 (0)