From c0acb7cfe914d6f3a224e1f03e4facea0fe97568 Mon Sep 17 00:00:00 2001 From: Joshua Humphries <2035234+jhump@users.noreply.github.com> Date: Fri, 5 Apr 2024 17:20:06 -0400 Subject: [PATCH] Update to conformance suite v1.0.1 (#253) This includes a few minor fixes, too: 1. For Connect unary RPCs, the error metadata included only headers, but should include both headers and trailers. 2. For gRPC and gRPC-Web protocols, when a "trailers-only" response is received (where there are only HTTP headers and no body or other trailers, and the headers are to be interpreted as trailers), report those headers as trailers. --- Makefile | 5 ++--- .../kotlin/com/connectrpc/conformance/client/Client.kt | 7 +------ .../com/connectrpc/protocols/ConnectInterceptor.kt | 6 +++--- .../kotlin/com/connectrpc/protocols/GRPCCompletion.kt | 3 +++ .../com/connectrpc/protocols/GRPCCompletionParser.kt | 4 ++++ .../kotlin/com/connectrpc/protocols/GRPCInterceptor.kt | 10 +++++++--- .../com/connectrpc/protocols/GRPCWebInterceptor.kt | 1 + 7 files changed, 21 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index f5fba49e..1ac4a108 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ BIN := .tmp/bin CACHE := .tmp/cache LICENSE_HEADER_YEAR_RANGE := 2022-2023 LICENSE_HEADER_VERSION := v1.30.0 -CONFORMANCE_VERSION := v1.0.0-rc4 +CONFORMANCE_VERSION := v1.0.1 PROTOC_VERSION ?= 25.3 GRADLE_ARGS ?= PROTOC := $(BIN)/protoc @@ -39,9 +39,8 @@ buildplugin: ## Build the connect-kotlin protoc plugin. clean: ## Cleans the underlying build. ./gradlew $(GRADLE_ARGS) clean -# TODO: remove the cross-tests and rely solely on new conformance suite .PHONY: runconformance -runconformance: generate $(CONNECT_CONFORMANCE) ## Run the new conformance test suite. +runconformance: generate $(CONNECT_CONFORMANCE) ./gradlew $(GRADLE_ARGS) conformance:client:google-java:installDist conformance:client:google-javalite:installDist $(CONNECT_CONFORMANCE) $(CONNECT_CONFORMANCE_ARGS) --conf conformance/client/lite-unary-config.yaml \ --known-failing @conformance/client/known-failing-unary-cases.txt -- \ diff --git a/conformance/client/src/main/kotlin/com/connectrpc/conformance/client/Client.kt b/conformance/client/src/main/kotlin/com/connectrpc/conformance/client/Client.kt index 7e4bb746..af6ff03e 100644 --- a/conformance/client/src/main/kotlin/com/connectrpc/conformance/client/Client.kt +++ b/conformance/client/src/main/kotlin/com/connectrpc/conformance/client/Client.kt @@ -385,15 +385,10 @@ class Client( ) } is ResponseMessage.Failure -> { - // TODO: report result.headers and result.trailers independently - // once reference server can report send them independently. - // https://github.com/connectrpc/conformance/pull/840. - // Until then, always report trailers as exception metadata (which - // may include headers). ClientResponseResult( headers = result.headers, error = result.cause, - trailers = result.cause.metadata, + trailers = result.trailers, numUnsentRequests = numUnsent, ) } diff --git a/library/src/main/kotlin/com/connectrpc/protocols/ConnectInterceptor.kt b/library/src/main/kotlin/com/connectrpc/protocols/ConnectInterceptor.kt index 596aff67..8f6af925 100644 --- a/library/src/main/kotlin/com/connectrpc/protocols/ConnectInterceptor.kt +++ b/library/src/main/kotlin/com/connectrpc/protocols/ConnectInterceptor.kt @@ -115,7 +115,7 @@ internal class ConnectInterceptor( val exception: ConnectException? val message: Buffer if (response.status != 200) { - exception = parseConnectUnaryException(response.status, response.headers, responseBody) + exception = parseConnectUnaryException(response.status, responseHeaders.plus(trailers), responseBody) // We've already read the response body to parse an error - don't read again. message = Buffer() } else { @@ -244,7 +244,7 @@ internal class ConnectInterceptor( } } - private fun parseConnectUnaryException(httpStatus: Int?, headers: Headers, source: Buffer?): ConnectException { + private fun parseConnectUnaryException(httpStatus: Int?, metadata: Headers, source: Buffer?): ConnectException { val code = Code.fromHTTPStatus(httpStatus) if (source == null) { return ConnectException(code, "unexpected status code: $httpStatus") @@ -261,7 +261,7 @@ internal class ConnectInterceptor( ConnectException( code = Code.fromName(errorPayloadJSON.code, code), message = errorPayloadJSON.message, - metadata = headers, + metadata = metadata, ).withErrorDetails( serializationStrategy.errorDetailParser(), errorDetails, diff --git a/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletion.kt b/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletion.kt index d0632b3c..e94f5235 100644 --- a/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletion.kt +++ b/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletion.kt @@ -33,6 +33,9 @@ internal data class GRPCCompletion( val errorDetails: List = emptyList(), // Set to either message headers (or trailers) where the gRPC status was found. val metadata: Headers, + // If true, this status was parsed from headers, in a "trailers-only" response. + // Otherwise, the status was parsed from trailers. + val trailersOnly: Boolean = false, // If false, this completion was synthesized and not actually present in metadata. val present: Boolean = true, ) { diff --git a/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletionParser.kt b/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletionParser.kt index efe23a19..b86e09d8 100644 --- a/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletionParser.kt +++ b/library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletionParser.kt @@ -37,6 +37,7 @@ internal class GRPCCompletionParser( val statusCode: Int val statusMetadata: Map> val statusFromHeaders = parseStatus(headers) + val trailersOnly: Boolean if (statusFromHeaders == null) { statusCode = parseStatus(trailers) ?: return GRPCCompletion( @@ -46,9 +47,11 @@ internal class GRPCCompletionParser( metadata = trailers, ) statusMetadata = trailers + trailersOnly = false } else { statusCode = statusFromHeaders statusMetadata = headers + trailersOnly = true } // Note: we report combined headers and trailers as exception meta, so // caller doesn't have to check both, which is particularly important @@ -60,6 +63,7 @@ internal class GRPCCompletionParser( message = parseMessage(statusMetadata).utf8(), errorDetails = connectErrorDetails(statusMetadata), metadata = exceptionMeta, + trailersOnly = trailersOnly, ) } diff --git a/library/src/main/kotlin/com/connectrpc/protocols/GRPCInterceptor.kt b/library/src/main/kotlin/com/connectrpc/protocols/GRPCInterceptor.kt index 23e1341a..7d69f0b3 100644 --- a/library/src/main/kotlin/com/connectrpc/protocols/GRPCInterceptor.kt +++ b/library/src/main/kotlin/com/connectrpc/protocols/GRPCInterceptor.kt @@ -76,10 +76,13 @@ internal class GRPCInterceptor( ) } val headers = response.headers - val trailers = response.trailers - val exception = completionParser + var trailers = response.trailers + val completion = completionParser .parse(headers, trailers) - .toConnectExceptionOrNull(serializationStrategy) + if (completion.trailersOnly) { + trailers = headers // report the headers also as trailers + } + val exception = completion.toConnectExceptionOrNull(serializationStrategy) val message = if (exception == null) { if (response.message.buffer.exhausted()) { return@UnaryFunction response.clone( @@ -112,6 +115,7 @@ internal class GRPCInterceptor( response.clone( message = message, cause = exception, + trailers = trailers, ) }, ) diff --git a/library/src/main/kotlin/com/connectrpc/protocols/GRPCWebInterceptor.kt b/library/src/main/kotlin/com/connectrpc/protocols/GRPCWebInterceptor.kt index d332c964..72115d91 100644 --- a/library/src/main/kotlin/com/connectrpc/protocols/GRPCWebInterceptor.kt +++ b/library/src/main/kotlin/com/connectrpc/protocols/GRPCWebInterceptor.kt @@ -105,6 +105,7 @@ internal class GRPCWebInterceptor( response.clone( message = Buffer(), cause = exception, + trailers = headers, ) } else { // Unpack the current message and trailers.