From 74403b002200bbc472395b8be4cb31e4faf99744 Mon Sep 17 00:00:00 2001 From: Ihor Vovk Date: Fri, 6 Dec 2024 14:23:26 +0100 Subject: [PATCH] Introduce Connect error codes in the Error type; cache error mappings (#44) --- README.md | 3 +- .../connect_rpc_scala/conformance/Main.scala | 14 ++- core/src/main/protobuf/connectrpc/error.proto | 23 +++- .../ivovk/connect_rpc_scala/Mappings.scala | 103 +++++++++++------- .../http/codec/JsonMessageCodecBuilder.scala | 8 +- .../http/json/ConnectErrorFormat.scala | 57 ++++++++++ .../http/json/ErrorDetailsAnyFormat.scala | 2 +- .../http/json/JsonSerializationTest.scala | 19 +++- 8 files changed, 180 insertions(+), 49 deletions(-) create mode 100644 core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ConnectErrorFormat.scala diff --git a/README.md b/README.md index 668e0a7..7a387fc 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,7 @@ docker build . --output "out" --progress=plain Execution results are output to STDOUT. Diagnostic data from the server itself is written to the log file `out/out.log`. -### Conformance tests status +### Connect protocol conformance tests status Current status: __77/79__ tests pass. @@ -159,4 +159,5 @@ Known issues: ## Future improvements - [x] Support GET-requests +- [ ] Support `google.api.http` annotations (GRPC transcoding) - [ ] Support non-unary (streaming) methods diff --git a/conformance/src/main/scala/org/ivovk/connect_rpc_scala/conformance/Main.scala b/conformance/src/main/scala/org/ivovk/connect_rpc_scala/conformance/Main.scala index 29fbd71..202a9df 100644 --- a/conformance/src/main/scala/org/ivovk/connect_rpc_scala/conformance/Main.scala +++ b/conformance/src/main/scala/org/ivovk/connect_rpc_scala/conformance/Main.scala @@ -4,7 +4,9 @@ import cats.effect.{IO, IOApp, Sync} import com.comcast.ip4s.{Port, host, port} import connectrpc.conformance.v1.{ConformanceServiceFs2GrpcTrailers, ServerCompatRequest, ServerCompatResponse} import org.http4s.ember.server.EmberServerBuilder +import org.http4s.server.middleware.Logger import org.ivovk.connect_rpc_scala.ConnectRouteBuilder +import org.slf4j.LoggerFactory import java.io.InputStream import java.nio.ByteBuffer @@ -27,6 +29,8 @@ import java.nio.ByteBuffer */ object Main extends IOApp.Simple { + private val logger = LoggerFactory.getLogger(getClass) + override def run: IO[Unit] = { val res = for req <- ServerCompatSerDeser.readRequest[IO](System.in).toResource @@ -36,7 +40,7 @@ object Main extends IOApp.Simple { ) app <- ConnectRouteBuilder.forService[IO](service) - .withJsonCodecConfigurator { + .withJsonCodecConfigurator { // Registering message types in TypeRegistry is required to pass com.google.protobuf.any.Any // JSON-serialization conformance tests _ @@ -45,10 +49,16 @@ object Main extends IOApp.Simple { } .build + logger = Logger.httpApp[IO]( + logHeaders = false, + logBody = false, + logAction = Some(str => IO(this.logger.trace(str))) + )(app) + server <- EmberServerBuilder.default[IO] .withHost(host"127.0.0.1") .withPort(port"0") // random port - .withHttpApp(app) + .withHttpApp(logger) .build addr = server.address diff --git a/core/src/main/protobuf/connectrpc/error.proto b/core/src/main/protobuf/connectrpc/error.proto index 5d5e499..418f545 100644 --- a/core/src/main/protobuf/connectrpc/error.proto +++ b/core/src/main/protobuf/connectrpc/error.proto @@ -16,6 +16,27 @@ syntax = "proto3"; package connectrpc; +enum Code { + CODE_UNSPECIFIED = 0; + CODE_CANCELED = 1; + CODE_UNKNOWN = 2; + CODE_INVALID_ARGUMENT = 3; + CODE_DEADLINE_EXCEEDED = 4; + CODE_NOT_FOUND = 5; + CODE_ALREADY_EXISTS = 6; + CODE_PERMISSION_DENIED = 7; + CODE_RESOURCE_EXHAUSTED = 8; + CODE_FAILED_PRECONDITION = 9; + CODE_ABORTED = 10; + CODE_OUT_OF_RANGE = 11; + CODE_UNIMPLEMENTED = 12; + CODE_INTERNAL = 13; + CODE_UNAVAILABLE = 14; + CODE_DATA_LOSS = 15; + CODE_UNAUTHENTICATED = 16; +} + + // This message is similar to the google.protobuf.Any message. // // Separate type was needed to introduce a separate JSON serializer for this message, since Any in error details @@ -30,7 +51,7 @@ message ErrorDetailsAny { message Error { // The error code. // For a list of Connect error codes see: https://connectrpc.com/docs/protocol#error-codes - string code = 1; + Code code = 1; // If this value is absent in a test case response definition, the contents of the // actual error message will not be checked. This is useful for certain kinds of // error conditions where the exact message to be used is not specified, only the diff --git a/core/src/main/scala/org/ivovk/connect_rpc_scala/Mappings.scala b/core/src/main/scala/org/ivovk/connect_rpc_scala/Mappings.scala index e106968..f2a7b4a 100644 --- a/core/src/main/scala/org/ivovk/connect_rpc_scala/Mappings.scala +++ b/core/src/main/scala/org/ivovk/connect_rpc_scala/Mappings.scala @@ -52,53 +52,74 @@ trait HeaderMappings { trait StatusCodeMappings { + private val httpStatusCodesByGrpcStatusCode: Array[org.http4s.Status] = { + val maxCode = io.grpc.Status.Code.values().map(_.value()).max + val codes = new Array[org.http4s.Status](maxCode + 1) + + io.grpc.Status.Code.values().foreach { code => + codes(code.value()) = code match { + case io.grpc.Status.Code.CANCELLED => + org.http4s.Status.fromInt(499).getOrElse(sys.error("Should not happen")) + case io.grpc.Status.Code.UNKNOWN => org.http4s.Status.InternalServerError + case io.grpc.Status.Code.INVALID_ARGUMENT => org.http4s.Status.BadRequest + case io.grpc.Status.Code.DEADLINE_EXCEEDED => org.http4s.Status.GatewayTimeout + case io.grpc.Status.Code.NOT_FOUND => org.http4s.Status.NotFound + case io.grpc.Status.Code.ALREADY_EXISTS => org.http4s.Status.Conflict + case io.grpc.Status.Code.PERMISSION_DENIED => org.http4s.Status.Forbidden + case io.grpc.Status.Code.RESOURCE_EXHAUSTED => org.http4s.Status.TooManyRequests + case io.grpc.Status.Code.FAILED_PRECONDITION => org.http4s.Status.BadRequest + case io.grpc.Status.Code.ABORTED => org.http4s.Status.Conflict + case io.grpc.Status.Code.OUT_OF_RANGE => org.http4s.Status.BadRequest + case io.grpc.Status.Code.UNIMPLEMENTED => org.http4s.Status.NotImplemented + case io.grpc.Status.Code.INTERNAL => org.http4s.Status.InternalServerError + case io.grpc.Status.Code.UNAVAILABLE => org.http4s.Status.ServiceUnavailable + case io.grpc.Status.Code.DATA_LOSS => org.http4s.Status.InternalServerError + case io.grpc.Status.Code.UNAUTHENTICATED => org.http4s.Status.Unauthorized + case _ => org.http4s.Status.InternalServerError + } + } + + codes + } + + private val connectErrorCodeByGrpcStatusCode: Array[connectrpc.Code] = { + val maxCode = io.grpc.Status.Code.values().map(_.value()).max + val codes = new Array[connectrpc.Code](maxCode + 1) + + io.grpc.Status.Code.values().foreach { code => + codes(code.value()) = code match { + case io.grpc.Status.Code.CANCELLED => connectrpc.Code.Canceled + case io.grpc.Status.Code.UNKNOWN => connectrpc.Code.Unknown + case io.grpc.Status.Code.INVALID_ARGUMENT => connectrpc.Code.InvalidArgument + case io.grpc.Status.Code.DEADLINE_EXCEEDED => connectrpc.Code.DeadlineExceeded + case io.grpc.Status.Code.NOT_FOUND => connectrpc.Code.NotFound + case io.grpc.Status.Code.ALREADY_EXISTS => connectrpc.Code.AlreadyExists + case io.grpc.Status.Code.PERMISSION_DENIED => connectrpc.Code.PermissionDenied + case io.grpc.Status.Code.RESOURCE_EXHAUSTED => connectrpc.Code.ResourceExhausted + case io.grpc.Status.Code.FAILED_PRECONDITION => connectrpc.Code.FailedPrecondition + case io.grpc.Status.Code.ABORTED => connectrpc.Code.Aborted + case io.grpc.Status.Code.OUT_OF_RANGE => connectrpc.Code.OutOfRange + case io.grpc.Status.Code.UNIMPLEMENTED => connectrpc.Code.Unimplemented + case io.grpc.Status.Code.INTERNAL => connectrpc.Code.Internal + case io.grpc.Status.Code.UNAVAILABLE => connectrpc.Code.Unavailable + case io.grpc.Status.Code.DATA_LOSS => connectrpc.Code.DataLoss + case io.grpc.Status.Code.UNAUTHENTICATED => connectrpc.Code.Unauthenticated + case _ => connectrpc.Code.Internal + } + } + + codes + } + extension (status: io.grpc.Status) { def toHttpStatus: org.http4s.Status = status.getCode.toHttpStatus - def toConnectCode: String = status.getCode.toConnectCode + def toConnectCode: connectrpc.Code = status.getCode.toConnectCode } // Url: https://connectrpc.com/docs/protocol/#error-codes extension (code: io.grpc.Status.Code) { - def toHttpStatus: org.http4s.Status = code match { - case io.grpc.Status.Code.CANCELLED => - org.http4s.Status.fromInt(499).getOrElse(org.http4s.Status.InternalServerError) - case io.grpc.Status.Code.UNKNOWN => org.http4s.Status.InternalServerError - case io.grpc.Status.Code.INVALID_ARGUMENT => org.http4s.Status.BadRequest - case io.grpc.Status.Code.DEADLINE_EXCEEDED => org.http4s.Status.GatewayTimeout - case io.grpc.Status.Code.NOT_FOUND => org.http4s.Status.NotFound - case io.grpc.Status.Code.ALREADY_EXISTS => org.http4s.Status.Conflict - case io.grpc.Status.Code.PERMISSION_DENIED => org.http4s.Status.Forbidden - case io.grpc.Status.Code.RESOURCE_EXHAUSTED => org.http4s.Status.TooManyRequests - case io.grpc.Status.Code.FAILED_PRECONDITION => org.http4s.Status.BadRequest - case io.grpc.Status.Code.ABORTED => org.http4s.Status.Conflict - case io.grpc.Status.Code.OUT_OF_RANGE => org.http4s.Status.BadRequest - case io.grpc.Status.Code.UNIMPLEMENTED => org.http4s.Status.NotImplemented - case io.grpc.Status.Code.INTERNAL => org.http4s.Status.InternalServerError - case io.grpc.Status.Code.UNAVAILABLE => org.http4s.Status.ServiceUnavailable - case io.grpc.Status.Code.DATA_LOSS => org.http4s.Status.InternalServerError - case io.grpc.Status.Code.UNAUTHENTICATED => org.http4s.Status.Unauthorized - case _ => org.http4s.Status.InternalServerError - } - - def toConnectCode: String = code match { - case io.grpc.Status.Code.CANCELLED => "canceled" - case io.grpc.Status.Code.UNKNOWN => "unknown" - case io.grpc.Status.Code.INVALID_ARGUMENT => "invalid_argument" - case io.grpc.Status.Code.DEADLINE_EXCEEDED => "deadline_exceeded" - case io.grpc.Status.Code.NOT_FOUND => "not_found" - case io.grpc.Status.Code.ALREADY_EXISTS => "already_exists" - case io.grpc.Status.Code.PERMISSION_DENIED => "permission_denied" - case io.grpc.Status.Code.RESOURCE_EXHAUSTED => "resource_exhausted" - case io.grpc.Status.Code.FAILED_PRECONDITION => "failed_precondition" - case io.grpc.Status.Code.ABORTED => "aborted" - case io.grpc.Status.Code.OUT_OF_RANGE => "out_of_range" - case io.grpc.Status.Code.UNIMPLEMENTED => "unimplemented" - case io.grpc.Status.Code.INTERNAL => "internal" - case io.grpc.Status.Code.UNAVAILABLE => "unavailable" - case io.grpc.Status.Code.DATA_LOSS => "data_loss" - case io.grpc.Status.Code.UNAUTHENTICATED => "unauthenticated" - case _ => "internal" - } + def toHttpStatus: org.http4s.Status = httpStatusCodesByGrpcStatusCode(code.value()) + def toConnectCode: connectrpc.Code = connectErrorCodeByGrpcStatusCode(code.value()) } } diff --git a/core/src/main/scala/org/ivovk/connect_rpc_scala/http/codec/JsonMessageCodecBuilder.scala b/core/src/main/scala/org/ivovk/connect_rpc_scala/http/codec/JsonMessageCodecBuilder.scala index cfb91e5..b5818b6 100644 --- a/core/src/main/scala/org/ivovk/connect_rpc_scala/http/codec/JsonMessageCodecBuilder.scala +++ b/core/src/main/scala/org/ivovk/connect_rpc_scala/http/codec/JsonMessageCodecBuilder.scala @@ -1,7 +1,7 @@ package org.ivovk.connect_rpc_scala.http.codec import cats.effect.Sync -import org.ivovk.connect_rpc_scala.http.json.ErrorDetailsAnyFormat +import org.ivovk.connect_rpc_scala.http.json.{ConnectErrorFormat, ErrorDetailsAnyFormat} import scalapb.json4s.{FormatRegistry, JsonFormat, TypeRegistry} import scalapb.{GeneratedMessage, GeneratedMessageCompanion, json4s} @@ -29,7 +29,11 @@ case class JsonMessageCodecBuilder[F[_] : Sync] private( val formatRegistry = this.formatRegistry .registerMessageFormatter[connectrpc.ErrorDetailsAny]( ErrorDetailsAnyFormat.writer, - ErrorDetailsAnyFormat.printer + ErrorDetailsAnyFormat.parser + ) + .registerMessageFormatter[connectrpc.Error]( + ConnectErrorFormat.writer, + ConnectErrorFormat.parser ) val parser = new json4s.Parser() diff --git a/core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ConnectErrorFormat.scala b/core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ConnectErrorFormat.scala new file mode 100644 index 0000000..5e3b272 --- /dev/null +++ b/core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ConnectErrorFormat.scala @@ -0,0 +1,57 @@ +package org.ivovk.connect_rpc_scala.http.json + +import connectrpc.{Error, ErrorDetailsAny} +import org.json4s.JsonAST.{JArray, JString, JValue} +import org.json4s.MonadicJValue.* +import org.json4s.{JNothing, JObject} +import scalapb.json4s.{Parser, Printer} + +object ConnectErrorFormat { + + private val stringErrorCodes: Array[JString] = { + val maxCode = connectrpc.Code.values.map(_.value).max + val codes = new Array[JString](maxCode + 1) + + connectrpc.Code.values.foreach { code => + codes(code.value) = JString(code.name.substring("CODE_".length).toLowerCase) + } + + codes + } + + val writer: (Printer, Error) => JValue = { (printer, error) => + JObject(List.concat( + Some("code" -> stringErrorCodes(error.code.value)), + error.message.map("message" -> JString(_)), + Option(error.details).filterNot(_.isEmpty).map(d => "details" -> JArray(d.map(printer.toJson).toList)), + )) + } + + val parser: (Parser, JValue) => Error = { + case (parser, obj@JObject(fields)) => + val code = obj \ "code" match + case JString(code) => + connectrpc.Code.fromName(s"CODE_${code.toUpperCase}") + .getOrElse(throw new IllegalArgumentException(s"Unknown error code: $code")) + case _ => throw new IllegalArgumentException(s"Error parsing Error: $obj") + + val message = obj \ "message" match + case JString(message) => Some(message) + case JNothing => None + case _ => throw new IllegalArgumentException(s"Error parsing Error: $obj") + + val details = obj \ "details" match + case JArray(details) => details.map(parser.fromJson[ErrorDetailsAny]) + case JNothing => Seq.empty + case _ => throw new IllegalArgumentException(s"Error parsing Error: $obj") + + Error( + code = code, + message = message, + details = details, + ) + case (_, other) => + throw new IllegalArgumentException(s"Expected an object, got $other") + } + +} diff --git a/core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ErrorDetailsAnyFormat.scala b/core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ErrorDetailsAnyFormat.scala index 8fccf70..17f3195 100644 --- a/core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ErrorDetailsAnyFormat.scala +++ b/core/src/main/scala/org/ivovk/connect_rpc_scala/http/json/ErrorDetailsAnyFormat.scala @@ -22,7 +22,7 @@ object ErrorDetailsAnyFormat { ) } - val printer: (Parser, JValue) => ErrorDetailsAny = { + val parser: (Parser, JValue) => ErrorDetailsAny = { case (parser, obj@JObject(fields)) => (obj \ "type", obj \ "value") match { case (JString(t), JString(v)) => diff --git a/core/src/test/scala/org/ivovk/connect_rpc_scala/http/json/JsonSerializationTest.scala b/core/src/test/scala/org/ivovk/connect_rpc_scala/http/json/JsonSerializationTest.scala index fd9030d..31f03df 100644 --- a/core/src/test/scala/org/ivovk/connect_rpc_scala/http/json/JsonSerializationTest.scala +++ b/core/src/test/scala/org/ivovk/connect_rpc_scala/http/json/JsonSerializationTest.scala @@ -10,7 +10,7 @@ class JsonSerializationTest extends AnyFunSuite { val formatRegistry = json4s.JsonFormat.DefaultRegistry .registerMessageFormatter[connectrpc.ErrorDetailsAny]( ErrorDetailsAnyFormat.writer, - ErrorDetailsAnyFormat.printer + ErrorDetailsAnyFormat.parser ) val parser = new json4s.Parser().withFormatRegistry(formatRegistry) @@ -22,4 +22,21 @@ class JsonSerializationTest extends AnyFunSuite { assert(parsed == any) } + + test("Error serialization") { + val formatRegistry = json4s.JsonFormat.DefaultRegistry + .registerMessageFormatter[connectrpc.Error]( + ConnectErrorFormat.writer, + ConnectErrorFormat.parser + ) + + val parser = new json4s.Parser().withFormatRegistry(formatRegistry) + val printer = new json4s.Printer().withFormatRegistry(formatRegistry) + + val error = connectrpc.Error(connectrpc.Code.FailedPrecondition, Some("message"), Seq.empty) + val json = printer.print(error) + val parsed = parser.fromJsonString[connectrpc.Error](json) + + assert(parsed == error) + } }