Browse Source

Combine client and server error into runtime error (#1801)

Motivation:

Previously we had client error and server error to represent errors
which could only be thrown by each peer. However, some errors can be
thrown by both which resulted in the addition of runtime error. To
reduce the number of errors which can be thrown we should merge client
and server errors into the runtime error.

Modifications:

- Merge client and server error codes into runtime error
- Remove client error and server error

Results:

Less code duplication, fewer error types
George Barnett 1 year ago
parent
commit
d8550a887c

+ 0 - 142
Sources/GRPCCore/ClientError.swift

@@ -1,142 +0,0 @@
-/*
- * Copyright 2023, gRPC Authors All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/// A runtime error thrown by the client.
-///
-/// In contrast to ``RPCError``, the ``ClientError`` represents errors which happen at a scope
-/// wider than an individual RPC. For example, attempting to start a client which is already
-/// stopped would result in a ``ClientError``.
-public struct ClientError: Error, Hashable, @unchecked Sendable {
-  private var storage: Storage
-
-  // Ensures the underlying storage is unique.
-  private mutating func ensureUniqueStorage() {
-    if !isKnownUniquelyReferenced(&self.storage) {
-      self.storage = self.storage.copy()
-    }
-  }
-
-  /// The code indicating the domain of the error.
-  public var code: Code {
-    get { self.storage.code }
-    set {
-      self.ensureUniqueStorage()
-      self.storage.code = newValue
-    }
-  }
-
-  /// A message providing more details about the error which may include details specific to this
-  /// instance of the error.
-  public var message: String {
-    get { self.storage.message }
-    set {
-      self.ensureUniqueStorage()
-      self.storage.message = newValue
-    }
-  }
-
-  /// The original error which led to this error being thrown.
-  public var cause: Error? {
-    get { self.storage.cause }
-    set {
-      self.ensureUniqueStorage()
-      self.storage.cause = newValue
-    }
-  }
-
-  /// Creates a new error.
-  ///
-  /// - Parameters:
-  ///   - code: The error code.
-  ///   - message: A description of the error.
-  ///   - cause: The original error which led to this error being thrown.
-  public init(code: Code, message: String, cause: Error? = nil) {
-    self.storage = Storage(code: code, message: message, cause: cause)
-  }
-}
-
-extension ClientError: CustomStringConvertible {
-  public var description: String {
-    if let cause = self.cause {
-      return "\(self.code): \"\(self.message)\" (cause: \"\(cause)\")"
-    } else {
-      return "\(self.code): \"\(self.message)\""
-    }
-  }
-}
-
-extension ClientError {
-  private final class Storage: Hashable {
-    var code: Code
-    var message: String
-    var cause: Error?
-
-    init(code: Code, message: String, cause: Error?) {
-      self.code = code
-      self.message = message
-      self.cause = cause
-    }
-
-    func copy() -> Storage {
-      return Storage(code: self.code, message: self.message, cause: self.cause)
-    }
-
-    func hash(into hasher: inout Hasher) {
-      hasher.combine(self.code)
-      hasher.combine(self.message)
-    }
-
-    static func == (lhs: Storage, rhs: Storage) -> Bool {
-      return lhs.code == rhs.code && lhs.message == rhs.message
-    }
-  }
-}
-
-extension ClientError {
-  public struct Code: Hashable, Sendable {
-    private enum Value {
-      case clientIsAlreadyRunning
-      case clientIsStopped
-      case transportError
-    }
-
-    private var value: Value
-    private init(_ value: Value) {
-      self.value = value
-    }
-
-    /// At attempt to start the client was made but it is already running.
-    public static var clientIsAlreadyRunning: Self {
-      Self(.clientIsAlreadyRunning)
-    }
-
-    /// At attempt to start the client was made but it has already stopped.
-    public static var clientIsStopped: Self {
-      Self(.clientIsStopped)
-    }
-
-    /// The transport threw an error whilst connected.
-    public static var transportError: Self {
-      Self(.transportError)
-    }
-  }
-}
-
-extension ClientError.Code: CustomStringConvertible {
-  public var description: String {
-    String(describing: self.value)
-  }
-}

+ 4 - 4
Sources/GRPCCore/GRPCClient.swift

@@ -196,12 +196,12 @@ public struct GRPCClient: Sendable {
         // The value wasn't exchanged so the original value can't be 'notStarted'.
         fatalError()
       case .running:
-        throw ClientError(
+        throw RuntimeError(
           code: .clientIsAlreadyRunning,
           message: "The client is already running and can only be started once."
         )
       case .stopping, .stopped:
-        throw ClientError(
+        throw RuntimeError(
           code: .clientIsStopped,
           message: "The client has stopped and can only be started once."
         )
@@ -216,7 +216,7 @@ public struct GRPCClient: Sendable {
     do {
       try await self.transport.connect(lazily: false)
     } catch {
-      throw ClientError(
+      throw RuntimeError(
         code: .transportError,
         message: "The transport threw an error while connected.",
         cause: error
@@ -385,7 +385,7 @@ public struct GRPCClient: Sendable {
       // queuing the request if not yet started.
       ()
     case .stopping, .stopped:
-      throw ClientError(
+      throw RuntimeError(
         code: .clientIsStopped,
         message: "Client has been stopped. Can't make any more RPCs."
       )

+ 7 - 7
Sources/GRPCCore/GRPCServer.swift

@@ -55,7 +55,7 @@ import Atomics
 /// ## Starting and stopping the server
 ///
 /// Once you have configured the server call ``run()`` to start it. Calling ``run()`` starts each
-/// of the server's transports. A ``ServerError`` is thrown if any of the transports can't be
+/// of the server's transports. A ``RuntimeError`` is thrown if any of the transports can't be
 /// started.
 ///
 /// ```swift
@@ -154,7 +154,7 @@ public struct GRPCServer: Sendable {
   /// Starts the server and runs until all registered transports have closed.
   ///
   /// No RPCs are processed until all transports are listening. If a transport fails to start
-  /// listening then all open transports are closed and a ``ServerError`` is thrown.
+  /// listening then all open transports are closed and a ``RuntimeError`` is thrown.
   ///
   /// This function returns when all transports have stopped listening and all requests have been
   /// handled. You can signal to transports that they should stop listening by calling
@@ -163,7 +163,7 @@ public struct GRPCServer: Sendable {
   /// To stop the server more abruptly you can cancel the task that this function is running in.
   ///
   /// - Note: You can only call this function once, repeated calls will result in a
-  ///   ``ServerError`` being thrown.
+  ///   ``RuntimeError`` being thrown.
   public func run() async throws {
     let (wasNotStarted, actualState) = self.state.compareExchange(
       expected: .notStarted,
@@ -176,13 +176,13 @@ public struct GRPCServer: Sendable {
       case .notStarted:
         fatalError()
       case .starting, .running:
-        throw ServerError(
+        throw RuntimeError(
           code: .serverIsAlreadyRunning,
           message: "The server is already running and can only be started once."
         )
 
       case .stopping, .stopped:
-        throw ServerError(
+        throw RuntimeError(
           code: .serverIsStopped,
           message: "The server has stopped and can only be started once."
         )
@@ -195,7 +195,7 @@ public struct GRPCServer: Sendable {
     }
 
     if self.transports.isEmpty {
-      throw ServerError(
+      throw RuntimeError(
         code: .noTransportsConfigured,
         message: """
           Can't start server, no transports are configured. You must add at least one transport \
@@ -217,7 +217,7 @@ public struct GRPCServer: Sendable {
         // Some listeners may have started and have streams which need closing.
         await self.rejectRequests(listeners)
 
-        throw ServerError(
+        throw RuntimeError(
           code: .failedToStartTransport,
           message: """
             Server didn't start because the '\(type(of: transport))' transport threw an error \

+ 43 - 0
Sources/GRPCCore/RuntimeError.swift

@@ -108,6 +108,13 @@ extension RuntimeError {
   public struct Code: Hashable, Sendable {
     private enum Value {
       case invalidArgument
+      case serverIsAlreadyRunning
+      case serverIsStopped
+      case failedToStartTransport
+      case noTransportsConfigured
+      case clientIsAlreadyRunning
+      case clientIsStopped
+      case transportError
     }
 
     private var value: Value
@@ -115,9 +122,45 @@ extension RuntimeError {
       self.value = value
     }
 
+    /// An argument was invalid.
     public static var invalidArgument: Self {
       Self(.invalidArgument)
     }
+
+    /// At attempt to start the server was made but it is already running.
+    public static var serverIsAlreadyRunning: Self {
+      Self(.serverIsAlreadyRunning)
+    }
+
+    /// At attempt to start the server was made but it has already stopped.
+    public static var serverIsStopped: Self {
+      Self(.serverIsStopped)
+    }
+
+    /// The server couldn't be started because a transport failed to start.
+    public static var failedToStartTransport: Self {
+      Self(.failedToStartTransport)
+    }
+
+    /// The server couldn't be started because no transports were configured.
+    public static var noTransportsConfigured: Self {
+      Self(.noTransportsConfigured)
+    }
+
+    /// At attempt to start the client was made but it is already running.
+    public static var clientIsAlreadyRunning: Self {
+      Self(.clientIsAlreadyRunning)
+    }
+
+    /// At attempt to start the client was made but it has already stopped.
+    public static var clientIsStopped: Self {
+      Self(.clientIsStopped)
+    }
+
+    /// The transport threw an error whilst connected.
+    public static var transportError: Self {
+      Self(.transportError)
+    }
   }
 }
 

+ 0 - 148
Sources/GRPCCore/ServerError.swift

@@ -1,148 +0,0 @@
-/*
- * Copyright 2023, gRPC Authors All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/// A runtime error thrown by the server.
-///
-/// In contrast to ``RPCError``, the ``ServerError`` represents errors which happen at a scope
-/// wider than an individual RPC. For example, attempting to start a server which is already
-/// stopped would result in a ``ServerError``.
-public struct ServerError: Error, Hashable, @unchecked Sendable {
-  private var storage: Storage
-
-  // Ensures the underlying storage is unique.
-  private mutating func ensureUniqueStorage() {
-    if !isKnownUniquelyReferenced(&self.storage) {
-      self.storage = self.storage.copy()
-    }
-  }
-
-  /// The code indicating the domain of the error.
-  public var code: Code {
-    get { self.storage.code }
-    set {
-      self.ensureUniqueStorage()
-      self.storage.code = newValue
-    }
-  }
-
-  /// A message providing more details about the error which may include details specific to this
-  /// instance of the error.
-  public var message: String {
-    get { self.storage.message }
-    set {
-      self.ensureUniqueStorage()
-      self.storage.message = newValue
-    }
-  }
-
-  /// The original error which led to this error being thrown.
-  public var cause: Error? {
-    get { self.storage.cause }
-    set {
-      self.ensureUniqueStorage()
-      self.storage.cause = newValue
-    }
-  }
-
-  /// Creates a new error.
-  ///
-  /// - Parameters:
-  ///   - code: The error code.
-  ///   - message: A description of the error.
-  ///   - cause: The original error which led to this error being thrown.
-  public init(code: Code, message: String, cause: Error? = nil) {
-    self.storage = Storage(code: code, message: message, cause: cause)
-  }
-}
-
-extension ServerError: CustomStringConvertible {
-  public var description: String {
-    if let cause = self.cause {
-      return "\(self.code): \"\(self.message)\" (cause: \"\(cause)\")"
-    } else {
-      return "\(self.code): \"\(self.message)\""
-    }
-  }
-}
-
-extension ServerError {
-  private final class Storage: Hashable {
-    var code: Code
-    var message: String
-    var cause: Error?
-
-    init(code: Code, message: String, cause: Error?) {
-      self.code = code
-      self.message = message
-      self.cause = cause
-    }
-
-    func copy() -> Storage {
-      return Storage(code: self.code, message: self.message, cause: self.cause)
-    }
-
-    func hash(into hasher: inout Hasher) {
-      hasher.combine(self.code)
-      hasher.combine(self.message)
-    }
-
-    static func == (lhs: Storage, rhs: Storage) -> Bool {
-      return lhs.code == rhs.code && lhs.message == rhs.message
-    }
-  }
-}
-
-extension ServerError {
-  public struct Code: Hashable, Sendable {
-    private enum Value {
-      case serverIsAlreadyRunning
-      case serverIsStopped
-      case failedToStartTransport
-      case noTransportsConfigured
-    }
-
-    private var value: Value
-    private init(_ value: Value) {
-      self.value = value
-    }
-
-    /// At attempt to start the server was made but it is already running.
-    public static var serverIsAlreadyRunning: Self {
-      Self(.serverIsAlreadyRunning)
-    }
-
-    /// At attempt to start the server was made but it has already stopped.
-    public static var serverIsStopped: Self {
-      Self(.serverIsStopped)
-    }
-
-    /// The server couldn't be started because a transport failed to start.
-    public static var failedToStartTransport: Self {
-      Self(.failedToStartTransport)
-    }
-
-    /// The server couldn't be started because no transports were configured.
-    public static var noTransportsConfigured: Self {
-      Self(.noTransportsConfigured)
-    }
-  }
-}
-
-extension ServerError.Code: CustomStringConvertible {
-  public var description: String {
-    String(describing: self.value)
-  }
-}

+ 4 - 4
Tests/GRPCCoreTests/GRPCClientTests.swift

@@ -265,7 +265,7 @@ final class GRPCClientTests: XCTestCase {
       client.close()
 
       // RPC should fail now.
-      await XCTAssertThrowsErrorAsync(ofType: ClientError.self) {
+      await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
         try await client.unary(
           request: .init(message: [3, 1, 4, 1, 5]),
           descriptor: BinaryEcho.Methods.collect,
@@ -287,7 +287,7 @@ final class GRPCClientTests: XCTestCase {
           client.close()
 
           // Attempts to start a new RPC should fail.
-          await XCTAssertThrowsErrorAsync(ofType: ClientError.self) {
+          await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
             try await client.unary(
               request: .init(message: [3, 1, 4, 1, 5]),
               descriptor: BinaryEcho.Methods.collect,
@@ -372,7 +372,7 @@ final class GRPCClientTests: XCTestCase {
     try await task.value
 
     // Client is stopped, should throw an error.
-    await XCTAssertThrowsErrorAsync(ofType: ClientError.self) {
+    await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
       try await client.run()
     } errorHandler: { error in
       XCTAssertEqual(error.code, .clientIsStopped)
@@ -388,7 +388,7 @@ final class GRPCClientTests: XCTestCase {
     try await Task.sleep(for: .milliseconds(10))
 
     // Client is already running, should throw an error.
-    await XCTAssertThrowsErrorAsync(ofType: ClientError.self) {
+    await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
       try await client.run()
     } errorHandler: { error in
       XCTAssertEqual(error.code, .clientIsAlreadyRunning)

+ 4 - 4
Tests/GRPCCoreTests/GRPCServerTests.swift

@@ -313,7 +313,7 @@ final class GRPCServerTests: XCTestCase {
 
   func testTestRunServerWithNoTransport() async throws {
     let server = GRPCServer(transports: [], services: [])
-    await XCTAssertThrowsErrorAsync(ofType: ServerError.self) {
+    await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
       try await server.run()
     } errorHandler: { error in
       XCTAssertEqual(error.code, .noTransportsConfigured)
@@ -328,7 +328,7 @@ final class GRPCServerTests: XCTestCase {
     try await task.value
 
     // Server is stopped, should throw an error.
-    await XCTAssertThrowsErrorAsync(ofType: ServerError.self) {
+    await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
       try await server.run()
     } errorHandler: { error in
       XCTAssertEqual(error.code, .serverIsStopped)
@@ -337,7 +337,7 @@ final class GRPCServerTests: XCTestCase {
 
   func testRunServerWhenTransportThrows() async throws {
     let server = GRPCServer(transports: [ThrowOnRunServerTransport()], services: [])
-    await XCTAssertThrowsErrorAsync(ofType: ServerError.self) {
+    await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
       try await server.run()
     } errorHandler: { error in
       XCTAssertEqual(error.code, .failedToStartTransport)
@@ -379,7 +379,7 @@ final class GRPCServerTests: XCTestCase {
         }
       }
 
-      await XCTAssertThrowsErrorAsync(ofType: ServerError.self) {
+      await XCTAssertThrowsErrorAsync(ofType: RuntimeError.self) {
         try await server.run()
       } errorHandler: { error in
         XCTAssertEqual(error.code, .failedToStartTransport)

+ 5 - 5
Tests/GRPCCoreTests/ServerErrorTests.swift → Tests/GRPCCoreTests/RuntimeErrorTests.swift

@@ -17,10 +17,10 @@ import GRPCCore
 import XCTest
 
 @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
-final class ServerErrorTests: XCTestCase {
+final class RuntimeErrorTests: XCTestCase {
   func testCopyOnWrite() {
-    // ServerError has a heap based storage, so check CoW semantics are correctly implemented.
-    let error1 = ServerError(code: .failedToStartTransport, message: "Failed to start transport")
+    // RuntimeError has a heap based storage, so check CoW semantics are correctly implemented.
+    let error1 = RuntimeError(code: .failedToStartTransport, message: "Failed to start transport")
     var error2 = error1
     error2.code = .serverIsAlreadyRunning
     XCTAssertEqual(error1.code, .failedToStartTransport)
@@ -38,10 +38,10 @@ final class ServerErrorTests: XCTestCase {
   }
 
   func testCustomStringConvertible() {
-    let error1 = ServerError(code: .failedToStartTransport, message: "Failed to start transport")
+    let error1 = RuntimeError(code: .failedToStartTransport, message: "Failed to start transport")
     XCTAssertDescription(error1, #"failedToStartTransport: "Failed to start transport""#)
 
-    let error2 = ServerError(
+    let error2 = RuntimeError(
       code: .failedToStartTransport,
       message: "Failed to start transport",
       cause: CancellationError()