Skip to content

Commit 73e2296

Browse files
committed
grpc-pb: Address PR comments
Signed-off-by: Johannes Zottele <[email protected]>
1 parent d604bd2 commit 73e2296

File tree

13 files changed

+37
-32
lines changed

13 files changed

+37
-32
lines changed

grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/internal/readPacked.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
package kotlinx.rpc.grpc.internal
66

7-
import kotlinx.rpc.grpc.ProtobufDecodingException
7+
import kotlinx.rpc.grpc.pb.ProtobufDecodingException
88
import kotlinx.rpc.grpc.pb.WireDecoder
99

1010
internal expect fun WireDecoder.pushLimit(byteLen: Int): Int

grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/pb/KTag.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ internal fun KTag.Companion.from(rawKTag: UInt): KTag {
4040
val type = (rawKTag and K_TAG_TYPE_MASK).toInt()
4141
val field = (rawKTag shr K_TAG_TYPE_BITS).toInt()
4242
if (!isValidFieldNr(field)) {
43-
error("Invalid field number: $field")
43+
throw ProtobufDecodingException("Invalid field number: $field")
4444
}
4545
if (type >= WireType.entries.size) {
46-
error("Invalid wire type: $type")
46+
throw ProtobufDecodingException("Invalid wire type: $type")
4747
}
4848
return KTag(field, WireType.entries[type])
4949
}

grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/GrpcException.kt renamed to grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/pb/ProtobufException.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22
* Copyright 2023-2025 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
33
*/
44

5-
package kotlinx.rpc.grpc
5+
package kotlinx.rpc.grpc.pb
66

7-
public sealed class GrpcException : RuntimeException {
7+
public sealed class ProtobufException : RuntimeException {
88
protected constructor(message: String, cause: Throwable? = null) : super(message, cause)
99
}
1010

1111

12-
public class ProtobufDecodingException : GrpcException {
12+
public class ProtobufDecodingException : ProtobufException {
1313
internal constructor(message: String, cause: Throwable? = null) : super(message, cause)
1414

1515
public companion object Companion {
@@ -35,6 +35,6 @@ public class ProtobufDecodingException : GrpcException {
3535
}
3636
}
3737

38-
public class ProtobufEncodingException : GrpcException {
38+
public class ProtobufEncodingException : ProtobufException {
3939
internal constructor(message: String, cause: Throwable? = null) : super(message, cause)
4040
}

grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/pb/WireDecoder.kt

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
package kotlinx.rpc.grpc.pb
66

77
import kotlinx.io.Buffer
8-
import kotlinx.rpc.grpc.ProtobufDecodingException
98
import kotlinx.rpc.grpc.internal.popLimit
109
import kotlinx.rpc.grpc.internal.pushLimit
1110
import kotlinx.rpc.internal.utils.InternalRpcApi
@@ -20,10 +19,12 @@ internal const val MAX_PACKED_BULK_SIZE: Int = 1_000_000
2019
* This decoder is used by first calling [readTag], than looking up the field based on the field number in the returned,
2120
* tag and then calling the actual `read*()` method to read the value to the corresponding field.
2221
*
23-
* [hadError] indicates an error during decoding. While calling `read*()` is safe, the returned values
24-
* are meaningless if [hadError] returns `true`.
22+
* All `read*()` methods will throw an exception if the expected value couldn't be decoded.
23+
* Because of optimization reasons, the exception is platform-dependent. To unify them
24+
* wrap the decoding in a [checkForPlatformDecodeException] call, which turn platform-specific exceptions
25+
* into a [ProtobufDecodingException].
2526
*
26-
* NOTE: If the [hadError] after a call to `read*()` returns `false`, it doesn't mean that the
27+
* NOTE: If a call to `read*()` doesn't throw an error, it doesn't mean that the
2728
* value is correctly decoded. E.g., the following test will pass:
2829
* ```kt
2930
* val fieldNr = 1
@@ -33,10 +34,12 @@ internal const val MAX_PACKED_BULK_SIZE: Int = 1_000_000
3334
* assertTrue(encoder.writeInt32(fieldNr, 12312))
3435
* encoder.flush()
3536
*
36-
* WireDecoder(buffer).use { decoder ->
37-
* decoder.readTag()
38-
* decoder.readBool()
39-
* assertFalse(decoder.hasError())
37+
* checkForPlatformDecodeException {
38+
* WireDecoder(buffer).use { decoder ->
39+
* decoder.readTag()
40+
* decoder.readBool()
41+
* assertFalse(decoder.hasError())
42+
* }
4043
* }
4144
* ```
4245
*/
@@ -92,12 +95,16 @@ public interface WireDecoder : AutoCloseable {
9295
WireType.FIXED32 -> readFixed32()
9396
WireType.FIXED64 -> readFixed64()
9497
WireType.LENGTH_DELIMITED -> readBytes()
95-
WireType.START_GROUP -> error("Unexpected START_GROUP wire type (KRPC-193)")
98+
WireType.START_GROUP -> throw ProtobufDecodingException("Unexpected START_GROUP wire type (KRPC-193)")
9699
WireType.END_GROUP -> {} // nothing to do
97100
}
98101
}
99102
}
100103

104+
/**
105+
* Turns exceptions thrown by different platforms during decoding into [ProtobufDecodingException].
106+
*/
107+
@InternalRpcApi
101108
public expect fun checkForPlatformDecodeException(block: () -> Unit)
102109

103110
/**
@@ -110,4 +117,5 @@ public expect fun checkForPlatformDecodeException(block: () -> Unit)
110117
*
111118
* @param source The buffer containing the encoded wire-format data.
112119
*/
120+
@InternalRpcApi
113121
public expect fun WireDecoder(source: Buffer): WireDecoder

grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/pb/WireEncoder.kt

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ import kotlinx.rpc.internal.utils.InternalRpcApi
1010
/**
1111
* A platform-specific class that encodes values into protobuf's wire format.
1212
*
13-
* If one `write*()` method returns false, the encoding of the value failed
14-
* and no further encodings can be performed on this [WireEncoder].
13+
* If one `write*()` method fails to encode the value in the buffer,
14+
* it will throw a platform-specific exception.
15+
*
16+
* Wrap the encoding of a message with [checkForPlatformEncodeException] to
17+
* turn all thrown platform-specific exceptions into [ProtobufEncodingException]s.
1518
*
1619
* [flush] must be called to ensure that all data is written to the [Sink].
1720
*/
@@ -59,7 +62,11 @@ public interface WireEncoder {
5962

6063
}
6164

62-
65+
/**
66+
* Turns exceptions thrown by different platforms during encoding into [ProtobufEncodingException].
67+
*/
68+
@InternalRpcApi
6369
public expect fun checkForPlatformEncodeException(block: () -> Unit)
6470

71+
@InternalRpcApi
6572
public expect fun WireEncoder(sink: Sink): WireEncoder

grpc/grpc-core/src/commonTest/kotlin/kotlinx/rpc/grpc/pb/ProtosTest.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import asInternal
1313
import encodeWith
1414
import invoke
1515
import kotlinx.io.Buffer
16-
import kotlinx.rpc.grpc.ProtobufDecodingException
1716
import kotlinx.rpc.grpc.codec.MessageCodec
1817
import kotlinx.rpc.grpc.test.*
1918
import kotlinx.rpc.grpc.test.common.*

grpc/grpc-core/src/commonTest/kotlin/kotlinx/rpc/grpc/pb/WireCodecTest.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
package kotlinx.rpc.grpc.pb
66

77
import kotlinx.io.Buffer
8-
import kotlinx.rpc.grpc.ProtobufDecodingException
98
import kotlin.test.*
109

1110
enum class TestPlatform {

grpc/grpc-core/src/jvmMain/kotlin/kotlinx/rpc/grpc/Server.jvm.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ private fun io.grpc.Server.toKotlin(): Server {
3333
override val isTerminated: Boolean
3434
get() = this@toKotlin.isTerminated
3535

36-
override fun start() : Server {
36+
override fun start(): Server {
3737
this@toKotlin.start()
3838
return this
3939
}

grpc/grpc-core/src/jvmMain/kotlin/kotlinx/rpc/grpc/pb/WireDecoder.jvm.kt

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import com.google.protobuf.CodedInputStream
88
import com.google.protobuf.InvalidProtocolBufferException
99
import kotlinx.io.Buffer
1010
import kotlinx.io.asInputStream
11-
import kotlinx.rpc.grpc.ProtobufDecodingException
1211
import kotlinx.rpc.grpc.internal.readPackedVarInternal
1312

1413
internal class WireDecoderJvm(source: Buffer) : WireDecoder {
@@ -119,14 +118,10 @@ public actual fun checkForPlatformDecodeException(block: () -> Unit) {
119118
try {
120119
return block()
121120
} catch (e: InvalidProtocolBufferException) {
122-
throw e.toDecodingException()
121+
throw ProtobufDecodingException(e.message ?: "Failed to decode protobuf message.", e)
123122
}
124123
}
125124

126125
public actual fun WireDecoder(source: Buffer): WireDecoder {
127126
return WireDecoderJvm(source)
128127
}
129-
130-
private fun InvalidProtocolBufferException.toDecodingException(): ProtobufDecodingException {
131-
return ProtobufDecodingException(message ?: "Failed to decode protobuf message.", cause)
132-
}

grpc/grpc-core/src/jvmMain/kotlin/kotlinx/rpc/grpc/pb/WireEncoder.jvm.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import com.google.protobuf.CodedOutputStream
88
import kotlinx.io.IOException
99
import kotlinx.io.Sink
1010
import kotlinx.io.asOutputStream
11-
import kotlinx.rpc.grpc.ProtobufEncodingException
1211

1312
private class WireEncoderJvm(sink: Sink) : WireEncoder {
1413
private val codedOutputStream = CodedOutputStream.newInstance(sink.asOutputStream())

0 commit comments

Comments
 (0)