update to rc4 of conformance suite and fix several issues

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-rc3
+CONFORMANCE_VERSION := v1.0.0-rc4
 PROTOC_VERSION ?= 26.0
 GRADLE_ARGS ?=
 PROTOC := $(BIN)/protoc
@@ -43,34 +43,34 @@ clean: ## Cleans the underlying build.
 runconformance: generate $(CONNECT_CONFORMANCE) ## Run the new conformance test suite.
 	./gradlew $(GRADLE_ARGS) conformance:client:google-java:installDist conformance:client:google-javalite:installDist
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/lite-unary-config.yaml \
-		--known-failing @conformance/client/known-failing-unary-cases.txt -- \
+		--known-failing @conformance/client/known-failing-unary-cases.txt --trace -- \
 		conformance/client/google-javalite/build/install/google-javalite/bin/google-javalite \
 		--style suspend
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/lite-unary-config.yaml \
-		--known-failing @conformance/client/known-failing-unary-cases.txt -- \
+		--known-failing @conformance/client/known-failing-unary-cases.txt --trace -- \
 		conformance/client/google-javalite/build/install/google-javalite/bin/google-javalite \
 		--style callback
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/lite-unary-config.yaml \
-		--known-failing @conformance/client/known-failing-unary-cases.txt -- \
+		--known-failing @conformance/client/known-failing-unary-cases.txt --trace -- \
 		conformance/client/google-javalite/build/install/google-javalite/bin/google-javalite \
 		--style blocking
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/standard-unary-config.yaml \
-		--known-failing @conformance/client/known-failing-unary-cases.txt -- \
+		--known-failing @conformance/client/known-failing-unary-cases.txt --trace -- \
 		conformance/client/google-java/build/install/google-java/bin/google-java \
 		--style suspend
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/standard-unary-config.yaml \
-		--known-failing @conformance/client/known-failing-unary-cases.txt -- \
+		--known-failing @conformance/client/known-failing-unary-cases.txt --trace -- \
 		conformance/client/google-java/build/install/google-java/bin/google-java \
 		--style callback
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/standard-unary-config.yaml \
-		--known-failing @conformance/client/known-failing-unary-cases.txt -- \
+		--known-failing @conformance/client/known-failing-unary-cases.txt --trace -- \
 		conformance/client/google-java/build/install/google-java/bin/google-java \
 		--style blocking
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/lite-stream-config.yaml \
-		--known-failing @conformance/client/known-failing-stream-cases.txt -- \
+		--known-failing @conformance/client/known-failing-stream-cases.txt --trace -- \
 		conformance/client/google-javalite/build/install/google-javalite/bin/google-javalite
 	$(CONNECT_CONFORMANCE) -v --mode client --conf conformance/client/standard-stream-config.yaml \
-		--known-failing @conformance/client/known-failing-stream-cases.txt -- \
+		--known-failing @conformance/client/known-failing-stream-cases.txt --trace -- \
 		conformance/client/google-java/build/install/google-java/bin/google-java
 
 ifeq ($(UNAME_OS),Darwin)

conformance/client/google-java/src/main/kotlin/com/connectrpc/conformance/client/java/JavaHelpers.kt

@@ -251,7 +251,7 @@ class JavaHelpers {
     }
 
     private class TlsCredsImpl(
-        private val msg: com.connectrpc.conformance.v1.ClientCompatRequest.TLSCreds,
+        private val msg: com.connectrpc.conformance.v1.TLSCreds,
     ) : TlsCreds {
         override val cert: ByteString
             get() = msg.cert

conformance/client/google-javalite/src/main/kotlin/com/connectrpc/conformance/client/javalite/JavaLiteHelpers.kt

@@ -233,7 +233,7 @@ class JavaLiteHelpers {
     }
 
     private class TlsCredsImpl(
-        private val msg: com.connectrpc.conformance.v1.ClientCompatRequest.TLSCreds,
+        private val msg: com.connectrpc.conformance.v1.TLSCreds,
     ) : TlsCreds {
         override val cert: ByteString
             get() = msg.cert

conformance/client/known-failing-unary-cases.txt

@@ -1,32 +1,13 @@
-# If error body is JSON null, it is interpreted as an unknown error
-# instead of falling back to basing code on the HTTP status.
-Connect Error and End-Stream/**/error/null
-
 # Deadline headers are not currently set.
 Deadline Propagation/**
 
-# This response doesn't look like a normal Connect response, so it
-# goes through okhttp's default 408 code handling, which retries.
-# The retry triggers an error:
-#     client sent another request (#2) for the same test case
-HTTP to Connect Code Mapping/**/request-timeout
-
 # Bug: response content-type is not correctly checked
-Unexpected Responses/**/unexpected-content-type
+**/unexpected-content-type
 
 # Bug: "trailers-only" responses are not correctly identified.
 # If headers contain "grpc-status", this client assumes it is a
 # trailers-only response. However, a trailers-only response should
 # instead be identified by lack of body or HTTP trailers.
 gRPC Unexpected Responses/**/trailers-only/*
+gRPC-Web Unexpected Responses/**/trailers-only/ignore-header-if-body-present
 
-# Bug: if gRPC unary response contains zero messages or
-# more than one message, client does not complain if
-# status trailers says "ok"
-gRPC Unexpected Responses/**/unary-multiple-responses
-gRPC Unexpected Responses/**/unary-ok-but-no-response
-gRPC-Web Unexpected Responses/**/unary-ok-but-no-response
-
-# Bug: incorrect code attribution for these failures (INTERNAL instead of UNKNOWN)
-gRPC-Web Unexpected Responses/**/missing-status
-gRPC-Web Unexpected Responses/**/trailers-in-body/unary-multiple-responses

conformance/client/src/main/kotlin/com/connectrpc/conformance/client/Client.kt

@@ -385,10 +385,15 @@ 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.trailers,
+                    trailers = result.cause.metadata,
                     numUnsentRequests = numUnsent,
                 )
             }

conformance/client/src/main/kotlin/com/connectrpc/conformance/client/adapt/ClientStreamClient.kt

@@ -103,7 +103,7 @@ abstract class ClientStreamClient<Req : MessageLite, Resp : MessageLite>(
                             return ResponseMessage.Failure(
                                 cause = connEx,
                                 headers = underlying.responseHeaders().await(),
-                                trailers = connEx.metadata,
+                                trailers = underlying.responseTrailers().await(),
                             )
                         }
                     }

library/src/main/kotlin/com/connectrpc/Code.kt

@@ -47,32 +47,24 @@ enum class Code(val codeName: String, val value: Int) {
         fun fromHTTPStatus(status: Int?): Code {
             return when (status) {
                 null -> UNKNOWN
-                400 -> INVALID_ARGUMENT
+                400 -> INTERNAL_ERROR
                 401 -> UNAUTHENTICATED
                 403 -> PERMISSION_DENIED
                 404 -> UNIMPLEMENTED
-                408 -> DEADLINE_EXCEEDED
-                409 -> ABORTED
-                412 -> FAILED_PRECONDITION
-                413 -> RESOURCE_EXHAUSTED
-                415 -> INTERNAL_ERROR
-                429 -> UNAVAILABLE
-                431 -> RESOURCE_EXHAUSTED
-                499 -> CANCELED
-                502, 503, 504 -> UNAVAILABLE
+                429, 502, 503, 504 -> UNAVAILABLE
                 else -> UNKNOWN
             }
         }
-        fun fromName(name: String?): Code {
+        fun fromName(name: String?, ifNotKnown: Code = UNKNOWN): Code {
             if (name == null) {
-                return UNKNOWN
+                return ifNotKnown
             }
             for (value in values()) {
                 if (value.codeName == name) {
                     return value
                 }
             }
-            return UNKNOWN
+            return ifNotKnown
         }
         fun fromValue(value: Int?): Code? {
             if (value == null) {

library/src/main/kotlin/com/connectrpc/ConnectErrorDetail.kt

@@ -16,14 +16,16 @@ package com.connectrpc
 
 import okio.ByteString
 
-// An ErrorDetail is a self-describing Protobuf message attached to an [*Error].
+// A ConnectErrorDetail is a self-describing Protobuf message attached to a
+// [ConnectException].
+//
 // Error details are sent over the network to clients, which can then work with
 // strongly-typed data rather than trying to parse a complex error message. For
 // example, you might use details to send a localized error message or retry
 // parameters to the client.
 //
-// The [google.golang.org/genproto/googleapis/rpc/errdetails] package contains a
-// variety of Protobuf messages commonly used as error details.
+// The [com.google.rpc](https://googleapis.github.io/googleapis/java/all/latest/apidocs/com/google/rpc/package-summary.html)
+// package contains a variety of Protobuf messages commonly used as error details.
 data class ConnectErrorDetail(
     val type: String,
     val payload: ByteString,

library/src/main/kotlin/com/connectrpc/ConnectException.kt

@@ -20,19 +20,30 @@ import kotlin.reflect.KClass
  * Typed error provided by Connect RPCs that may optionally wrap additional typed custom errors
  * using [details].
  */
-data class ConnectException(
+class ConnectException private constructor(
     // The resulting status code.
     val code: Code,
-    private val errorDetailParser: ErrorDetailParser? = null,
     // User-readable error message.
-    override val message: String? = null,
+    override val message: String?,
     // Client-side exception that occurred, resulting in the error.
-    val exception: Throwable? = null,
-    // List of typed errors that were provided by the server.
-    val details: List<ConnectErrorDetail> = emptyList(),
+    val exception: Throwable?,
     // Additional key-values that were provided by the server.
-    val metadata: Headers = emptyMap(),
+    val metadata: Headers,
+    // Optional parser for messages in details. Will be non-null if
+    // details is non-empty.
+    private val errorDetailParser: ErrorDetailParser?,
+    // List of typed errors that were provided by the server.
+    val details: List<ConnectErrorDetail>,
 ) : Exception(message, exception) {
+    /**
+     * Constructs a new ConnectException.
+     */
+    constructor (
+        code: Code,
+        message: String? = null,
+        exception: Throwable? = null,
+        metadata: Headers = emptyMap(),
+    ) : this(code, message, exception, metadata, null, emptyList())
 
     /**
      * Unpacks values from [details] and returns the first matching error, if any.
@@ -51,16 +62,31 @@ data class ConnectException(
     }
 
     /**
-     * Creates a new [ConnectException] with the specified [ErrorDetailParser].
+     * Creates a new [ConnectException] with the specified error details and
+     * accompanying (non-null) detail parser.
      */
-    fun setErrorParser(errorParser: ErrorDetailParser): ConnectException {
+    fun withErrorDetails(errorParser: ErrorDetailParser, details: List<ConnectErrorDetail>): ConnectException {
         return ConnectException(
             code,
-            errorParser,
             message,
             exception,
+            metadata,
+            errorParser,
             details,
+        )
+    }
+
+    /**
+     * Creates a new [ConnectException] with the specified metadata.
+     */
+    fun withMetadata(metadata: Headers): ConnectException {
+        return ConnectException(
+            code,
+            message,
+            exception,
             metadata,
+            errorDetailParser,
+            details,
         )
     }
 }

library/src/main/kotlin/com/connectrpc/impl/ClientOnlyStream.kt

@@ -19,7 +19,9 @@ import com.connectrpc.ClientOnlyStreamInterface
 import com.connectrpc.Code
 import com.connectrpc.ConnectException
 import com.connectrpc.Headers
+import com.connectrpc.asConnectException
 import kotlinx.coroutines.Deferred
+import kotlinx.coroutines.channels.ClosedReceiveChannelException
 
 /**
  * Concrete implementation of [ClientOnlyStreamInterface].
@@ -35,12 +37,23 @@ internal class ClientOnlyStream<Input, Output>(
         val resultChannel = messageStream.responseChannel()
         try {
             messageStream.sendClose()
-            val message = resultChannel.receive()
+            val message = resultChannel.receiveCatching()
+            if (message.isFailure) {
+                val ex = message.exceptionOrNull()
+                if (ex == null || ex is ClosedReceiveChannelException) {
+                    throw ConnectException(
+                        code = Code.UNIMPLEMENTED,
+                        message = "unary stream has no messages",
+                        exception = ex,
+                    )
+                }
+                throw asConnectException(ex)
+            }
             val additionalMessage = resultChannel.receiveCatching()
             if (additionalMessage.isSuccess) {
-                throw ConnectException(code = Code.UNKNOWN, message = "unary stream has multiple messages")
+                throw ConnectException(code = Code.UNIMPLEMENTED, message = "unary stream has multiple messages")
             }
-            return message
+            return message.getOrThrow()
         } finally {
             resultChannel.cancel()
         }

library/src/main/kotlin/com/connectrpc/impl/ProtocolClient.kt

@@ -109,11 +109,10 @@ class ProtocolClient(
                     )
                     return@httpClientUnary
                 }
-                val exception = finalResponse.cause?.setErrorParser(serializationStrategy.errorDetailParser())
-                if (exception != null) {
+                if (finalResponse.cause != null) {
                     onResult(
                         ResponseMessage.Failure(
-                            exception,
+                            finalResponse.cause,
                             finalResponse.headers,
                             finalResponse.trailers,
                         ),

library/src/main/kotlin/com/connectrpc/protocols/ConnectInterceptor.kt

@@ -106,7 +106,6 @@ internal class ConnectInterceptor(
                         trailers = trailers,
                         cause = ConnectException(
                             code = Code.INTERNAL_ERROR,
-                            errorDetailParser = serializationStrategy.errorDetailParser(),
                             message = e.message,
                             exception = e,
                         ),
@@ -234,10 +233,11 @@ internal class ConnectInterceptor(
             StreamResult.Complete(
                 cause = ConnectException(
                     code = code,
-                    errorDetailParser = serializationStrategy.errorDetailParser(),
                     message = endStreamResponseJSON.error.message,
-                    details = parseErrorDetails(endStreamResponseJSON.error),
                     metadata = metadata.orEmpty(),
+                ).withErrorDetails(
+                    serializationStrategy.errorDetailParser(),
+                    parseErrorDetails(endStreamResponseJSON.error),
                 ),
             )
         }
@@ -246,23 +246,24 @@ internal class ConnectInterceptor(
     private fun parseConnectUnaryException(httpStatus: Int?, headers: Headers, source: Buffer?): ConnectException {
         val code = Code.fromHTTPStatus(httpStatus)
         if (source == null) {
-            return ConnectException(code, serializationStrategy.errorDetailParser(), "unexpected status code: $httpStatus")
+            return ConnectException(code, "unexpected status code: $httpStatus")
         }
         return source.use { bufferedSource ->
             val adapter = moshi.adapter(ErrorPayloadJSON::class.java).nonNull()
             val errorJSON = bufferedSource.readUtf8()
             val errorPayloadJSON = try {
                 adapter.fromJson(errorJSON)
             } catch (e: Exception) {
-                return ConnectException(code, serializationStrategy.errorDetailParser(), errorJSON, e)
+                return ConnectException(code, errorJSON, e)
             }
             val errorDetails = parseErrorDetails(errorPayloadJSON!!)
             ConnectException(
-                code = Code.fromName(errorPayloadJSON.code),
-                errorDetailParser = serializationStrategy.errorDetailParser(),
+                code = Code.fromName(errorPayloadJSON.code, code),
                 message = errorPayloadJSON.message,
-                details = errorDetails,
                 metadata = headers,
+            ).withErrorDetails(
+                serializationStrategy.errorDetailParser(),
+                errorDetails,
             )
         }
     }

library/src/main/kotlin/com/connectrpc/protocols/Envelope.kt

@@ -67,11 +67,9 @@ class Envelope {
             }
             val headerByte = source.readByte().toInt()
             val length = source.readInt().toLong()
-            val message = if (source.size > length) {
+            val message = if (source.size >= length) {
                 // extract relevant subset for this message
                 Buffer().write(source as Source, length)
-            } else if (source.size == length) {
-                source
             } else {
                 throw ConnectException(
                     code = Code.INTERNAL_ERROR,

library/src/main/kotlin/com/connectrpc/protocols/GRPCCompletion.kt

@@ -46,10 +46,11 @@ internal data class GRPCCompletion(
         } else {
             ConnectException(
                 code = code,
-                errorDetailParser = serializationStrategy.errorDetailParser(),
                 message = message,
-                details = errorDetails,
                 metadata = metadata,
+            ).withErrorDetails(
+                serializationStrategy.errorDetailParser(),
+                errorDetails,
             )
         }
     }