Fix error description forwarding (#143)

### Motivation Fixes apple/swift-openapi-generator#730. We were not correctly keeping `CustomStringConvertible` and `LocalizedError` conformances separate for wrapper errors like `ClientError` and `ServerError`. This lead to some user-thrown errors (in handlers, transports, and middlewares) to print less information than the error was actually providing (using a different method). ### Modifications Properly untangle the two printing codepaths, and only call `localizedDescription` from the wrapper error's `errorDescription`. Also made the `localizedDescription` strings a bit more user-friendly and less detailed, as in some apps these errors might get directly rendered by a UI component that calls `localizedDescription`. ### Result Error logging should now match adopter expectations. ### Test Plan Added unit tests for `{Client,Server}Error` printing methods.
apple · Feb 24, 2025 · acb9425 · acb9425
1 parent c118c19
commit acb9425
Show file tree

Hide file tree

Showing 11 changed files with 116 additions and 19 deletions.
diff --git a/Sources/OpenAPIRuntime/Conversion/ErrorExtensions.swift b/Sources/OpenAPIRuntime/Conversion/ErrorExtensions.swift
@@ -114,12 +114,18 @@ struct MultiError: Swift.Error, LocalizedError, CustomStringConvertible {
     var description: String {
         let combinedDescription =
             errors.map { error in
-                guard let error = error as? (any PrettyStringConvertible) else { return error.localizedDescription }
+                guard let error = error as? (any PrettyStringConvertible) else { return "\(error)" }
                 return error.prettyDescription
             }
             .enumerated().map { ($0.offset + 1, $0.element) }.map { "Error \($0.0): [\($0.1)]" }.joined(separator: ", ")
         return "MultiError (contains \(errors.count) error\(errors.count == 1 ? "" : "s")): \(combinedDescription)"
     }
 
-    var errorDescription: String? { description }
+    var errorDescription: String? {
+        if let first = errors.first {
+            return "Mutliple errors encountered, first one: \(first.localizedDescription)."
+        } else {
+            return "No errors"
+        }
+    }
 }
diff --git a/Sources/OpenAPIRuntime/Errors/ClientError.swift b/Sources/OpenAPIRuntime/Errors/ClientError.swift
@@ -109,9 +109,7 @@ public struct ClientError: Error {
     // MARK: Private
 
     fileprivate var underlyingErrorDescription: String {
-        guard let prettyError = underlyingError as? (any PrettyStringConvertible) else {
-            return underlyingError.localizedDescription
-        }
+        guard let prettyError = underlyingError as? (any PrettyStringConvertible) else { return "\(underlyingError)" }
         return prettyError.prettyDescription
     }
 }
@@ -133,5 +131,7 @@ extension ClientError: LocalizedError {
     /// This computed property provides a localized human-readable description of the client error, which is suitable for displaying to users.
     ///
     /// - Returns: A localized string describing the client error.
-    public var errorDescription: String? { description }
+    public var errorDescription: String? {
+        "Client encountered an error invoking the operation \"\(operationID)\", caused by \"\(causeDescription)\", underlying error: \(underlyingError.localizedDescription)."
+    }
 }
diff --git a/Sources/OpenAPIRuntime/Errors/CodingErrors.swift b/Sources/OpenAPIRuntime/Errors/CodingErrors.swift
@@ -21,7 +21,7 @@ extension DecodingError: PrettyStringConvertible {
         case .keyNotFound(let key, let context): output = "keyNotFound \(key) - \(context.prettyDescription)"
         case .typeMismatch(let type, let context): output = "typeMismatch \(type) - \(context.prettyDescription)"
         case .valueNotFound(let type, let context): output = "valueNotFound \(type) - \(context.prettyDescription)"
-        @unknown default: output = "unknown: \(localizedDescription)"
+        @unknown default: output = "unknown: \(self)"
         }
         return "DecodingError: \(output)"
     }
@@ -30,7 +30,7 @@ extension DecodingError: PrettyStringConvertible {
 extension DecodingError.Context: PrettyStringConvertible {
     var prettyDescription: String {
         let path = codingPath.map(\.description).joined(separator: "/")
-        return "at \(path): \(debugDescription) (underlying error: \(underlyingError?.localizedDescription ?? "<nil>"))"
+        return "at \(path): \(debugDescription) (underlying error: \(underlyingError.map { "\($0)" } ?? "<nil>"))"
     }
 }
 
@@ -39,7 +39,7 @@ extension EncodingError: PrettyStringConvertible {
         let output: String
         switch self {
         case .invalidValue(let value, let context): output = "invalidValue \(value) - \(context.prettyDescription)"
-        @unknown default: output = "unknown: \(localizedDescription)"
+        @unknown default: output = "unknown: \(self)"
         }
         return "EncodingError: \(output)"
     }
@@ -48,6 +48,6 @@ extension EncodingError: PrettyStringConvertible {
 extension EncodingError.Context: PrettyStringConvertible {
     var prettyDescription: String {
         let path = codingPath.map(\.description).joined(separator: "/")
-        return "at \(path): \(debugDescription) (underlying error: \(underlyingError?.localizedDescription ?? "<nil>"))"
+        return "at \(path): \(debugDescription) (underlying error: \(underlyingError.map { "\($0)" } ?? "<nil>"))"
     }
 }
diff --git a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift
@@ -121,6 +121,10 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
             return "Unexpected response body, expected content type: \(expectedContentType), body: \(body)"
         }
     }
+
+    // MARK: - LocalizedError
+
+    var errorDescription: String? { description }
 }
 
 /// Throws an error to indicate an unexpected HTTP response status.

diff --git a/Sources/OpenAPIRuntime/Errors/ServerError.swift b/Sources/OpenAPIRuntime/Errors/ServerError.swift
@@ -82,9 +82,7 @@ public struct ServerError: Error {
     // MARK: Private
 
     fileprivate var underlyingErrorDescription: String {
-        guard let prettyError = underlyingError as? (any PrettyStringConvertible) else {
-            return underlyingError.localizedDescription
-        }
+        guard let prettyError = underlyingError as? (any PrettyStringConvertible) else { return "\(underlyingError)" }
         return prettyError.prettyDescription
     }
 }
@@ -106,5 +104,7 @@ extension ServerError: LocalizedError {
     /// This computed property provides a localized human-readable description of the server error, which is suitable for displaying to users.
     ///
     /// - Returns: A localized string describing the server error.
-    public var errorDescription: String? { description }
+    public var errorDescription: String? {
+        "Server encountered an error handling the operation \"\(operationID)\", caused by \"\(causeDescription)\", underlying error: \(underlyingError.localizedDescription)."
+    }
 }
diff --git a/Sources/OpenAPIRuntime/Interface/ServerTransport.swift b/Sources/OpenAPIRuntime/Interface/ServerTransport.swift
@@ -197,7 +197,7 @@ public protocol ServerTransport {
 ///                 print("<<<: \(response.status.code)")
 ///                 return (response, responseBody)
 ///             } catch {
-///                 print("!!!: \(error.localizedDescription)")
+///                 print("!!!: \(error)")
 ///                 throw error
 ///             }
 ///         }

diff --git a/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Client.swift b/Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Client.swift
@@ -285,5 +285,5 @@ public func XCTAssertEqualStringifiedData(
     do {
         let actualString = String(decoding: try expression1(), as: UTF8.self)
         XCTAssertEqual(actualString, try expression2(), file: file, line: line)
-    } catch { XCTFail(error.localizedDescription, file: file, line: line) }
+    } catch { XCTFail("\(error)", file: file, line: line) }
 }
diff --git a/Tests/OpenAPIRuntimeTests/Errors/Test_ClientError.swift b/Tests/OpenAPIRuntimeTests/Errors/Test_ClientError.swift
@@ -0,0 +1,39 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the SwiftOpenAPIGenerator open source project
+//
+// Copyright (c) 2025 Apple Inc. and the SwiftOpenAPIGenerator project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of SwiftOpenAPIGenerator project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+import HTTPTypes
+@_spi(Generated) @testable import OpenAPIRuntime
+import XCTest
+
+final class Test_ServerError: XCTestCase {
+    func testPrinting() throws {
+        let upstreamError = RuntimeError.handlerFailed(PrintableError())
+        let error: any Error = ServerError(
+            operationID: "op",
+            request: .init(soar_path: "/test", method: .get),
+            requestBody: nil,
+            requestMetadata: .init(),
+            causeDescription: upstreamError.prettyDescription,
+            underlyingError: upstreamError.underlyingError ?? upstreamError
+        )
+        XCTAssertEqual(
+            "\(error)",
+            "Server error - cause description: 'User handler threw an error.', underlying error: Just description, operationID: op, request: GET /test [], requestBody: <nil>, metadata: Path parameters: [:], operationInput: <nil>, operationOutput: <nil>"
+        )
+        XCTAssertEqual(
+            error.localizedDescription,
+            "Server encountered an error handling the operation \"op\", caused by \"User handler threw an error.\", underlying error: Just errorDescription."
+        )
+    }
+}
diff --git a/Tests/OpenAPIRuntimeTests/Errors/Test_RuntimeError.swift b/Tests/OpenAPIRuntimeTests/Errors/Test_RuntimeError.swift
@@ -68,6 +68,12 @@ final class Test_RuntimeError: XCTestCase {
         )
         XCTAssertEqual(response.0.status, .badGateway)
     }
+
+    func testDescriptions() async throws {
+        let error: any Error = RuntimeError.transportFailed(PrintableError())
+        XCTAssertEqual("\(error)", "Transport threw an error.")
+        XCTAssertEqual(error.localizedDescription, "Transport threw an error.")
+    }
 }
 
 enum TestErrorConvertible: Error, HTTPResponseConvertible {

diff --git a/Tests/OpenAPIRuntimeTests/Errors/Test_ServerError.swift b/Tests/OpenAPIRuntimeTests/Errors/Test_ServerError.swift
@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the SwiftOpenAPIGenerator open source project
+//
+// Copyright (c) 2025 Apple Inc. and the SwiftOpenAPIGenerator project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of SwiftOpenAPIGenerator project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+import HTTPTypes
+@_spi(Generated) @testable import OpenAPIRuntime
+import XCTest
+
+final class Test_ClientError: XCTestCase {
+    func testPrinting() throws {
+        let upstreamError = RuntimeError.transportFailed(PrintableError())
+        let error: any Error = ClientError(
+            operationID: "op",
+            operationInput: "test",
+            causeDescription: upstreamError.prettyDescription,
+            underlyingError: upstreamError.underlyingError ?? upstreamError
+        )
+        XCTAssertEqual(
+            "\(error)",
+            "Client error - cause description: 'Transport threw an error.', underlying error: Just description, operationID: op, operationInput: test, request: <nil>, requestBody: <nil>, baseURL: <nil>, response: <nil>, responseBody: <nil>"
+        )
+        XCTAssertEqual(
+            error.localizedDescription,
+            "Client encountered an error invoking the operation \"op\", caused by \"Transport threw an error.\", underlying error: Just errorDescription."
+        )
+    }
+}
diff --git a/Tests/OpenAPIRuntimeTests/Test_Runtime.swift b/Tests/OpenAPIRuntimeTests/Test_Runtime.swift
@@ -210,6 +210,11 @@ extension ArraySlice<UInt8> {
 
 struct TestError: Error, Equatable {}
 
+struct PrintableError: Error, CustomStringConvertible, LocalizedError {
+    var description: String { "Just description" }
+    var errorDescription: String? { "Just errorDescription" }
+}
+
 struct MockMiddleware: ClientMiddleware, ServerMiddleware {
     enum FailurePhase {
         case never
@@ -345,7 +350,7 @@ struct PrintingMiddleware: ClientMiddleware {
             print("Received: \(response.status)")
             return (response, responseBody)
         } catch {
-            print("Failed with error: \(error.localizedDescription)")
+            print("Failed with error: \(error)")
             throw error
         }
     }
@@ -373,7 +378,7 @@ public func XCTAssertEqualStringifiedData<S: Sequence>(
         }
         let actualString = String(decoding: Array(value1), as: UTF8.self)
         XCTAssertEqual(actualString, try expression2(), file: file, line: line)
-    } catch { XCTFail(error.localizedDescription, file: file, line: line) }
+    } catch { XCTFail("\(error)", file: file, line: line) }
 }
 
 /// Asserts that the string representation of binary data in an HTTP body is equal to an expected string.
@@ -454,7 +459,7 @@ public func XCTAssertEqualData<C1: Collection, C2: Collection>(
             file: file,
             line: line
         )
-    } catch { XCTFail(error.localizedDescription, file: file, line: line) }
+    } catch { XCTFail("\(error)", file: file, line: line) }
 }
 
 /// Asserts that the data matches the expected value.