Browse Source

Don't remove status and message from trailing metadata (#470)

- removing the status and message triggers an unnecessary CoW
George Barnett 6 years ago
parent
commit
4df3d91a0c

+ 1 - 5
Sources/SwiftGRPCNIO/HTTP1ToRawGRPCClientCodec.swift

@@ -152,11 +152,7 @@ extension HTTP1ToRawGRPCClientCodec: ChannelInboundHandler {
       GRPCStatusMessageMarshaller.unmarshall($0)
     }
 
-    var trailingCustomMetadata = trailers ?? HTTPHeaders()
-    trailingCustomMetadata.remove(name: GRPCHeaderName.statusCode)
-    trailingCustomMetadata.remove(name: GRPCHeaderName.statusMessage)
-
-    let status = GRPCStatus(code: statusCode, message: statusMessage, trailingMetadata: trailingCustomMetadata)
+    let status = GRPCStatus(code: statusCode, message: statusMessage, trailingMetadata: trailers ?? HTTPHeaders())
 
     context.fireChannelRead(wrapInboundOut(.status(status)))
     return .ignore

+ 19 - 9
Tests/SwiftGRPCNIOTests/ServerThrowingTests.swift

@@ -24,6 +24,16 @@ import XCTest
 let thrownError = GRPCStatus(code: .internalError, message: "expected error")
 let transformedError = GRPCStatus(code: .aborted, message: "transformed error")
 
+func assertEqualStatusIgnoringTrailers(_ status: GRPCStatus, _ otherStatus: GRPCStatus?) {
+  guard let otherStatus = otherStatus else {
+    XCTFail("status was nil")
+    return
+  }
+
+  XCTAssertEqual(status.code, otherStatus.code)
+  XCTAssertEqual(status.message, otherStatus.message)
+}
+
 // Motivation for two different providers: Throwing immediately causes the event observer future (in the
 // client-streaming and bidi-streaming cases) to throw immediately, _before_ the corresponding handler has even added
 // to the channel. We want to test that case as well as the one where we throw only _after_ the handler has been added
@@ -121,36 +131,36 @@ class ServerErrorTransformingTests: ServerThrowingTests {
 extension ServerThrowingTests {
   func testUnary() throws {
     let call = client.get(Echo_EchoRequest(text: "foo"))
-    XCTAssertEqual(expectedError, try call.status.wait())
+    assertEqualStatusIgnoringTrailers(expectedError, try call.status.wait())
     XCTAssertThrowsError(try call.response.wait()) {
-      XCTAssertEqual(expectedError, $0 as? GRPCStatus)
+      assertEqualStatusIgnoringTrailers(expectedError, $0 as? GRPCStatus)
     }
   }
 
-  func testClientStreaming() {
+  func testClientStreaming() throws {
     let call = client.collect()
     XCTAssertNoThrow(try call.sendEnd().wait())
-    XCTAssertEqual(expectedError, try call.status.wait())
+    assertEqualStatusIgnoringTrailers(expectedError, try call.status.wait())
 
     if type(of: makeEchoProvider()) != ErrorReturningEchoProviderNIO.self {
       // With `ErrorReturningEchoProviderNIO` we actually _return_ a response, which means that the `response` future
       // will _not_ fail, so in that case this test doesn't apply.
       XCTAssertThrowsError(try call.response.wait()) {
-        XCTAssertEqual(expectedError, $0 as? GRPCStatus)
+        assertEqualStatusIgnoringTrailers(expectedError, $0 as? GRPCStatus)
       }
     }
   }
 
-  func testServerStreaming() {
+  func testServerStreaming() throws {
     let call = client.expand(Echo_EchoRequest(text: "foo")) { XCTFail("no message expected, got \($0)") }
     // Nothing to throw here, but the `status` should be the expected error.
-    XCTAssertEqual(expectedError, try call.status.wait())
+    assertEqualStatusIgnoringTrailers(expectedError, try call.status.wait())
   }
 
-  func testBidirectionalStreaming() {
+  func testBidirectionalStreaming() throws {
     let call = client.update() { XCTFail("no message expected, got \($0)") }
     XCTAssertNoThrow(try call.sendEnd().wait())
     // Nothing to throw here, but the `status` should be the expected error.
-    XCTAssertEqual(expectedError, try call.status.wait())
+    assertEqualStatusIgnoringTrailers(expectedError, try call.status.wait())
   }
 }