From 13fbbc892197f7841f430454d924858a6f5725b5 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Sat, 8 Feb 2025 13:00:31 +0100 Subject: [PATCH 1/4] [Experiment] Client error mapper --- .../Interface/UniversalClient.swift | 101 ++++++++++-------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift index 5afff2b1..0e962ddd 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift @@ -38,17 +38,22 @@ import struct Foundation.URL /// The middlewares to be invoked before the transport. public var middlewares: [any ClientMiddleware] + /// An error mapping closure to allow customizing the error thrown by the client. + public var errorMapper: (@Sendable (ClientError) -> any Error)? + /// Internal initializer that takes an initialized `Converter`. internal init( serverURL: URL, converter: Converter, transport: any ClientTransport, - middlewares: [any ClientMiddleware] + middlewares: [any ClientMiddleware], + errorMapper: (@Sendable (ClientError) -> any Error)? ) { self.serverURL = serverURL self.converter = converter self.transport = transport self.middlewares = middlewares + self.errorMapper = errorMapper } /// Creates a new client. @@ -56,13 +61,15 @@ import struct Foundation.URL serverURL: URL = .defaultOpenAPIServerURL, configuration: Configuration = .init(), transport: any ClientTransport, - middlewares: [any ClientMiddleware] = [] + middlewares: [any ClientMiddleware] = [], + errorMapper: (@Sendable (ClientError) -> any Error)? = nil ) { self.init( serverURL: serverURL, converter: Converter(configuration: configuration), transport: transport, - middlewares: middlewares + middlewares: middlewares, + errorMapper: errorMapper ) } @@ -135,57 +142,65 @@ import struct Foundation.URL underlyingError: underlyingError ) } - let (request, requestBody): (HTTPRequest, HTTPBody?) = try await wrappingErrors { - try serializer(input) - } mapError: { error in - makeError(error: error) - } - 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) + do { + let (request, requestBody): (HTTPRequest, HTTPBody?) = try await wrappingErrors { + try serializer(input) } mapError: { error in - makeError( - request: request, - requestBody: requestBody, - baseURL: baseURL, - error: RuntimeError.transportFailed(error) - ) + makeError(error: error) } - } - for middleware in middlewares.reversed() { - let tmp = next - next = { (_request, _body, _url) in + var next: @Sendable (HTTPRequest, HTTPBody?, URL) async throws -> (HTTPResponse, HTTPBody?) = { + (_request, _body, _url) in try await wrappingErrors { - try await middleware.intercept( - _request, - body: _body, - baseURL: _url, - operationID: operationID, - next: tmp - ) + try await transport.send(_request, body: _body, baseURL: _url, operationID: operationID) } mapError: { error in makeError( request: request, requestBody: requestBody, baseURL: baseURL, - error: RuntimeError.middlewareFailed(middlewareType: type(of: middleware), error) + error: RuntimeError.transportFailed(error) ) } } - } - let (response, responseBody): (HTTPResponse, HTTPBody?) = try await next(request, requestBody, baseURL) - return try await wrappingErrors { - try await deserializer(response, responseBody) - } mapError: { error in - makeError( - request: request, - requestBody: requestBody, - baseURL: baseURL, - response: response, - responseBody: responseBody, - error: error - ) + for middleware in middlewares.reversed() { + let tmp = next + next = { (_request, _body, _url) in + try await wrappingErrors { + try await middleware.intercept( + _request, + body: _body, + baseURL: _url, + operationID: operationID, + next: tmp + ) + } mapError: { error in + makeError( + request: request, + requestBody: requestBody, + baseURL: baseURL, + error: RuntimeError.middlewareFailed(middlewareType: type(of: middleware), error) + ) + } + } + } + let (response, responseBody): (HTTPResponse, HTTPBody?) = try await next(request, requestBody, baseURL) + return try await wrappingErrors { + try await deserializer(response, responseBody) + } mapError: { error in + makeError( + request: request, + requestBody: requestBody, + baseURL: baseURL, + response: response, + responseBody: responseBody, + error: error + ) + } + } catch { + if let errorMapper, let clientError = error as? ClientError { + throw errorMapper(clientError) + } else { + throw error + } } } } From 73ea1d913a4a347af28e4ee786de54355d225c3b Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Sat, 8 Feb 2025 13:07:02 +0100 Subject: [PATCH 2/4] Add a test --- .../Interface/Test_UniversalClient.swift | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift index b0063e70..24364029 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift @@ -119,6 +119,26 @@ final class Test_UniversalClient: Test_Runtime { } } + func testErrorPropagation_customErrorMapper() async throws { + do { + let client = UniversalClient( + transport: MockClientTransport.failing, + errorMapper: { clientError in + // Don't care about the extra context, just wants the underlyingError + clientError.underlyingError + } + ) + try await client.send( + input: "input", + forOperation: "op", + serializer: { input in (HTTPRequest(soar_path: "/", method: .post), MockClientTransport.requestBody) }, + deserializer: { response, body in fatalError() } + ) + } catch { + XCTAssertTrue(error is TestError, "Threw an unexpected error: \(type(of: error))") + } + } + func testErrorPropagation_middlewareOnResponse() async throws { do { let client = UniversalClient( From 78bd078ab86080088dddb245b38f28dc6903462f Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Sat, 8 Feb 2025 13:21:53 +0100 Subject: [PATCH 3/4] Extend the customization to both client and server --- .../Conversion/Configuration.swift | 13 ++++++++- .../Deprecated/Deprecated.swift | 25 +++++++++++++++++ .../Interface/UniversalClient.swift | 5 ++-- .../Interface/UniversalServer.swift | 25 +++++++++++++++-- .../Interface/Test_UniversalClient.swift | 12 ++++---- .../Interface/Test_UniversalServer.swift | 28 +++++++++++++++++++ 6 files changed, 96 insertions(+), 12 deletions(-) diff --git a/Sources/OpenAPIRuntime/Conversion/Configuration.swift b/Sources/OpenAPIRuntime/Conversion/Configuration.swift index 2ee7ab00..6de3adaf 100644 --- a/Sources/OpenAPIRuntime/Conversion/Configuration.swift +++ b/Sources/OpenAPIRuntime/Conversion/Configuration.swift @@ -152,6 +152,12 @@ public struct Configuration: Sendable { /// Custom XML coder for encoding and decoding xml bodies. public var xmlCoder: (any CustomCoder)? + /// An error mapping closure to allow customizing the error thrown by the client. + public var clientErrorMapper: (@Sendable (ClientError) -> any Error)? + + /// An error mapping closure to allow customizing the error thrown by the server. + public var serverErrorMapper: (@Sendable (ServerError) -> any Error)? + /// Creates a new configuration with the specified values. /// /// - Parameters: @@ -160,15 +166,20 @@ public struct Configuration: Sendable { /// - jsonEncodingOptions: The options for the underlying JSON encoder. /// - multipartBoundaryGenerator: The generator to use when creating mutlipart bodies. /// - xmlCoder: Custom XML coder for encoding and decoding xml bodies. Only required when using XML body payloads. + /// - errorMapper: An error mapping closure to allow customizing the final error thrown. public init( dateTranscoder: any DateTranscoder = .iso8601, jsonEncodingOptions: JSONEncodingOptions = [.sortedKeys, .prettyPrinted], multipartBoundaryGenerator: any MultipartBoundaryGenerator = .random, - xmlCoder: (any CustomCoder)? = nil + xmlCoder: (any CustomCoder)? = nil, + clientErrorMapper: (@Sendable (ClientError) -> any Error)? = nil, + serverErrorMapper: (@Sendable (ServerError) -> any Error)? = nil ) { self.dateTranscoder = dateTranscoder self.jsonEncodingOptions = jsonEncodingOptions self.multipartBoundaryGenerator = multipartBoundaryGenerator self.xmlCoder = xmlCoder + self.clientErrorMapper = clientErrorMapper + self.serverErrorMapper = serverErrorMapper } } diff --git a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift index 2ce41750..41d797c5 100644 --- a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift +++ b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift @@ -58,6 +58,31 @@ extension Configuration { xmlCoder: xmlCoder ) } + + /// Creates a new configuration with the specified values. + /// + /// - Parameters: + /// - dateTranscoder: The transcoder to use when converting between date + /// and string values. + /// - jsonEncodingOptions: The options for the underlying JSON encoder. + /// - multipartBoundaryGenerator: The generator to use when creating mutlipart bodies. + /// - xmlCoder: Custom XML coder for encoding and decoding xml bodies. Only required when using XML body payloads. + @available(*, deprecated, renamed: "init(dateTranscoder:jsonEncodingOptions:multipartBoundaryGenerator:xmlCoder:clientErrorMapper:serverErrorMapper:)") + @_disfavoredOverload public init( + dateTranscoder: any DateTranscoder = .iso8601, + jsonEncodingOptions: JSONEncodingOptions = [.sortedKeys, .prettyPrinted], + multipartBoundaryGenerator: any MultipartBoundaryGenerator = .random, + xmlCoder: (any CustomCoder)? = nil + ) { + self.init( + dateTranscoder: dateTranscoder, + jsonEncodingOptions: [.sortedKeys, .prettyPrinted], + multipartBoundaryGenerator: multipartBoundaryGenerator, + xmlCoder: xmlCoder, + clientErrorMapper: nil, + serverErrorMapper: nil + ) + } } extension AsyncSequence where Element == ArraySlice, Self: Sendable { diff --git a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift index 0e962ddd..3a2bf2e0 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalClient.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalClient.swift @@ -61,15 +61,14 @@ import struct Foundation.URL serverURL: URL = .defaultOpenAPIServerURL, configuration: Configuration = .init(), transport: any ClientTransport, - middlewares: [any ClientMiddleware] = [], - errorMapper: (@Sendable (ClientError) -> any Error)? = nil + middlewares: [any ClientMiddleware] = [] ) { self.init( serverURL: serverURL, converter: Converter(configuration: configuration), transport: transport, middlewares: middlewares, - errorMapper: errorMapper + errorMapper: configuration.clientErrorMapper ) } diff --git a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift index 4608dafe..2b3906c1 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift @@ -40,12 +40,22 @@ import struct Foundation.URLComponents /// The middlewares to be invoked before the handler receives the request. public var middlewares: [any ServerMiddleware] + /// An error mapping closure to allow customizing the error thrown by the server handler. + public var errorMapper: (@Sendable (ServerError) -> any Error)? + /// Internal initializer that takes an initialized converter. - internal init(serverURL: URL, converter: Converter, handler: APIHandler, middlewares: [any ServerMiddleware]) { + internal init( + serverURL: URL, + converter: Converter, + handler: APIHandler, + middlewares: [any ServerMiddleware], + errorMapper: (@Sendable (ServerError) -> any Error)? + ) { self.serverURL = serverURL self.converter = converter self.handler = handler self.middlewares = middlewares + self.errorMapper = errorMapper } /// Creates a new server with the specified parameters. @@ -59,7 +69,8 @@ import struct Foundation.URLComponents serverURL: serverURL, converter: Converter(configuration: configuration), handler: handler, - middlewares: middlewares + middlewares: middlewares, + errorMapper: configuration.serverErrorMapper ) } @@ -169,7 +180,15 @@ import struct Foundation.URLComponents } } } - return try await next(request, requestBody, metadata) + do { + return try await next(request, requestBody, metadata) + } catch { + if let errorMapper, let serverError = error as? ServerError { + throw errorMapper(serverError) + } else { + throw error + } + } } /// Returns the path with the server URL's path prefix prepended. diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift index 24364029..b7ddfa56 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift @@ -122,11 +122,13 @@ final class Test_UniversalClient: Test_Runtime { func testErrorPropagation_customErrorMapper() async throws { do { let client = UniversalClient( - transport: MockClientTransport.failing, - errorMapper: { clientError in - // Don't care about the extra context, just wants the underlyingError - clientError.underlyingError - } + configuration: .init( + clientErrorMapper: { clientError in + // Don't care about the extra context, just wants the underlyingError + clientError.underlyingError + } + ), + transport: MockClientTransport.failing ) try await client.send( input: "input", diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift index e65afe4f..ed9af63f 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift @@ -129,6 +129,34 @@ final class Test_UniversalServer: Test_Runtime { } } + func testErrorPropagation_errorMapper() async throws { + do { + let server = UniversalServer( + handler: MockHandler(shouldFail: true), + configuration: .init( + serverErrorMapper: { serverError in + // Don't care about the extra context, just wants the underlyingError + serverError.underlyingError + } + ) + ) + _ = 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 { + XCTAssertTrue(error is TestError, "Threw an unexpected error: \(type(of: error))") + } + } + func testErrorPropagation_serializer() async throws { do { let server = UniversalServer(handler: MockHandler()) From 47a89bc77675ec3d554714cbee136c3d2a3b3764 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Sun, 9 Feb 2025 12:32:05 +0100 Subject: [PATCH 4/4] Soundness fixes --- .../OpenAPIRuntime/Conversion/Configuration.swift | 3 ++- Sources/OpenAPIRuntime/Deprecated/Deprecated.swift | 8 ++++++-- .../OpenAPIRuntime/Interface/UniversalServer.swift | 4 +--- .../Interface/Test_UniversalClient.swift | 14 +++++--------- .../Interface/Test_UniversalServer.swift | 14 +++++--------- 5 files changed, 19 insertions(+), 24 deletions(-) diff --git a/Sources/OpenAPIRuntime/Conversion/Configuration.swift b/Sources/OpenAPIRuntime/Conversion/Configuration.swift index 6de3adaf..fd3a2048 100644 --- a/Sources/OpenAPIRuntime/Conversion/Configuration.swift +++ b/Sources/OpenAPIRuntime/Conversion/Configuration.swift @@ -166,7 +166,8 @@ public struct Configuration: Sendable { /// - jsonEncodingOptions: The options for the underlying JSON encoder. /// - multipartBoundaryGenerator: The generator to use when creating mutlipart bodies. /// - xmlCoder: Custom XML coder for encoding and decoding xml bodies. Only required when using XML body payloads. - /// - errorMapper: An error mapping closure to allow customizing the final error thrown. + /// - clientErrorMapper: An error mapping closure to allow customizing the error thrown by the client. + /// - serverErrorMapper: An error mapping closure to allow customizing the error thrown by the server. public init( dateTranscoder: any DateTranscoder = .iso8601, jsonEncodingOptions: JSONEncodingOptions = [.sortedKeys, .prettyPrinted], diff --git a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift index 41d797c5..8b5d19ee 100644 --- a/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift +++ b/Sources/OpenAPIRuntime/Deprecated/Deprecated.swift @@ -67,8 +67,12 @@ extension Configuration { /// - jsonEncodingOptions: The options for the underlying JSON encoder. /// - multipartBoundaryGenerator: The generator to use when creating mutlipart bodies. /// - xmlCoder: Custom XML coder for encoding and decoding xml bodies. Only required when using XML body payloads. - @available(*, deprecated, renamed: "init(dateTranscoder:jsonEncodingOptions:multipartBoundaryGenerator:xmlCoder:clientErrorMapper:serverErrorMapper:)") - @_disfavoredOverload public init( + @available( + *, + deprecated, + renamed: + "init(dateTranscoder:jsonEncodingOptions:multipartBoundaryGenerator:xmlCoder:clientErrorMapper:serverErrorMapper:)" + ) @_disfavoredOverload public init( dateTranscoder: any DateTranscoder = .iso8601, jsonEncodingOptions: JSONEncodingOptions = [.sortedKeys, .prettyPrinted], multipartBoundaryGenerator: any MultipartBoundaryGenerator = .random, diff --git a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift index 2b3906c1..866f05b6 100644 --- a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift +++ b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift @@ -180,9 +180,7 @@ import struct Foundation.URLComponents } } } - do { - return try await next(request, requestBody, metadata) - } catch { + do { return try await next(request, requestBody, metadata) } catch { if let errorMapper, let serverError = error as? ServerError { throw errorMapper(serverError) } else { diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift index b7ddfa56..2e3d3be2 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalClient.swift @@ -122,12 +122,10 @@ final class Test_UniversalClient: Test_Runtime { func testErrorPropagation_customErrorMapper() async throws { do { let client = UniversalClient( - configuration: .init( - clientErrorMapper: { clientError in - // Don't care about the extra context, just wants the underlyingError - clientError.underlyingError - } - ), + configuration: .init(clientErrorMapper: { clientError in + // Don't care about the extra context, just wants the underlyingError + clientError.underlyingError + }), transport: MockClientTransport.failing ) try await client.send( @@ -136,9 +134,7 @@ final class Test_UniversalClient: Test_Runtime { serializer: { input in (HTTPRequest(soar_path: "/", method: .post), MockClientTransport.requestBody) }, deserializer: { response, body in fatalError() } ) - } catch { - XCTAssertTrue(error is TestError, "Threw an unexpected error: \(type(of: error))") - } + } catch { XCTAssertTrue(error is TestError, "Threw an unexpected error: \(type(of: error))") } } func testErrorPropagation_middlewareOnResponse() async throws { diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift index ed9af63f..6a9fe816 100644 --- a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift +++ b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift @@ -133,12 +133,10 @@ final class Test_UniversalServer: Test_Runtime { do { let server = UniversalServer( handler: MockHandler(shouldFail: true), - configuration: .init( - serverErrorMapper: { serverError in - // Don't care about the extra context, just wants the underlyingError - serverError.underlyingError - } - ) + configuration: .init(serverErrorMapper: { serverError in + // Don't care about the extra context, just wants the underlyingError + serverError.underlyingError + }) ) _ = try await server.handle( request: .init(soar_path: "/", method: .post), @@ -152,9 +150,7 @@ final class Test_UniversalServer: Test_Runtime { }, serializer: { output, _ in fatalError() } ) - } catch { - XCTAssertTrue(error is TestError, "Threw an unexpected error: \(type(of: error))") - } + } catch { XCTAssertTrue(error is TestError, "Threw an unexpected error: \(type(of: error))") } } func testErrorPropagation_serializer() async throws {