Add a 'cause' to 'GRPCStatus' (#1290)

Motivation:

In some causes it's useful to know the underlying error which caused an
RPC to complete with a particular status. The current way of achieving
this is by embeddeding it in the status message. While the information
may be relevant to some callers it is noise to others. Moreover, to
those that want the information, they have to know how to search for it
(some strings may embed errors in different ways and so on). This also
precludes them from being able to switch on a particular error type, for
example.

Modifications:

- Add an optional 'cause' field to 'GRPCStatus', an error associated
  with the status.
- Adding cause directly into 'GRPCStatus' made it a little too wide so a
  backing storage was added for the storage and message. This is a bit
  of a shame since we now unconditionally allocate for messages but the
  happy path ('ok') should be without message or cause and so should not
  allocate.

Result:

- Causal errors can be attached to 'GRPCStatus'

Sources/GRPC/GRPCStatus.swift

@@ -20,20 +20,81 @@ import NIOHTTP2
 
 /// Encapsulates the result of a gRPC call.
 public struct GRPCStatus: Error {
-  /// The status message of the RPC.
-  public var message: String?
+  /// Storage for message/cause. In the happy case ('ok') there will not be a message or cause
+  /// and this will reference a static storage containing nil values. Making it optional makes the
+  /// setters for message and cause a little messy.
+  private var storage: Storage
 
   /// The status code of the RPC.
   public var code: Code
 
+  /// The status message of the RPC.
+  public var message: String? {
+    get {
+      return self.storage.message
+    }
+    set {
+      if isKnownUniquelyReferenced(&self.storage) {
+        self.storage.message = newValue
+      } else {
+        self.storage = .makeStorage(message: newValue, cause: self.storage.cause)
+      }
+    }
+  }
+
+  /// The cause of an error (not 'ok') status. This value is never transmitted over the wire and is
+  /// **not** included in equality checks.
+  public var cause: Error? {
+    get {
+      return self.storage.cause
+    }
+    set {
+      if isKnownUniquelyReferenced(&self.storage) {
+        self.storage.cause = newValue
+      } else {
+        self.storage = .makeStorage(message: self.storage.message, cause: newValue)
+      }
+    }
+  }
+
+  // Backing storage for 'message' and 'cause'.
+  private final class Storage {
+    // On many happy paths there will be no message or cause, so we'll use this shared reference
+    // instead of allocating a new storage each time.
+    //
+    // Alternatively: `GRPCStatus` could hold a storage optionally however doing so made the code
+    // quite unreadable.
+    private static let none = Storage(message: nil, cause: nil)
+
+    private init(message: String?, cause: Error?) {
+      self.message = message
+      self.cause = cause
+    }
+
+    fileprivate var message: Optional<String>
+    fileprivate var cause: Optional<Error>
+
+    fileprivate static func makeStorage(message: String?, cause: Error?) -> Storage {
+      if message == nil, cause == nil {
+        return Storage.none
+      } else {
+        return Storage(message: message, cause: cause)
+      }
+    }
+  }
+
   /// Whether the status is '.ok'.
   public var isOk: Bool {
     return self.code == .ok
   }
 
   public init(code: Code, message: String?) {
+    self.init(code: code, message: message, cause: nil)
+  }
+
+  public init(code: Code, message: String? = nil, cause: Error? = nil) {
     self.code = code
-    self.message = message
+    self.storage = .makeStorage(message: message, cause: cause)
   }
 
   // Frequently used "default" statuses.
@@ -66,6 +127,12 @@ extension GRPCStatus: CustomStringConvertible {
   }
 }
 
+extension GRPCStatus {
+  internal var testingOnly_storageObjectIdentifier: ObjectIdentifier {
+    return ObjectIdentifier(self.storage)
+  }
+}
+
 extension GRPCStatus {
   /// Status codes for gRPC operations (replicated from `status_code_enum.h` in the
   /// [gRPC core library](https://github.com/grpc/grpc)).
@@ -257,13 +324,13 @@ extension GRPCStatus: GRPCStatusTransformable {
 
 extension NIOHTTP2Errors.StreamClosed: GRPCStatusTransformable {
   public func makeGRPCStatus() -> GRPCStatus {
-    return .init(code: .unavailable, message: self.localizedDescription)
+    return .init(code: .unavailable, message: self.localizedDescription, cause: self)
   }
 }
 
 extension NIOHTTP2Errors.IOOnClosedConnection: GRPCStatusTransformable {
   public func makeGRPCStatus() -> GRPCStatus {
-    return .init(code: .unavailable, message: "The connection is closed")
+    return .init(code: .unavailable, message: "The connection is closed", cause: self)
   }
 }
 
@@ -271,10 +338,12 @@ extension ChannelError: GRPCStatusTransformable {
   public func makeGRPCStatus() -> GRPCStatus {
     switch self {
     case .inputClosed, .outputClosed, .ioOnClosedChannel:
-      return .init(code: .unavailable, message: "The connection is closed")
+      return .init(code: .unavailable, message: "The connection is closed", cause: self)
 
     default:
-      return .processingError
+      var processingError = GRPCStatus.processingError
+      processingError.cause = self
+      return processingError
     }
   }
 }

Tests/GRPCTests/GRPCStatusTests.swift

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import GRPC
+@testable import GRPC
 import XCTest
 
 class GRPCStatusTests: GRPCTestCase {
@@ -50,4 +50,54 @@ class GRPCStatusTests: GRPCTestCase {
       String(describing: GRPCStatus(code: .failedPrecondition, message: "invalid state"))
     )
   }
+
+  func testCoWSemanticsModifyingMessage() {
+    let nilStorageID = GRPCStatus.ok.testingOnly_storageObjectIdentifier
+    var status = GRPCStatus(code: .resourceExhausted)
+
+    // No message/cause, so uses the nil backing storage.
+    XCTAssertEqual(status.testingOnly_storageObjectIdentifier, nilStorageID)
+
+    status.message = "no longer using the nil backing storage"
+    let storageID = status.testingOnly_storageObjectIdentifier
+    XCTAssertNotEqual(storageID, nilStorageID)
+    XCTAssertEqual(status.message, "no longer using the nil backing storage")
+
+    // The storage of status should be uniquely ref'd, so setting message to nil should not change
+    // the backing storage (even if the nil storage could now be used).
+    status.message = nil
+    XCTAssertEqual(status.testingOnly_storageObjectIdentifier, storageID)
+    XCTAssertNil(status.message)
+  }
+
+  func testCoWSemanticsModifyingCause() {
+    let nilStorageID = GRPCStatus.ok.testingOnly_storageObjectIdentifier
+    var status = GRPCStatus(code: .cancelled)
+
+    // No message/cause, so uses the nil backing storage.
+    XCTAssertEqual(status.testingOnly_storageObjectIdentifier, nilStorageID)
+
+    status.cause = ConnectionPoolError.tooManyWaiters
+    let storageID = status.testingOnly_storageObjectIdentifier
+    XCTAssertNotEqual(storageID, nilStorageID)
+    XCTAssert(status.cause is ConnectionPoolError)
+
+    // The storage of status should be uniquely ref'd, so setting cause to nil should not change
+    // the backing storage (even if the nil storage could now be used).
+    status.cause = nil
+    XCTAssertEqual(status.testingOnly_storageObjectIdentifier, storageID)
+    XCTAssertNil(status.cause)
+  }
+
+  func testStatusesWithNoMessageOrCauseShareBackingStorage() {
+    let validStatusCodes = (0 ... 16)
+    let statuses: [GRPCStatus] = validStatusCodes.map { code in
+      // 0...16 are all valid, '!' is fine.
+      let code = GRPCStatus.Code(rawValue: code)!
+      return GRPCStatus(code: code)
+    }
+
+    let storageIDs = Set(statuses.map { $0.testingOnly_storageObjectIdentifier })
+    XCTAssertEqual(storageIDs.count, 1)
+  }
 }