review feedback

connectrpc · Dec 7, 2023 · 371f1c0 · 371f1c0
1 parent 756c3b0
commit 371f1c0
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 9 deletions.
diff --git a/conformance/google-java/src/test/kotlin/com/connectrpc/conformance/java/ConformanceTest.kt b/conformance/google-java/src/test/kotlin/com/connectrpc/conformance/java/ConformanceTest.kt
@@ -56,7 +56,7 @@ class ConformanceTest(
 ) : BaseConformanceTest(protocol, serverType) {
     companion object {
         private val responseHeaders = mapOf(Pair("x-grpc-test-echo-initial", listOf("test_initial_metadata_value")))
-        private val responseTrailers = mapOf(Pair("x-grpc-test-echo-trailing-bin", listOf("CgsKCwoL")), /* base64-encoded 0x0a0b0a0b0a0b */)
+        private val responseTrailers = mapOf(Pair("x-grpc-test-echo-trailing-bin", listOf("CgsKCwoL"))) // base64-encoded 0x0a0b0a0b0a0b
         private val requestHeaders = responseHeaders + responseTrailers
     }
 

diff --git a/...ce/google-javalite/src/test/kotlin/com/connectrpc/conformance/javalite/ConformanceTest.kt b/...ce/google-javalite/src/test/kotlin/com/connectrpc/conformance/javalite/ConformanceTest.kt
@@ -56,7 +56,7 @@ class ConformanceTest(
 ) : BaseConformanceTest(protocol, serverType) {
     companion object {
         private val responseHeaders = mapOf(Pair("x-grpc-test-echo-initial", listOf("test_initial_metadata_value")))
-        private val responseTrailers = mapOf(Pair("x-grpc-test-echo-trailing-bin", listOf("CgsKCwoL")), /* base64-encoded 0x0a0b0a0b0a0b */)
+        private val responseTrailers = mapOf(Pair("x-grpc-test-echo-trailing-bin", listOf("CgsKCwoL"))) // base64-encoded 0x0a0b0a0b0a0b
         private val requestHeaders = responseHeaders + responseTrailers
     }
 

diff --git a/library/src/main/kotlin/com/connectrpc/http/HTTPClientInterface.kt b/library/src/main/kotlin/com/connectrpc/http/HTTPClientInterface.kt
@@ -75,9 +75,17 @@ class Stream(
 
     fun receiveClose() {
         if (!isReceiveClosed.getAndSet(true)) {
-            onReceiveClose()
-            // closing receive side implicitly closes send side, too
-            isSendClosed.set(true)
+            try {
+                onReceiveClose()
+            } finally {
+                // When receive side is closed, the send side is
+                // implicitly closed as well.
+                // We don't use sendClose() because we don't want to
+                // invoke onSendClose() since that will try to actually
+                // half-close the HTTP stream, which will fail since the
+                // closing the receive side cancels the entire thing.
+                isSendClosed.set(true)
+            }
         }
     }
 

diff --git a/library/src/main/kotlin/com/connectrpc/impl/ProtocolClient.kt b/library/src/main/kotlin/com/connectrpc/impl/ProtocolClient.kt
@@ -216,10 +216,6 @@ class ProtocolClient(
                         )
                         channel.send(message)
                     } catch (e: Throwable) {
-                        // TODO: setting isComplete, responseTrailers, and RPC status
-                        //       here seems wrong. What would prevent the call to
-                        //       channel.send such that we don't bother getting the
-                        //       actual result/trailers from the server?
                         isComplete = true
                         try {
                             channel.close(ConnectException(Code.UNKNOWN, exception = e))