Skip to content

Commit a33196d

Browse files
committed
Fix errors serialization
1 parent 64454e8 commit a33196d

File tree

8 files changed

+52
-96
lines changed

8 files changed

+52
-96
lines changed

README.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,13 @@ Diagnostic data from the server itself is written to the log file `out/out.log`.
150150

151151
### Conformance tests status
152152

153-
Current status: 11/79 tests pass.
153+
Current status: __77/79__ tests pass.
154154

155155
Known issues:
156156

157-
* `google.protobuf.Any` serialization [doesn't follow](https://github.com/connectrpc/conformance/issues/948) Connect-RPC
158-
spec: [#32](https://github.com/igor-vovk/connect-rpc-scala/issues/32)
157+
* Response headers are ignored in case of an error from the server
159158

160159
## Future improvements
161160

162-
[x] Support GET-requests
163-
[ ] Support non-unary (streaming) methods
161+
- [x] Support GET-requests
162+
- [ ] Support non-unary (streaming) methods

conformance/src/main/scala/org/ivovk/connect_rpc_scala/conformance/ConformanceServiceImpl.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class ConformanceServiceImpl[F[_] : Async] extends ConformanceServiceFs2GrpcTrai
6969
val status = Status.fromCodeValue(code.value)
7070
.withDescription(message.orNull)
7171
.augmentDescription(
72-
TextFormat.printToSingleLineUnicodeString(requestInfo.toProtoAny)
72+
TextFormat.printToSingleLineUnicodeString(requestInfo.toProtoErrorAny)
7373
)
7474

7575
throw new StatusRuntimeException(status)

conformance/src/main/scala/org/ivovk/connect_rpc_scala/conformance/Main.scala

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@ import cats.effect.{IO, IOApp, Sync}
44
import com.comcast.ip4s.{Port, host, port}
55
import connectrpc.conformance.v1.{ConformanceServiceFs2GrpcTrailers, ServerCompatRequest, ServerCompatResponse}
66
import org.http4s.ember.server.EmberServerBuilder
7-
import org.ivovk.connect_rpc_scala.http.ConnectAnyFormat
87
import org.ivovk.connect_rpc_scala.ConnectRouteBuilder
9-
import scalapb.json4s.{AnyFormat, JsonFormat, TypeRegistry}
8+
import scalapb.json4s.TypeRegistry
109

1110
import java.io.InputStream
1211
import java.nio.ByteBuffer
@@ -46,13 +45,7 @@ object Main extends IOApp.Simple {
4645
.addMessage[connectrpc.conformance.v1.UnaryRequest]
4746
.addMessage[connectrpc.conformance.v1.IdempotentUnaryRequest]
4847
.addMessage[connectrpc.conformance.v1.ConformancePayload.RequestInfo]
49-
).withFormatRegistry(
50-
JsonFormat.DefaultRegistry
51-
.registerMessageFormatter[com.google.protobuf.any.Any](
52-
ConnectAnyFormat.anyWriter,
53-
AnyFormat.anyParser
54-
)
55-
)
48+
)
5649
}
5750
.build
5851

core/src/main/scala/org/ivovk/connect_rpc_scala/ConnectHandler.scala

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.ivovk.connect_rpc_scala
22

3-
import cats.Endo
43
import cats.data.EitherT
54
import cats.effect.Async
65
import cats.implicits.*
@@ -111,12 +110,12 @@ class ConnectHandler[F[_] : Async](
111110
MetadataUtils.newCaptureMetadataInterceptor(responseHeaderMetadata, responseTrailerMetadata),
112111
),
113112
method.descriptor,
114-
CallOptions.DEFAULT
115-
.pipe(
116-
req.timeout.fold[Endo[CallOptions]](identity) { timeout =>
117-
_.withDeadlineAfter(timeout, MILLISECONDS)
118-
}
119-
),
113+
CallOptions.DEFAULT.pipe(
114+
req.timeout match {
115+
case Some(timeout) => _.withDeadlineAfter(timeout, MILLISECONDS)
116+
case None => identity
117+
}
118+
),
120119
message
121120
)
122121
}).map { response =>
@@ -166,8 +165,8 @@ class ConnectHandler[F[_] : Async](
166165
(messageParts.mkString("\n"), details)
167166
)
168167

169-
//val message = messageWithDetails.map(_._1)
170-
//val details = messageWithDetails.map(_._2).getOrElse(Seq.empty)
168+
val message = messageWithDetails.map(_._1)
169+
val details = messageWithDetails.map(_._2).getOrElse(Seq.empty)
171170

172171
val httpStatus = grpcStatus.toHttpStatus
173172
val connectCode = grpcStatus.toConnectCode
@@ -179,7 +178,7 @@ class ConnectHandler[F[_] : Async](
179178

180179
Response[F](httpStatus).withEntity(connectrpc.Error(
181180
code = connectCode,
182-
message = messageWithDetails.map(_._1),
181+
message = message,
183182
details = details
184183
))
185184
}

core/src/main/scala/org/ivovk/connect_rpc_scala/ConnectRouteBuilder.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@ import org.http4s.{HttpApp, HttpRoutes, Method}
99
import org.ivovk.connect_rpc_scala.grpc.*
1010
import org.ivovk.connect_rpc_scala.http.*
1111
import org.ivovk.connect_rpc_scala.http.QueryParams.*
12+
import org.ivovk.connect_rpc_scala.http.json.ConnectJsonRegistry
1213
import scalapb.json4s.{JsonFormat, Printer}
1314

1415
import java.util.concurrent.Executor
1516
import scala.concurrent.ExecutionContext
1617
import scala.concurrent.duration.*
18+
import scala.util.chaining.*
1719

1820
object ConnectRouteBuilder {
1921

@@ -75,7 +77,9 @@ case class ConnectRouteBuilder[F[_] : Async] private(
7577
import httpDsl.*
7678

7779
val compressor = Compressor[F]
78-
val jsonPrinter = jsonPrinterConfigurator(JsonFormat.printer)
80+
val jsonPrinter = JsonFormat.printer
81+
.withFormatRegistry(ConnectJsonRegistry.default)
82+
.pipe(jsonPrinterConfigurator)
7983

8084
val codecRegistry = MessageCodecRegistry[F](
8185
JsonMessageCodec[F](compressor, jsonPrinter),

core/src/main/scala/org/ivovk/connect_rpc_scala/Mappings.scala

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,16 @@ trait StatusCodeMappings {
108108
trait ProtoMappings {
109109

110110
extension [T <: GeneratedMessage](t: T) {
111-
def toProtoAny: com.google.protobuf.any.Any =
111+
private def any(typeUrlPrefix: String = "type.googleapis.com/"): com.google.protobuf.any.Any =
112112
com.google.protobuf.any.Any(
113-
typeUrl = "type.googleapis.com/" + t.companion.scalaDescriptor.fullName,
113+
typeUrl = typeUrlPrefix + t.companion.scalaDescriptor.fullName,
114114
value = t.toByteString
115115
)
116116

117+
def toProtoAny: com.google.protobuf.any.Any = any()
118+
119+
def toProtoErrorAny: com.google.protobuf.any.Any = any("")
120+
117121
def toProtoStruct: Struct = toValue(t.toPMessage).kind match {
118122
case Value.Kind.StructValue(struct) => struct
119123
case _ => throw new IllegalArgumentException("Expected a struct value")

core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ConnectAnyFormat.scala

Lines changed: 12 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -2,82 +2,26 @@ package org.ivovk.connect_rpc_scala.http.json
22

33
import com.google.protobuf.any.Any as PBAny
44
import org.json4s.JsonAST.{JObject, JString, JValue}
5-
import scalapb.json4s.Printer
5+
import scalapb.json4s.{AnyFormat, Printer}
66

7+
import java.util.Base64
78
import scala.language.existentials
89

910
object ConnectAnyFormat {
10-
// Messages that have special representation are parsed/serialized from a `value` field of the
11-
// any.
12-
private val SpecialValues: Set[scalapb.GeneratedMessageCompanion[_]] = (
13-
com.google.protobuf.struct.StructProto.messagesCompanions ++
14-
com.google.protobuf.wrappers.WrappersProto.messagesCompanions ++
15-
Seq(
16-
com.google.protobuf.any.Any,
17-
com.google.protobuf.duration.Duration,
18-
com.google.protobuf.timestamp.Timestamp,
19-
com.google.protobuf.field_mask.FieldMask
20-
)
21-
).toSet
22-
23-
val anyWriter: (Printer, PBAny) => JValue = { case (printer, any) =>
24-
// Find the companion so it can be used to JSON-serialize the message. Perhaps this can be circumvented by
25-
// including the original GeneratedMessage with the Any (at least in memory).
26-
val cmp = printer.typeRegistry
27-
.findType(any.typeUrl)
28-
.getOrElse(
29-
throw new IllegalStateException(
30-
s"Unknown type ${any.typeUrl} in Any. Add a TypeRegistry that supports this type to the Printer."
31-
)
32-
)
3311

34-
// Unpack the message...
35-
val message = any.unpack(cmp)
12+
private val base64enc = Base64.getEncoder.withoutPadding()
3613

37-
// ... and add the @type marker to the resulting JSON
38-
if (SpecialValues.contains(cmp))
14+
val anyWriter: (Printer, PBAny) => JValue = { (printer, any) =>
15+
// Error details format in Connect protocol serialized differently from the standard spec.
16+
// It can be distinguished by the typeURL without the "type.googleapis.com/" prefix.
17+
if (!any.typeUrl.contains("/")) {
3918
JObject(
40-
"@type" -> JString(any.typeUrl),
41-
"value" -> printer.toJson(message)
19+
"type" -> JString(any.typeUrl),
20+
"value" -> JString(base64enc.encodeToString(any.value.toByteArray))
4221
)
43-
else
44-
printer.toJson(message) match {
45-
case JObject(fields) =>
46-
JObject(("@type" -> JString(any.typeUrl)) +: fields)
47-
case value =>
48-
// Safety net, this shouldn't happen
49-
throw new IllegalStateException(
50-
s"Message of type ${any.typeUrl} emitted non-object JSON: $value"
51-
)
52-
}
22+
} else {
23+
AnyFormat.anyWriter(printer, any)
24+
}
5325
}
5426

55-
// val anyParser: (Parser, JValue) => PBAny = {
56-
// case (parser, obj @ JObject(fields)) =>
57-
// obj \ "@type" match {
58-
// case JString(typeUrl) =>
59-
// val cmp = parser.typeRegistry
60-
// .findType(typeUrl)
61-
// .getOrElse(
62-
// throw new JsonFormatException(
63-
// s"Unknown type ${typeUrl} in Any. Add a TypeRegistry that supports this type to the Parser."
64-
// )
65-
// )
66-
// val input = if (SpecialValues.contains(cmp)) obj \ "value" else obj
67-
// val message = parser.fromJson(input, true)(cmp)
68-
// PBAny(typeUrl = typeUrl, value = message.toByteString)
69-
//
70-
// case JNothing =>
71-
// throw new JsonFormatException(s"Missing type url when parsing $obj")
72-
//
73-
// case unknown =>
74-
// throw new JsonFormatException(
75-
// s"Expected string @type field, got $unknown"
76-
// )
77-
// }
78-
//
79-
// case (_, unknown) =>
80-
// throw new JsonFormatException(s"Expected an object, got $unknown")
81-
// }
8227
}
83-
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package org.ivovk.connect_rpc_scala.http.json
2+
3+
import scalapb.json4s.{AnyFormat, FormatRegistry, JsonFormat}
4+
5+
object ConnectJsonRegistry {
6+
7+
val default: FormatRegistry = JsonFormat.DefaultRegistry
8+
.registerMessageFormatter[com.google.protobuf.any.Any](
9+
ConnectAnyFormat.anyWriter,
10+
AnyFormat.anyParser
11+
)
12+
13+
}

0 commit comments

Comments
 (0)