Provide errors to failed client promises rather than a status (#859)

Motivation:

If an RPC fails for some reason we will convert the error into an
appropriate `GRPCStatus`. This status is used to succeed the status
promise and fail any other promises (initial and trailing metadata, and
if applicable, the response promise). Without looking at logs it's
harder to determine the exact nature of the error.

Modifications:

- Fail the initial metadata, trailing metadata and response promise with
  the error that the status was derived from.

Result:

More helpful errors.

Sources/GRPC/ClientCalls/ClientCallTransport.swift

@@ -373,9 +373,11 @@ extension ChannelTransport {
       var loggerWithState = self.logger
       loggerWithState[metadataKey: "call_state"] = "\(self.describeCallState())"
       let errorStatus: GRPCStatus
+      let errorWithoutContext: Error
 
       if let errorWithContext = error as? GRPCError.WithContext {
         errorStatus = errorWithContext.error.makeGRPCStatus()
+        errorWithoutContext = errorWithContext.error
         self.errorDelegate?.didCatchError(
           errorWithContext.error,
           logger: loggerWithState,
@@ -384,14 +386,16 @@ extension ChannelTransport {
         )
       } else if let transformable = error as? GRPCStatusTransformable {
         errorStatus = transformable.makeGRPCStatus()
+        errorWithoutContext = error
         self.errorDelegate?.didCatchErrorWithoutContext(error, logger: loggerWithState)
       } else {
         errorStatus = .processingError
+        errorWithoutContext = error
         self.errorDelegate?.didCatchErrorWithoutContext(error, logger: loggerWithState)
       }
 
       // Update our state: we're closing.
-      self.close(withStatus: errorStatus)
+      self.close(error: errorWithoutContext, status: errorStatus)
       promise?.fail(errorStatus)
 
     case .closed:
@@ -402,7 +406,7 @@ extension ChannelTransport {
   /// Close the call, if it's not yet closed with the given status.
   ///
   /// Must be called from the event loop.
-  private func close(withStatus status: GRPCStatus) {
+  private func close(error: Error, status: GRPCStatus) {
     self.eventLoop.preconditionInEventLoop()
 
     switch self.state {
@@ -416,7 +420,7 @@ extension ChannelTransport {
       self.scheduledTimeout = nil
 
       // Fail any outstanding promises.
-      self.responseContainer.fail(with: status)
+      self.responseContainer.fail(with: error, status: status)
 
       // Fail any buffered writes.
       while !self.requestBuffer.isEmpty {
@@ -439,7 +443,7 @@ extension ChannelTransport {
       self.scheduledTimeout = nil
 
       // Fail any outstanding promises.
-      self.responseContainer.fail(with: status)
+      self.responseContainer.fail(with: error, status: status)
 
       // Close the channel.
       channel.close(mode: .all, promise: nil)
@@ -502,7 +506,7 @@ extension ChannelTransport: ClientCallInbound {
         // We're not really failing the status here; in some cases the server may fast fail, in which
         // case we'll only see trailing metadata and status: we should fail the initial metadata and
         // response in that case.
-        self.responseContainer.fail(with: status)
+        self.responseContainer.fail(with: status, status: status)
       }
 
     case .closed:

Sources/GRPC/ClientCalls/ResponsePartContainer.swift

@@ -37,16 +37,16 @@ internal struct ResponsePartContainer<Response: GRPCPayload> {
 
   /// Fail all promises - except for the status promise - with the given error status. Succeed the
   /// status promise.
-  mutating func fail(with status: GRPCStatus) {
-    self.lazyInitialMetadataPromise.fail(status)
+  mutating func fail(with error: Error, status: GRPCStatus) {
+    self.lazyInitialMetadataPromise.fail(error)
 
     switch self.responseHandler {
     case .unary(let response):
-      response.fail(status)
+      response.fail(error)
     case .stream:
       ()
     }
-    self.lazyTrailingMetadataPromise.fail(status)
+    self.lazyTrailingMetadataPromise.fail(error)
     // We always succeed the status.
     self.lazyStatusPromise.succeed(status)
   }

Tests/GRPCTests/ChannelTransportTests.swift

@@ -340,6 +340,36 @@ class ChannelTransportTests: GRPCTestCase {
     // Promise should fail.
     XCTAssertThrowsError(try requestHeadPromise.futureResult.wait())
   }
+
+  func testErrorsAreNotAlwaysStatus() throws {
+    let channel = EmbeddedChannel()
+    let responsePromise = channel.eventLoop.makePromise(of: Response.self)
+    let container = ResponsePartContainer<Response>(
+      eventLoop: channel.eventLoop,
+      unaryResponsePromise: responsePromise
+    )
+
+    let transport = self.makeEmbeddedTransport(channel: channel, container: container)
+    transport.activate(stream: channel)
+
+    // Send an error
+    transport.receiveError(GRPCError.RPCCancelledByClient())
+
+    XCTAssertThrowsError(try transport.responseContainer.lazyInitialMetadataPromise.getFutureResult().wait()) { error in
+      XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
+    }
+
+    XCTAssertThrowsError(try transport.responseContainer.lazyTrailingMetadataPromise.getFutureResult().wait()) { error in
+      XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
+    }
+
+    XCTAssertThrowsError(try responsePromise.futureResult.wait()) { error in
+      XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
+    }
+
+    // Status never fails.
+    XCTAssertNoThrow(try transport.responseContainer.lazyStatusPromise.getFutureResult().wait())
+  }
 }
 
 extension _GRPCClientRequestPart {

Tests/GRPCTests/ClientCancellingTests.swift

@@ -27,7 +27,7 @@ class ClientCancellingTests: EchoTestCaseBase {
     call.cancel(promise: nil)
 
     call.response.whenFailure { error in
-      XCTAssertEqual((error as? GRPCStatus)?.code, .cancelled)
+      XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
       responseReceived.fulfill()
     }
 
@@ -47,7 +47,7 @@ class ClientCancellingTests: EchoTestCaseBase {
     call.cancel(promise: nil)
 
     call.response.whenFailure { error in
-      XCTAssertEqual((error as? GRPCStatus)?.code, .cancelled)
+      XCTAssertTrue(error is GRPCError.RPCCancelledByClient)
       responseReceived.fulfill()
     }
 

Tests/GRPCTests/ClientTimeoutTests.swift

@@ -50,17 +50,13 @@ class ClientTimeoutTests: GRPCTestCase {
     XCTAssertNoThrow(try self.channel.finish())
   }
 
-  func assertDeadlineExceeded(_ response: EventLoopFuture<Echo_EchoResponse>, expectation: XCTestExpectation) {
+  func assertRPCTimedOut(_ response: EventLoopFuture<Echo_EchoResponse>, expectation: XCTestExpectation) {
     response.whenComplete { result in
       switch result {
       case .success(let response):
         XCTFail("unexpected response: \(response)")
       case .failure(let error):
-        if let status = error as? GRPCStatus {
-          XCTAssertEqual(status.code, .deadlineExceeded)
-        } else {
-          XCTFail("unexpected error: \(error)")
-        }
+        XCTAssertTrue(error is GRPCError.RPCTimedOut)
       }
       expectation.fulfill()
     }
@@ -105,7 +101,7 @@ class ClientTimeoutTests: GRPCTestCase {
     let call = client.collect()
     channel.embeddedEventLoop.advanceTime(by: self.timeout)
 
-    self.assertDeadlineExceeded(call.response, expectation: responseExpectation)
+    self.assertRPCTimedOut(call.response, expectation: responseExpectation)
     self.assertDeadlineExceeded(call.status, expectation: statusExpectation)
     self.wait(for: [responseExpectation, statusExpectation], timeout: self.testTimeout)
   }
@@ -116,7 +112,7 @@ class ClientTimeoutTests: GRPCTestCase {
 
     let call = client.collect()
 
-    self.assertDeadlineExceeded(call.response, expectation: responseExpectation)
+    self.assertRPCTimedOut(call.response, expectation: responseExpectation)
     self.assertDeadlineExceeded(call.status, expectation: statusExpectation)
 
     call.sendMessage(Echo_EchoRequest(text: "foo"), promise: nil)

Tests/GRPCTests/XCTestManifests.swift

@@ -22,6 +22,7 @@ extension ChannelTransportTests {
         ("testBufferedWritesAreFailedOnClose", testBufferedWritesAreFailedOnClose),
         ("testChannelBecomesInactive", testChannelBecomesInactive),
         ("testChannelError", testChannelError),
+        ("testErrorsAreNotAlwaysStatus", testErrorsAreNotAlwaysStatus),
         ("testInboundMethodsAfterShutdown", testInboundMethodsAfterShutdown),
         ("testOutboundMethodsAfterShutdown", testOutboundMethodsAfterShutdown),
         ("testTimeoutAfterActivating", testTimeoutAfterActivating),