From 0545d61b2a03e8d689b9611ba4e86fe6c0a723f9 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Wed, 25 Oct 2023 16:50:50 +0200 Subject: [PATCH 1/5] Better propagate ClientError/ServerError underlying errors --- .../OpenAPIRuntime/Errors/ClientError.swift | 10 +- .../OpenAPIRuntime/Errors/RuntimeError.swift | 18 +- .../OpenAPIRuntime/Errors/ServerError.swift | 12 +- .../Interface/UniversalClient.swift | 25 ++- .../Interface/UniversalServer.swift | 14 +- .../Interface/Test_UniversalClient.swift | 154 ++++++++++++++++++ .../Interface/Test_UniversalServer.swift | 127 ++++++++++++++- Tests/OpenAPIRuntimeTests/Test_Runtime.swift | 2 + 8 files changed, 348 insertions(+), 14 deletions(-) create mode 100644 Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift diff --git a/Sources/OpenAPIRuntime/Errors/ClientError.swift b/Sources/OpenAPIRuntime/Errors/ClientError.swift index 53df18f3..b820bd4a 100644 --- a/Sources/OpenAPIRuntime/Errors/ClientError.swift +++ b/Sources/OpenAPIRuntime/Errors/ClientError.swift @@ -64,6 +64,10 @@ public struct ClientError: Error { /// Will be nil if the error resulted before the response was received. public var responseBody: HTTPBody? + /// A user-facing description of what caused the underlying error + /// to be thrown. + public var causeDescription: String + /// The underlying error that caused the operation to fail. public var underlyingError: any Error @@ -76,6 +80,8 @@ public struct ClientError: Error { /// - baseURL: The base URL for HTTP requests. /// - response: The HTTP response received during the operation. /// - responseBody: The HTTP response body received during the operation. + /// - causeDescription: A user-facing description of what caused + /// the underlying error to be thrown. /// - underlyingError: The underlying error that caused the operation /// to fail. public init( @@ -86,6 +92,7 @@ public struct ClientError: Error { baseURL: URL? = nil, response: HTTPResponse? = nil, responseBody: HTTPBody? = nil, + causeDescription: String, underlyingError: any Error ) { self.operationID = operationID @@ -95,6 +102,7 @@ public struct ClientError: Error { self.baseURL = baseURL self.response = response self.responseBody = responseBody + self.causeDescription = causeDescription self.underlyingError = underlyingError } @@ -115,7 +123,7 @@ extension ClientError: CustomStringConvertible { /// /// - Returns: A string describing the client error and its associated details. public var description: String { - "Client error - operationID: \(operationID), operationInput: \(String(describing: operationInput)), request: \(request?.prettyDescription ?? ""), requestBody: \(requestBody?.prettyDescription ?? ""), baseURL: \(baseURL?.absoluteString ?? ""), response: \(response?.prettyDescription ?? ""), responseBody: \(responseBody?.prettyDescription ?? "") , underlying error: \(underlyingErrorDescription)" + "Client error - cause description: '\(causeDescription)', underlying error: \(underlyingErrorDescription), operationID: \(operationID), operationInput: \(String(describing: operationInput)), request: \(request?.prettyDescription ?? ""), requestBody: \(requestBody?.prettyDescription ?? ""), baseURL: \(baseURL?.absoluteString ?? ""), response: \(response?.prettyDescription ?? ""), responseBody: \(responseBody?.prettyDescription ?? "")" } } diff --git a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift index 74eb1ef3..4c06048d 100644 --- a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift +++ b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift @@ -60,6 +60,16 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret case unexpectedResponseStatus(expectedStatus: String, response: any Sendable) case unexpectedResponseBody(expectedContent: String, body: any Sendable) + /// A wrapped root cause error, if one was thrown by other code. + var underlyingError: (any Error)? { + switch self { + case .transportFailed(let error), .handlerFailed(let error): + return error + default: + return nil + } + } + // MARK: CustomStringConvertible var description: String { @@ -99,10 +109,10 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret return "Missing required request body" case .missingRequiredResponseBody: return "Missing required response body" - case .transportFailed(let underlyingError): - return "Transport failed with error: \(underlyingError.localizedDescription)" - case .handlerFailed(let underlyingError): - return "User handler failed with error: \(underlyingError.localizedDescription)" + case .transportFailed: + return "Transport threw an error." + case .handlerFailed: + return "User handler threw an error." case .unexpectedResponseStatus(let expectedStatus, let response): return "Unexpected response, expected status code: \(expectedStatus), response: \(response)" case .unexpectedResponseBody(let expectedContentType, let body): diff --git a/Sources/OpenAPIRuntime/Errors/ServerError.swift b/Sources/OpenAPIRuntime/Errors/ServerError.swift index 1fee3a96..7595a890 100644 --- a/Sources/OpenAPIRuntime/Errors/ServerError.swift +++ b/Sources/OpenAPIRuntime/Errors/ServerError.swift @@ -40,6 +40,10 @@ public struct ServerError: Error { /// Is nil if error was thrown before/during Output -> response conversion. public var operationOutput: (any Sendable)? + /// A user-facing description of what caused the underlying error + /// to be thrown. + public var causeDescription: String + /// The underlying error that caused the operation to fail. public var underlyingError: any Error @@ -51,6 +55,8 @@ public struct ServerError: Error { /// - requestMetadata: The request metadata extracted by the server. /// - operationInput: An operation-specific Input value. /// - operationOutput: An operation-specific Output value. + /// - causeDescription: A user-facing description of what caused + /// the underlying error to be thrown. /// - underlyingError: The underlying error that caused the operation /// to fail. public init( @@ -60,7 +66,8 @@ public struct ServerError: Error { requestMetadata: ServerRequestMetadata, operationInput: (any Sendable)? = nil, operationOutput: (any Sendable)? = nil, - underlyingError: (any Error) + causeDescription: String, + underlyingError: any Error ) { self.operationID = operationID self.request = request @@ -68,6 +75,7 @@ public struct ServerError: Error { self.requestMetadata = requestMetadata self.operationInput = operationInput self.operationOutput = operationOutput + self.causeDescription = causeDescription self.underlyingError = underlyingError } @@ -88,7 +96,7 @@ extension ServerError: CustomStringConvertible { /// /// - Returns: A string describing the server error and its associated details. public var description: String { - "Server error - operationID: \(operationID), request: \(request.prettyDescription), requestBody: \(requestBody?.prettyDescription ?? ""), metadata: \(requestMetadata.description), operationInput: \(operationInput.map { String(describing: $0) } ?? ""), operationOutput: \(operationOutput.map { String(describing: $0) } ?? ""), underlying error: \(underlyingErrorDescription)" + "Server error - cause description: '\(causeDescription)', underlying error: \(underlyingErrorDescription), operationID: \(operationID), request: \(request.prettyDescription), requestBody: \(requestBody?.prettyDescription ?? ""), metadata: \(requestMetadata.description), operationInput: \(operationInput.map { String(describing: $0) } ?? ""), operationOutput: \(operationOutput.map { String(describing: $0) } ?? "")" } } diff --git a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift index 06eaaf79..8c6a05ae 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift @@ -110,7 +110,16 @@ import Foundation responseBody: HTTPBody? = nil, error: any Error ) -> any Error { - ClientError( + let causeDescription: String + let underlyingError: any Error + if let runtimeError = error as? RuntimeError { + causeDescription = runtimeError.prettyDescription + underlyingError = runtimeError.underlyingError ?? error + } else { + causeDescription = "Unknown" + underlyingError = error + } + return ClientError( operationID: operationID, operationInput: input, request: request, @@ -118,7 +127,8 @@ import Foundation baseURL: baseURL, response: response, responseBody: responseBody, - underlyingError: error + causeDescription: causeDescription, + underlyingError: underlyingError ) } let (request, requestBody): (HTTPRequest, HTTPBody?) = try await wrappingErrors { @@ -154,12 +164,19 @@ import Foundation } return try await next(request, requestBody, baseURL) } mapError: { error in - makeError(request: request, baseURL: baseURL, error: error) + makeError(request: request, requestBody: requestBody, baseURL: baseURL, error: error) } return try await wrappingErrors { try await deserializer(response, responseBody) } mapError: { error in - makeError(request: request, baseURL: baseURL, response: response, error: error) + makeError( + request: request, + requestBody: requestBody, + baseURL: baseURL, + response: response, + responseBody: responseBody, + error: error + ) } } } diff --git a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift index 6fba52a2..c14dea72 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift @@ -119,14 +119,24 @@ import struct Foundation.URLComponents output: OperationOutput? = nil, error: any Error ) -> any Error { - ServerError( + let causeDescription: String + let underlyingError: any Error + if let runtimeError = error as? RuntimeError { + causeDescription = runtimeError.prettyDescription + underlyingError = runtimeError.underlyingError ?? error + } else { + causeDescription = "Unknown" + underlyingError = error + } + return ServerError( operationID: operationID, request: request, requestBody: requestBody, requestMetadata: metadata, operationInput: input, operationOutput: output, - underlyingError: error + causeDescription: causeDescription, + underlyingError: underlyingError ) } var next: @Sendable (HTTPRequest, HTTPBody?, ServerRequestMetadata) async throws -> (HTTPResponse, HTTPBody?) = diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift new file mode 100644 index 00000000..b5fd056d --- /dev/null +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift @@ -0,0 +1,154 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftOpenAPIGenerator open source project +// +// Copyright (c) 2023 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 XCTest +import HTTPTypes +import Foundation +@_spi(Generated) @testable import OpenAPIRuntime + +struct MockClientTransport: ClientTransport { + var sendBlock: @Sendable (HTTPRequest, HTTPBody?, URL, String) async throws -> (HTTPResponse, HTTPBody?) + func send( + _ request: HTTPRequest, + body: HTTPBody?, + baseURL: URL, + operationID: String + ) async throws -> (HTTPResponse, HTTPBody?) { + try await sendBlock(request, body, baseURL, operationID) + } + + static let requestBody: HTTPBody = HTTPBody("hello") + static let responseBody: HTTPBody = HTTPBody("bye") + + static var successful: Self { + MockClientTransport { _, _, _, _ in + (HTTPResponse(status: .ok), responseBody) + } + } + + static var failing: Self { + MockClientTransport { _, _, _, _ in + throw TestError() + } + } +} + +final class Test_UniversalClient: Test_Runtime { + + func testSuccess() async throws { + let client = UniversalClient(transport: MockClientTransport.successful) + let output = try await client.send( + input: "input", + forOperation: "op", + serializer: { input in + ( + HTTPRequest(soar_path: "/", method: .post), + MockClientTransport.requestBody + ) + }, + deserializer: { response, body in + let body = try XCTUnwrap(body) + let string = try await String(collecting: body, upTo: 10) + return string + } + ) + XCTAssertEqual(output, "bye") + } + + func testErrorPropagation_serializer() async throws { + do { + let client = UniversalClient(transport: MockClientTransport.successful) + try await client.send( + input: "input", + forOperation: "op", + serializer: { input in + throw TestError() + }, + deserializer: { response, body in + fatalError() + } + ) + } catch { + let clientError = try XCTUnwrap(error as? ClientError) + XCTAssertEqual(clientError.operationID, "op") + XCTAssertEqual(clientError.operationInput as? String, "input") + XCTAssertEqual(clientError.causeDescription, "Unknown") + XCTAssertEqual(clientError.underlyingError as? TestError, TestError()) + XCTAssertNil(clientError.request) + XCTAssertNil(clientError.requestBody) + XCTAssertNil(clientError.baseURL) + XCTAssertNil(clientError.response) + XCTAssertNil(clientError.responseBody) + } + } + + func testErrorPropagation_transport() async throws { + do { + let client = UniversalClient(transport: MockClientTransport.failing) + try await client.send( + input: "input", + forOperation: "op", + serializer: { input in + ( + HTTPRequest(soar_path: "/", method: .post), + MockClientTransport.requestBody + ) + }, + deserializer: { response, body in + fatalError() + } + ) + } catch { + let clientError = try XCTUnwrap(error as? ClientError) + XCTAssertEqual(clientError.operationID, "op") + XCTAssertEqual(clientError.operationInput as? String, "input") + XCTAssertEqual(clientError.causeDescription, "Transport threw an error.") + XCTAssertEqual(clientError.underlyingError as? TestError, TestError()) + XCTAssertEqual(clientError.request, HTTPRequest(soar_path: "/", method: .post)) + XCTAssertEqual(clientError.requestBody, MockClientTransport.requestBody) + XCTAssertEqual(clientError.baseURL, URL(string: "/")) + XCTAssertNil(clientError.response) + XCTAssertNil(clientError.responseBody) + } + } + + func testErrorPropagation_deserializer() async throws { + do { + let client = UniversalClient(transport: MockClientTransport.successful) + try await client.send( + input: "input", + forOperation: "op", + serializer: { input in + ( + HTTPRequest(soar_path: "/", method: .post), + MockClientTransport.requestBody + ) + }, + deserializer: { response, body in + throw TestError() + } + ) + } catch { + let clientError = try XCTUnwrap(error as? ClientError) + XCTAssertEqual(clientError.operationID, "op") + XCTAssertEqual(clientError.operationInput as? String, "input") + XCTAssertEqual(clientError.causeDescription, "Unknown") + XCTAssertEqual(clientError.underlyingError as? TestError, TestError()) + XCTAssertEqual(clientError.request, HTTPRequest(soar_path: "/", method: .post)) + XCTAssertEqual(clientError.requestBody, MockClientTransport.requestBody) + XCTAssertEqual(clientError.baseURL, URL(string: "/")) + XCTAssertEqual(clientError.response, HTTPResponse(status: .ok)) + XCTAssertEqual(clientError.responseBody, MockClientTransport.responseBody) + } + } +} diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift index 61ef88f3..a6c2f02d 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift @@ -12,11 +12,136 @@ // //===----------------------------------------------------------------------===// import XCTest +import HTTPTypes +import Foundation @_spi(Generated) @testable import OpenAPIRuntime +struct MockHandler: Sendable { + var shouldFail: Bool = false + func greet(_ input: String) async throws -> String { + if shouldFail { + throw TestError() + } + guard input == "hello" else { + throw TestError() + } + return "bye" + } + + static let requestBody: HTTPBody = HTTPBody("hello") + static let responseBody: HTTPBody = HTTPBody("bye") +} + final class Test_UniversalServer: Test_Runtime { - struct MockHandler: Sendable {} + func testSuccess() async throws { + let server = UniversalServer(handler: MockHandler()) + let (response, responseBody) = try await server.handle( + request: .init(soar_path: "/", method: .post), + requestBody: .init("hello"), + metadata: .init(), + forOperation: "op", + using: { MockHandler.greet($0) }, + deserializer: { request, body, metadata in + let body = try XCTUnwrap(body) + return try await String(collecting: body, upTo: 10) + }, + serializer: { output, _ in + (HTTPResponse(status: .ok), MockHandler.responseBody) + } + ) + XCTAssertEqual(response, HTTPResponse(status: .ok)) + XCTAssertEqual(responseBody, MockHandler.responseBody) + } + + func testErrorPropagation_deserializer() async throws { + do { + let server = UniversalServer(handler: MockHandler()) + _ = try await server.handle( + request: .init(soar_path: "/", method: .post), + requestBody: MockHandler.requestBody, + metadata: .init(), + forOperation: "op", + using: { MockHandler.greet($0) }, + deserializer: { request, body, metadata in + throw TestError() + }, + serializer: { output, _ in + fatalError() + } + ) + } catch { + let serverError = try XCTUnwrap(error as? ServerError) + XCTAssertEqual(serverError.operationID, "op") + XCTAssertEqual(serverError.causeDescription, "Unknown") + XCTAssertEqual(serverError.underlyingError as? TestError, TestError()) + XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post)) + XCTAssertEqual(serverError.requestBody, MockHandler.requestBody) + XCTAssertEqual(serverError.requestMetadata, .init()) + XCTAssertNil(serverError.operationInput) + XCTAssertNil(serverError.operationOutput) + } + } + + func testErrorPropagation_transport() async throws { + do { + let server = UniversalServer(handler: MockHandler(shouldFail: true)) + _ = try await server.handle( + request: .init(soar_path: "/", method: .post), + requestBody: MockHandler.requestBody, + metadata: .init(), + forOperation: "op", + using: { MockHandler.greet($0) }, + deserializer: { request, body, metadata in + let body = try XCTUnwrap(body) + return try await String(collecting: body, upTo: 10) + }, + serializer: { output, _ in + fatalError() + } + ) + } catch { + let serverError = try XCTUnwrap(error as? ServerError) + XCTAssertEqual(serverError.operationID, "op") + XCTAssertEqual(serverError.causeDescription, "User handler threw an error.") + XCTAssertEqual(serverError.underlyingError as? TestError, TestError()) + XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post)) + XCTAssertEqual(serverError.requestBody, MockHandler.requestBody) + XCTAssertEqual(serverError.requestMetadata, .init()) + XCTAssertEqual(serverError.operationInput as? String, "hello") + XCTAssertNil(serverError.operationOutput) + } + } + + func testErrorPropagation_serializer() async throws { + do { + let server = UniversalServer(handler: MockHandler()) + _ = try await server.handle( + request: .init(soar_path: "/", method: .post), + requestBody: MockHandler.requestBody, + metadata: .init(), + forOperation: "op", + using: { MockHandler.greet($0) }, + deserializer: { request, body, metadata in + let body = try XCTUnwrap(body) + return try await String(collecting: body, upTo: 10) + }, + serializer: { output, _ in + throw TestError() + } + ) + } catch { + let serverError = try XCTUnwrap(error as? ServerError) + XCTAssertEqual(serverError.operationID, "op") + XCTAssertEqual(serverError.causeDescription, "Unknown") + XCTAssertEqual(serverError.underlyingError as? TestError, TestError()) + XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post)) + XCTAssertEqual(serverError.requestBody, MockHandler.requestBody) + XCTAssertEqual(serverError.requestMetadata, .init()) + XCTAssertEqual(serverError.operationInput as? String, "hello") + XCTAssertEqual(serverError.operationOutput as? String, "bye") + } + } func testApiPathComponentsWithServerPrefix_noPrefix() throws { let server = UniversalServer( diff --git a/Tests/OpenAPIRuntimeTests/Test_Runtime.swift b/Tests/OpenAPIRuntimeTests/Test_Runtime.swift index 4ab91a9f..f6a35835 100644 --- a/Tests/OpenAPIRuntimeTests/Test_Runtime.swift +++ b/Tests/OpenAPIRuntimeTests/Test_Runtime.swift @@ -155,6 +155,8 @@ class Test_Runtime: XCTestCase { } } +struct TestError: Error, Equatable {} + /// Asserts that a given URL's absolute string representation is equal to an expected string. /// /// - Parameters: From 640957a6bfa5dcbb0fb4a6fb2db17f49c915337e Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Wed, 25 Oct 2023 17:09:28 +0200 Subject: [PATCH 2/5] Start of work on improved middleware error propagation --- .../OpenAPIRuntime/Errors/RuntimeError.swift | 8 +- .../Interface/Test_UniversalClient.swift | 77 ++++++++++++++++++- .../Interface/Test_UniversalServer.swift | 34 ++++++++ Tests/OpenAPIRuntimeTests/Test_Runtime.swift | 43 +++++++++++ 4 files changed, 160 insertions(+), 2 deletions(-) diff --git a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift index 4c06048d..a102c3d4 100644 --- a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift +++ b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift @@ -54,6 +54,7 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret // Transport/Handler case transportFailed(any Error) + case middlewareFailed(middlewareType: Any.Type, any Error) case handlerFailed(any Error) // Unexpected response (thrown by shorthand APIs) @@ -63,7 +64,10 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret /// A wrapped root cause error, if one was thrown by other code. var underlyingError: (any Error)? { switch self { - case .transportFailed(let error), .handlerFailed(let error): + case + .transportFailed(let error), + .handlerFailed(let error), + .middlewareFailed(_, let error): return error default: return nil @@ -111,6 +115,8 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret return "Missing required response body" case .transportFailed: return "Transport threw an error." + case .middlewareFailed(middlewareType: let type, _): + return "Middleware of type '\(type)' threw an error." case .handlerFailed: return "User handler threw an error." case .unexpectedResponseStatus(let expectedStatus, let response): diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift index b5fd056d..1fd294f5 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift @@ -91,10 +91,50 @@ final class Test_UniversalClient: Test_Runtime { XCTAssertNil(clientError.responseBody) } } + + func testErrorPropagation_middlewareOnRequest() async throws { + do { + let client = UniversalClient( + transport: MockClientTransport.successful, + middlewares: [ + MockMiddleware(failurePhase: .onRequest) + ] + ) + try await client.send( + input: "input", + forOperation: "op", + serializer: { input in + ( + HTTPRequest(soar_path: "/", method: .post), + MockClientTransport.requestBody + ) + }, + deserializer: { response, body in + fatalError() + } + ) + } catch { + let clientError = try XCTUnwrap(error as? ClientError) + XCTAssertEqual(clientError.operationID, "op") + XCTAssertEqual(clientError.operationInput as? String, "input") + XCTAssertEqual(clientError.causeDescription, "Middleware of type 'MockMiddleware' threw an error.") + XCTAssertEqual(clientError.underlyingError as? TestError, TestError()) + XCTAssertEqual(clientError.request, HTTPRequest(soar_path: "/", method: .post)) + XCTAssertEqual(clientError.requestBody, MockClientTransport.requestBody) + XCTAssertEqual(clientError.baseURL, URL(string: "/")) + XCTAssertNil(clientError.response) + XCTAssertNil(clientError.responseBody) + } + } func testErrorPropagation_transport() async throws { do { - let client = UniversalClient(transport: MockClientTransport.failing) + let client = UniversalClient( + transport: MockClientTransport.failing, + middlewares: [ + MockMiddleware() + ] + ) try await client.send( input: "input", forOperation: "op", @@ -122,6 +162,41 @@ final class Test_UniversalClient: Test_Runtime { } } + func testErrorPropagation_middlewareOnResponse() async throws { + do { + let client = UniversalClient( + transport: MockClientTransport.successful, + middlewares: [ + MockMiddleware(failurePhase: .onResponse) + ] + ) + try await client.send( + input: "input", + forOperation: "op", + serializer: { input in + ( + HTTPRequest(soar_path: "/", method: .post), + MockClientTransport.requestBody + ) + }, + deserializer: { response, body in + fatalError() + } + ) + } catch { + let clientError = try XCTUnwrap(error as? ClientError) + XCTAssertEqual(clientError.operationID, "op") + XCTAssertEqual(clientError.operationInput as? String, "input") + XCTAssertEqual(clientError.causeDescription, "Middleware of type 'MockMiddleware' threw an error.") + XCTAssertEqual(clientError.underlyingError as? TestError, TestError()) + XCTAssertEqual(clientError.request, HTTPRequest(soar_path: "/", method: .post)) + XCTAssertEqual(clientError.requestBody, MockClientTransport.requestBody) + XCTAssertEqual(clientError.baseURL, URL(string: "/")) + XCTAssertEqual(clientError.response, HTTPResponse(status: .ok)) + XCTAssertEqual(clientError.responseBody, MockClientTransport.responseBody) + } + } + func testErrorPropagation_deserializer() async throws { do { let client = UniversalClient(transport: MockClientTransport.successful) diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift index a6c2f02d..8fe765cb 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift @@ -54,6 +54,40 @@ final class Test_UniversalServer: Test_Runtime { XCTAssertEqual(responseBody, MockHandler.responseBody) } + func testErrorPropagation_middlewareOnRequest() async throws { + do { + let server = UniversalServer( + handler: MockHandler(), + middlewares: [ + MockMiddleware(failurePhase: .onRequest) + ] + ) + _ = try await server.handle( + request: .init(soar_path: "/", method: .post), + requestBody: MockHandler.requestBody, + metadata: .init(), + forOperation: "op", + using: { MockHandler.greet($0) }, + deserializer: { request, body, metadata in + fatalError() + }, + serializer: { output, _ in + fatalError() + } + ) + } catch { + let serverError = try XCTUnwrap(error as? ServerError) + XCTAssertEqual(serverError.operationID, "op") + XCTAssertEqual(serverError.causeDescription, "Unknown") + XCTAssertEqual(serverError.underlyingError as? TestError, TestError()) + XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post)) + XCTAssertEqual(serverError.requestBody, MockHandler.requestBody) + XCTAssertEqual(serverError.requestMetadata, .init()) + XCTAssertNil(serverError.operationInput) + XCTAssertNil(serverError.operationOutput) + } + } + func testErrorPropagation_deserializer() async throws { do { let server = UniversalServer(handler: MockHandler()) diff --git a/Tests/OpenAPIRuntimeTests/Test_Runtime.swift b/Tests/OpenAPIRuntimeTests/Test_Runtime.swift index f6a35835..7d09a520 100644 --- a/Tests/OpenAPIRuntimeTests/Test_Runtime.swift +++ b/Tests/OpenAPIRuntimeTests/Test_Runtime.swift @@ -157,6 +157,49 @@ class Test_Runtime: XCTestCase { struct TestError: Error, Equatable {} +struct MockMiddleware: ClientMiddleware, ServerMiddleware { + enum FailurePhase { + case never + case onRequest + case onResponse + } + var failurePhase: FailurePhase = .never + + func intercept( + _ request: HTTPRequest, + body: HTTPBody?, + baseURL: URL, + operationID: String, + next: (HTTPRequest, HTTPBody?, URL) async throws -> (HTTPResponse, HTTPBody?) + ) async throws -> (HTTPResponse, HTTPBody?) { + if failurePhase == .onRequest { + throw TestError() + } + let (response, responseBody) = try await next(request, body, baseURL) + if failurePhase == .onResponse { + throw TestError() + } + return (response, responseBody) + } + + func intercept( + _ request: HTTPRequest, + body: HTTPBody?, + metadata: ServerRequestMetadata, + operationID: String, + next: (HTTPRequest, HTTPBody?, ServerRequestMetadata) async throws -> (HTTPResponse, HTTPBody?) + ) async throws -> (HTTPResponse, HTTPBody?) { + if failurePhase == .onRequest { + throw TestError() + } + let (response, responseBody) = try await next(request, body, metadata) + if failurePhase == .onResponse { + throw TestError() + } + return (response, responseBody) + } +} + /// Asserts that a given URL's absolute string representation is equal to an expected string. /// /// - Parameters: From 9068ec813a22b8b80879cd0ac3bc97c3c00b96e9 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Wed, 25 Oct 2023 17:38:47 +0200 Subject: [PATCH 3/5] Middleware errors are now also enriched --- .../OpenAPIRuntime/Errors/RuntimeError.swift | 7 +- .../Interface/UniversalClient.swift | 67 +++++++++++++------ .../Interface/UniversalServer.swift | 44 ++++++++---- .../Interface/Test_UniversalClient.swift | 6 +- .../Interface/Test_UniversalServer.swift | 39 ++++++++++- Tests/OpenAPIRuntimeTests/Test_Runtime.swift | 4 +- 6 files changed, 122 insertions(+), 45 deletions(-) diff --git a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift index a102c3d4..09deae70 100644 --- a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift +++ b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift @@ -64,10 +64,9 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret /// A wrapped root cause error, if one was thrown by other code. var underlyingError: (any Error)? { switch self { - case - .transportFailed(let error), - .handlerFailed(let error), - .middlewareFailed(_, let error): + case .transportFailed(let error), + .handlerFailed(let error), + .middlewareFailed(_, let error): return error default: return nil diff --git a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift index 8c6a05ae..6c75a3ce 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift @@ -90,19 +90,20 @@ import Foundation serializer: @Sendable (OperationInput) throws -> (HTTPRequest, HTTPBody?), deserializer: @Sendable (HTTPResponse, HTTPBody?) async throws -> OperationOutput ) async throws -> OperationOutput where OperationInput: Sendable, OperationOutput: Sendable { - @Sendable - func wrappingErrors( + @Sendable func wrappingErrors( work: () async throws -> R, mapError: (any Error) -> any Error ) async throws -> R { do { return try await work() + } catch let error as ClientError { + throw error } catch { throw mapError(error) } } let baseURL = serverURL - func makeError( + @Sendable func makeError( request: HTTPRequest? = nil, requestBody: HTTPBody? = nil, baseURL: URL? = nil, @@ -110,6 +111,14 @@ import Foundation responseBody: HTTPBody? = nil, error: any Error ) -> any Error { + if var error = error as? ClientError { + error.request = error.request ?? request + error.requestBody = error.requestBody ?? requestBody + error.baseURL = error.baseURL ?? baseURL + error.response = error.response ?? response + error.responseBody = error.responseBody ?? responseBody + return error + } let causeDescription: String let underlyingError: any Error if let runtimeError = error as? RuntimeError { @@ -136,36 +145,50 @@ import Foundation } mapError: { error in makeError(error: error) } - let (response, responseBody): (HTTPResponse, HTTPBody?) = try await wrappingErrors { - var next: @Sendable (HTTPRequest, HTTPBody?, URL) async throws -> (HTTPResponse, HTTPBody?) = { + var next: @Sendable (HTTPRequest, HTTPBody?, URL) async throws -> (HTTPResponse, HTTPBody?) = { + (_request, _body, _url) in + try await wrappingErrors { + try await transport.send( + _request, + body: _body, + baseURL: _url, + operationID: operationID + ) + } mapError: { error in + makeError( + request: request, + requestBody: requestBody, + baseURL: baseURL, + error: RuntimeError.transportFailed(error) + ) + } + } + for middleware in middlewares.reversed() { + let tmp = next + next = { (_request, _body, _url) in try await wrappingErrors { - try await transport.send( + try await middleware.intercept( _request, body: _body, baseURL: _url, - operationID: operationID - ) - } mapError: { error in - RuntimeError.transportFailed(error) - } - } - for middleware in middlewares.reversed() { - let tmp = next - next = { - try await middleware.intercept( - $0, - body: $1, - baseURL: $2, operationID: operationID, next: tmp ) + } mapError: { error in + makeError( + request: request, + requestBody: requestBody, + baseURL: baseURL, + error: RuntimeError.middlewareFailed( + middlewareType: type(of: middleware), + error + ) + ) } } - return try await next(request, requestBody, baseURL) - } mapError: { error in - makeError(request: request, requestBody: requestBody, baseURL: baseURL, error: error) } + let (response, responseBody): (HTTPResponse, HTTPBody?) = try await next(request, requestBody, baseURL) return try await wrappingErrors { try await deserializer(response, responseBody) } mapError: { error in diff --git a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift index c14dea72..e523560f 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift @@ -102,23 +102,28 @@ import struct Foundation.URLComponents OperationInput, serializer: @Sendable @escaping (OperationOutput, HTTPRequest) throws -> (HTTPResponse, HTTPBody?) ) async throws -> (HTTPResponse, HTTPBody?) where OperationInput: Sendable, OperationOutput: Sendable { - @Sendable - func wrappingErrors( + @Sendable func wrappingErrors( work: () async throws -> R, mapError: (any Error) -> any Error ) async throws -> R { do { return try await work() + } catch let error as ServerError { + throw error } catch { throw mapError(error) } } - @Sendable - func makeError( + @Sendable func makeError( input: OperationInput? = nil, output: OperationOutput? = nil, error: any Error ) -> any Error { + if var error = error as? ServerError { + error.operationInput = error.operationInput ?? input + error.operationOutput = error.operationOutput ?? output + return error + } let causeDescription: String let underlyingError: any Error if let runtimeError = error as? RuntimeError { @@ -154,7 +159,10 @@ import struct Foundation.URLComponents return try await wrappingErrors { try await method(input) } mapError: { error in - RuntimeError.handlerFailed(error) + makeError( + input: input, + error: RuntimeError.handlerFailed(error) + ) } } mapError: { error in makeError(input: input, error: error) @@ -168,13 +176,25 @@ import struct Foundation.URLComponents for middleware in middlewares.reversed() { let tmp = next next = { - try await middleware.intercept( - $0, - body: $1, - metadata: $2, - operationID: operationID, - next: tmp - ) + _request, + _requestBody, + _metadata in + try await wrappingErrors { + try await middleware.intercept( + _request, + body: _requestBody, + metadata: _metadata, + operationID: operationID, + next: tmp + ) + } mapError: { error in + makeError( + error: RuntimeError.middlewareFailed( + middlewareType: type(of: middleware), + error + ) + ) + } } } return try await next(request, requestBody, metadata) diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift index 1fd294f5..64e38c86 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift @@ -91,7 +91,7 @@ final class Test_UniversalClient: Test_Runtime { XCTAssertNil(clientError.responseBody) } } - + func testErrorPropagation_middlewareOnRequest() async throws { do { let client = UniversalClient( @@ -192,8 +192,8 @@ final class Test_UniversalClient: Test_Runtime { XCTAssertEqual(clientError.request, HTTPRequest(soar_path: "/", method: .post)) XCTAssertEqual(clientError.requestBody, MockClientTransport.requestBody) XCTAssertEqual(clientError.baseURL, URL(string: "/")) - XCTAssertEqual(clientError.response, HTTPResponse(status: .ok)) - XCTAssertEqual(clientError.responseBody, MockClientTransport.responseBody) + XCTAssertNil(clientError.response) + XCTAssertNil(clientError.responseBody) } } diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift index 8fe765cb..88b2ae96 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift @@ -78,7 +78,7 @@ final class Test_UniversalServer: Test_Runtime { } catch { let serverError = try XCTUnwrap(error as? ServerError) XCTAssertEqual(serverError.operationID, "op") - XCTAssertEqual(serverError.causeDescription, "Unknown") + XCTAssertEqual(serverError.causeDescription, "Middleware of type 'MockMiddleware' threw an error.") XCTAssertEqual(serverError.underlyingError as? TestError, TestError()) XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post)) XCTAssertEqual(serverError.requestBody, MockHandler.requestBody) @@ -117,7 +117,7 @@ final class Test_UniversalServer: Test_Runtime { } } - func testErrorPropagation_transport() async throws { + func testErrorPropagation_handler() async throws { do { let server = UniversalServer(handler: MockHandler(shouldFail: true)) _ = try await server.handle( @@ -177,6 +177,41 @@ final class Test_UniversalServer: Test_Runtime { } } + func testErrorPropagation_middlewareOnResponse() async throws { + do { + let server = UniversalServer( + handler: MockHandler(), + middlewares: [ + MockMiddleware(failurePhase: .onResponse) + ] + ) + _ = try await server.handle( + request: .init(soar_path: "/", method: .post), + requestBody: MockHandler.requestBody, + metadata: .init(), + forOperation: "op", + using: { MockHandler.greet($0) }, + deserializer: { request, body, metadata in + let body = try XCTUnwrap(body) + return try await String(collecting: body, upTo: 10) + }, + serializer: { output, _ in + (HTTPResponse(status: .ok), MockHandler.responseBody) + } + ) + } catch { + let serverError = try XCTUnwrap(error as? ServerError) + XCTAssertEqual(serverError.operationID, "op") + XCTAssertEqual(serverError.causeDescription, "Middleware of type 'MockMiddleware' threw an error.") + XCTAssertEqual(serverError.underlyingError as? TestError, TestError()) + XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post)) + XCTAssertEqual(serverError.requestBody, MockHandler.requestBody) + XCTAssertEqual(serverError.requestMetadata, .init()) + XCTAssertNil(serverError.operationInput) + XCTAssertNil(serverError.operationOutput) + } + } + func testApiPathComponentsWithServerPrefix_noPrefix() throws { let server = UniversalServer( handler: MockHandler() diff --git a/Tests/OpenAPIRuntimeTests/Test_Runtime.swift b/Tests/OpenAPIRuntimeTests/Test_Runtime.swift index 7d09a520..704b1ef6 100644 --- a/Tests/OpenAPIRuntimeTests/Test_Runtime.swift +++ b/Tests/OpenAPIRuntimeTests/Test_Runtime.swift @@ -164,7 +164,7 @@ struct MockMiddleware: ClientMiddleware, ServerMiddleware { case onResponse } var failurePhase: FailurePhase = .never - + func intercept( _ request: HTTPRequest, body: HTTPBody?, @@ -181,7 +181,7 @@ struct MockMiddleware: ClientMiddleware, ServerMiddleware { } return (response, responseBody) } - + func intercept( _ request: HTTPRequest, body: HTTPBody?, From 820f26074edb36ed35e54cca0fd5d08cff731de1 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Thu, 26 Oct 2023 09:32:27 +0200 Subject: [PATCH 4/5] Add shims for the older initializer for backwards compatibility --- .../Deprecated/Deprecated.swift | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift index cb20c7e6..31fc7207 100644 --- a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift +++ b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift @@ -12,5 +12,91 @@ // //===----------------------------------------------------------------------===// import Foundation +import HTTPTypes // MARK: - Functionality to be removed in the future + +extension ClientError { + /// Creates a new error. + /// - Parameters: + /// - operationID: The OpenAPI operation identifier. + /// - operationInput: The operation-specific Input value. + /// - request: The HTTP request created during the operation. + /// - requestBody: The HTTP request body created during the operation. + /// - baseURL: The base URL for HTTP requests. + /// - response: The HTTP response received during the operation. + /// - responseBody: The HTTP response body received during the operation. + /// - underlyingError: The underlying error that caused the operation + /// to fail. + @available( + *, + deprecated, + renamed: + "ClientError.init(operationID:operationInput:request:requestBody:baseURL:response:responseBody:causeDescription:underlyingError:)", + message: "Use the initializer with a causeDescription parameter." + ) + public init( + operationID: String, + operationInput: any Sendable, + request: HTTPRequest? = nil, + requestBody: HTTPBody? = nil, + baseURL: URL? = nil, + response: HTTPResponse? = nil, + responseBody: HTTPBody? = nil, + underlyingError: any Error + ) { + self.init( + operationID: operationID, + operationInput: operationInput, + request: request, + requestBody: requestBody, + baseURL: baseURL, + response: response, + responseBody: responseBody, + causeDescription: "Legacy error without a causeDescription.", + underlyingError: underlyingError + ) + } +} + +extension ServerError { + /// Creates a new error. + /// - Parameters: + /// - operationID: The OpenAPI operation identifier. + /// - request: The HTTP request provided to the server. + /// - requestBody: The HTTP request body provided to the server. + /// - requestMetadata: The request metadata extracted by the server. + /// - operationInput: An operation-specific Input value. + /// - operationOutput: An operation-specific Output value. + /// - causeDescription: A user-facing description of what caused + /// the underlying error to be thrown. + /// - underlyingError: The underlying error that caused the operation + /// to fail. + @available( + *, + deprecated, + renamed: + "ServerError.init(operationID:request:requestBody:requestMetadata:operationInput:operationOutput:causeDescription:underlyingError:)", + message: "Use the initializer with a causeDescription parameter." + ) + public init( + operationID: String, + request: HTTPRequest, + requestBody: HTTPBody?, + requestMetadata: ServerRequestMetadata, + operationInput: (any Sendable)? = nil, + operationOutput: (any Sendable)? = nil, + underlyingError: any Error + ) { + self.init( + operationID: operationID, + request: request, + requestBody: requestBody, + requestMetadata: requestMetadata, + operationInput: operationInput, + operationOutput: operationOutput, + causeDescription: "Legacy error without a causeDescription.", + underlyingError: underlyingError + ) + } +} From 64307cc19f0c3bd3c21a2e1351ab96d94b99ca7f Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Thu, 26 Oct 2023 09:37:54 +0200 Subject: [PATCH 5/5] Fix a docs warning --- Sources/OpenAPIRuntime/Deprecated/Deprecated.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift index 31fc7207..bf454112 100644 --- a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift +++ b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift @@ -68,8 +68,6 @@ extension ServerError { /// - requestMetadata: The request metadata extracted by the server. /// - operationInput: An operation-specific Input value. /// - operationOutput: An operation-specific Output value. - /// - causeDescription: A user-facing description of what caused - /// the underlying error to be thrown. /// - underlyingError: The underlying error that caused the operation /// to fail. @available(