From 7fe0b73dbd23cb6554a817c9ada934099d4bd2ca Mon Sep 17 00:00:00 2001 From: Gayathri Sairamkrishnan Date: Tue, 17 Sep 2024 09:20:05 +0100 Subject: [PATCH 01/11] Proposal for error handling --- .../Documentation.docc/Proposals/SOAR-0011.md | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md new file mode 100644 index 00000000..60bbf1af --- /dev/null +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -0,0 +1,151 @@ +# SOAR-0011: Improved Error Handling + +Improve error handling by adding the ability for mapping application errors to HTTP responses. + +## Overview + +- Proposal: SOAR-0011 +- Author(s): [Gayathri Sairamkrishnan](https://github.com/gayathrisairam) +- Status: **Awaiting Review** +- Issue: [apple/swift-openapi-generator#609](https://github.com/apple/swift-openapi-generator/issues/609) +- Affected components: + - runtime + +### Introduction + +The goal of this proposal to improve the current error handling mechanism in Swift OpenAPI runtime. The proposal +introduces a way for users to map errors thrown by their handlers to specific HTTP responses. + +### Motivation + + When implementing a server with Swift OpenAPI Generator, users implement a type that conforms to a generated protocol, providing one method for each API operation defined in the OpenAPI document. At runtime, if this function throws, the runtime library transforms this into a 500 HTTP response (Internal Error). + +Instead, server developers may want to map errors thrown by the application to a more specific HTTP response. +Currently, this can be achieved by checking for each error type in each handler's catch block, converting it to an +appropriate HTTP response and returning it. + +For example, +```swift +func getGreeting(_ input: Operations.getGreeting.Input) async throws -> Operations.getGreeting.Output { + do { + let response = try callGreetingLib() + return .ok(.init(body: response)) + } catch let error { + switch error { + case GreetingError.authorizationError: + return (HTTPResponse(status: 404), nil) + case GreetingError.timeout: + return ... + } + } +} +``` +If a user wishes to map many errors, the error handling block scales linearly and introduces a lot of ceremony. + +### Proposed solution + +The proposed solution is twofold. + +1. Provide protocol(s) in `OpenAPIRuntime` to allow users to extend their error types with mappings to HTTP responses. + +2. Provide an (opt-in) middleware in OpenAPIRuntime that will call the conversion function on conforming error types when +constructing the HTTP response. + +Vapor has a similar mechanism called [AbortError](https://docs.vapor.codes/basics/errors/). + +HummingBird also has an [error handling mechanism](https://docs.hummingbird.codes/2.0/documentation/hummingbird/errorhandling/) +by allowing users to define a [HTTPError](https://docs.hummingbird.codes/2.0/documentation/hummingbird/httperror) + +The proposal aims to provide a transport agnostic error handling mechanism for OpenAPI users. + +### Detailed design + +#### Proposed Error protocols + +Users can choose to conform to one of the protocols described below depending on the level of specificity they would +like to have in the response. + +```swift +public protocol HTTPStatusConvertible { + var httpStatus: HTTPResponse.Status { get } +} + +public protocol HTTPHeadConvertible: HTTPStatusConvertible { + var httpHeaderFields: HTTPTypes.HTTPFields { get } +} + +public protocol HTTPResponseConvertible: HTTPHeadConvertible { + var httpBody: OpenAPIRuntime.HTTPBody { get } +} +``` + +#### Proposed Error Middleware + +The proposed error middleware in OpenAPIRuntime will convert the application error to the appropriate error response. +```swift +public struct ErrorMiddleware: ServerMiddleware { + func intercept(_ request: HTTPTypes.HTTPRequest, + body: OpenAPIRuntime.HTTPBody?, + metadata: OpenAPIRuntime.ServerRequestMetadata, + operationID: String, + next: @Sendable (HTTPTypes.HTTPRequest, OpenAPIRuntime.HTTPBody?, OpenAPIRuntime.ServerRequestMetadata) async throws -> (HTTPTypes.HTTPResponse, OpenAPIRuntime.HTTPBody?)) async throws -> (HTTPTypes.HTTPResponse, OpenAPIRuntime.HTTPBody?) { + do { + return try await next(request, body, metadata) + } catch let error as ServerError { + if let appError = error.underlyingError as? HTTPStatusConvertible else { + return (HTTPResponse(status: appError.httpStatus), nil) + } else if ... { + + } + } + } +} +``` + +Please note that the proposal places the responsibility to conform to the documented API in the hands of the user. +There's no mechanism to prevent the users from inadvertently transforming a thrown error into an undocumented response. + +#### Example usage + +1. Create an error type that conforms to one of the error protocol(s) +```swift +extension MyAppError: HTTPStatusConvertible { + var httpStatus: HTTPResponse.Status { + switch self { + case .invalidInputFormat: + HTTPResponse.Status.badRequest + case .authorizationError: + HTTPResponse.Status.forbidden + } + } +} +``` + +2. Opt in to the error middleware while registering the handler + +```swift +let handler = try await RequestHandler() +try handler.registerHandlers(on: transport, middlewares: [ErrorHandlingMiddleware()]) + +``` + +### API stability + +This feature is purely additive: +- Additional APIs in the runtime library + + +### Future directions + +A possible future direction is to add the error middleware by default by changing the [default value for the middlewares](https://github.com/apple/swift-openapi-runtime/blob/main/Sources/OpenAPIRuntime/Interface/UniversalServer.swift#L56) +argument in handler initialisation. + +### Alternatives considered + +An alternative here is to invoke the error conversion function directly from OpenAPIRuntime's handler. The feature would +still be opt-in as users have to explicitly conform to the new error protocols. + +However, there is a rare case where an application might depend on a library (for eg: an auth library) which in turn +depends on OpenAPIRuntime. If the authentication library conforms to the new error protocols, this would result in a +breaking change for the application, whereas an error middleware provides flexibility to the user on whether they +want to subscribe to the new behaviour or not. From b1481bd966065525c9abe940978e0ce0229978c7 Mon Sep 17 00:00:00 2001 From: gayathrisairam <168187165+gayathrisairam@users.noreply.github.com> Date: Tue, 17 Sep 2024 09:46:54 +0100 Subject: [PATCH 02/11] Update Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md Co-authored-by: Honza Dvorsky --- .../Documentation.docc/Proposals/SOAR-0011.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index 60bbf1af..db54dcb7 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -53,7 +53,7 @@ constructing the HTTP response. Vapor has a similar mechanism called [AbortError](https://docs.vapor.codes/basics/errors/). -HummingBird also has an [error handling mechanism](https://docs.hummingbird.codes/2.0/documentation/hummingbird/errorhandling/) +Hummingbird also has an [error handling mechanism](https://docs.hummingbird.codes/2.0/documentation/hummingbird/errorhandling/) by allowing users to define a [HTTPError](https://docs.hummingbird.codes/2.0/documentation/hummingbird/httperror) The proposal aims to provide a transport agnostic error handling mechanism for OpenAPI users. From e29736aa36612dd43fdd7536a1223981005cc06a Mon Sep 17 00:00:00 2001 From: gayathrisairam <168187165+gayathrisairam@users.noreply.github.com> Date: Tue, 17 Sep 2024 09:48:32 +0100 Subject: [PATCH 03/11] Update Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md Co-authored-by: Honza Dvorsky --- .../Documentation.docc/Proposals/SOAR-0011.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index db54dcb7..5572ca4e 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -113,9 +113,9 @@ extension MyAppError: HTTPStatusConvertible { var httpStatus: HTTPResponse.Status { switch self { case .invalidInputFormat: - HTTPResponse.Status.badRequest + .badRequest case .authorizationError: - HTTPResponse.Status.forbidden + .forbidden } } } From 205d78cb41d6f466065c2c0f8056fc94b732cec0 Mon Sep 17 00:00:00 2001 From: Gayathri Sairamkrishnan Date: Tue, 17 Sep 2024 09:53:13 +0100 Subject: [PATCH 04/11] Review comments --- .../Documentation.docc/Proposals/SOAR-0011.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index 5572ca4e..b8915b04 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -33,7 +33,7 @@ func getGreeting(_ input: Operations.getGreeting.Input) async throws -> Operatio } catch let error { switch error { case GreetingError.authorizationError: - return (HTTPResponse(status: 404), nil) + return .unauthorized(.init()) case GreetingError.timeout: return ... } @@ -125,7 +125,7 @@ extension MyAppError: HTTPStatusConvertible { ```swift let handler = try await RequestHandler() -try handler.registerHandlers(on: transport, middlewares: [ErrorHandlingMiddleware()]) +try handler.registerHandlers(on: transport, middlewares: [ErrorMiddleware()]) ``` From 09bd716d48c27ed9ae2f0892ad0dcd9811d1ccb6 Mon Sep 17 00:00:00 2001 From: gayathrisairam <168187165+gayathrisairam@users.noreply.github.com> Date: Wed, 18 Sep 2024 08:49:34 +0100 Subject: [PATCH 05/11] Update Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md Co-authored-by: Honza Dvorsky --- .../Documentation.docc/Proposals/SOAR-0011.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index b8915b04..0180cd7a 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -18,7 +18,7 @@ introduces a way for users to map errors thrown by their handlers to specific HT ### Motivation - When implementing a server with Swift OpenAPI Generator, users implement a type that conforms to a generated protocol, providing one method for each API operation defined in the OpenAPI document. At runtime, if this function throws, the runtime library transforms this into a 500 HTTP response (Internal Error). + When implementing a server with Swift OpenAPI Generator, users implement a type that conforms to a generated protocol, providing one method for each API operation defined in the OpenAPI document. At runtime, if this function throws, it's up to the server transport to transform it into an HTTP response status code – for example, some transport use `500 Internal Error`. Instead, server developers may want to map errors thrown by the application to a more specific HTTP response. Currently, this can be achieved by checking for each error type in each handler's catch block, converting it to an From e6fcb194d4f3ef75dbbcdf80b7477d115adc4b44 Mon Sep 17 00:00:00 2001 From: Gayathri Sairamkrishnan Date: Thu, 19 Sep 2024 09:01:47 +0100 Subject: [PATCH 06/11] Add link to SOAR-0011 in Proposals.md --- .../Documentation.docc/Proposals/Proposals.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/Proposals.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/Proposals.md index 752cc0a6..cb72312e 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/Proposals.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/Proposals.md @@ -52,3 +52,4 @@ If you have any questions, tag [Honza Dvorsky](https://github.com/czechboy0) or - - - +- From e4725c45b0511f04738b752ddcc5629d1ce949e1 Mon Sep 17 00:00:00 2001 From: gayathrisairam <168187165+gayathrisairam@users.noreply.github.com> Date: Fri, 4 Oct 2024 10:00:44 +0100 Subject: [PATCH 07/11] Update Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md Co-authored-by: Honza Dvorsky --- .../Documentation.docc/Proposals/SOAR-0011.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index 0180cd7a..04d1f3b3 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -6,7 +6,7 @@ Improve error handling by adding the ability for mapping application errors to H - Proposal: SOAR-0011 - Author(s): [Gayathri Sairamkrishnan](https://github.com/gayathrisairam) -- Status: **Awaiting Review** +- Status: **In Review** - Issue: [apple/swift-openapi-generator#609](https://github.com/apple/swift-openapi-generator/issues/609) - Affected components: - runtime From 76d31c6a01424f05d401e214ce7394511d07f3f8 Mon Sep 17 00:00:00 2001 From: Gayathri Sairamkrishnan Date: Mon, 7 Oct 2024 10:50:42 +0100 Subject: [PATCH 08/11] Update proposal based on discussion from forums --- .../Documentation.docc/Proposals/SOAR-0011.md | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index 04d1f3b3..475f2c46 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -10,6 +10,11 @@ Improve error handling by adding the ability for mapping application errors to H - Issue: [apple/swift-openapi-generator#609](https://github.com/apple/swift-openapi-generator/issues/609) - Affected components: - runtime +- Versions: + - v1.0 (2024-09-19): Initial version + - v1.1(2024-10-07): + - Replace the proposed solution to have a single error handling protocol, with the status being required and + headers/body being optional. ### Introduction @@ -18,7 +23,8 @@ introduces a way for users to map errors thrown by their handlers to specific HT ### Motivation - When implementing a server with Swift OpenAPI Generator, users implement a type that conforms to a generated protocol, providing one method for each API operation defined in the OpenAPI document. At runtime, if this function throws, it's up to the server transport to transform it into an HTTP response status code – for example, some transport use `500 Internal Error`. + When implementing a server with Swift OpenAPI Generator, users implement a type that conforms to a generated protocol, + providing one method for each API operation defined in the OpenAPI document. At runtime, if this function throws, it's up to the server transport to transform it into an HTTP response status code – for example, some transport use `500 Internal Error`. Instead, server developers may want to map errors thrown by the application to a more specific HTTP response. Currently, this can be achieved by checking for each error type in each handler's catch block, converting it to an @@ -46,7 +52,7 @@ If a user wishes to map many errors, the error handling block scales linearly an The proposed solution is twofold. -1. Provide protocol(s) in `OpenAPIRuntime` to allow users to extend their error types with mappings to HTTP responses. +1. Provide a protocol in `OpenAPIRuntime` to allow users to extend their error types with mappings to HTTP responses. 2. Provide an (opt-in) middleware in OpenAPIRuntime that will call the conversion function on conforming error types when constructing the HTTP response. @@ -62,20 +68,19 @@ The proposal aims to provide a transport agnostic error handling mechanism for O #### Proposed Error protocols -Users can choose to conform to one of the protocols described below depending on the level of specificity they would -like to have in the response. +Users can choose to conform to the error handling protocol below and optionally provide the optional fields depending on +the level of specificity they would like to have in the response. ```swift -public protocol HTTPStatusConvertible { +public protocol HTTPResponseConvertible { var httpStatus: HTTPResponse.Status { get } -} - -public protocol HTTPHeadConvertible: HTTPStatusConvertible { var httpHeaderFields: HTTPTypes.HTTPFields { get } + var httpBody: OpenAPIRuntime.HTTPBody? { get } } -public protocol HTTPResponseConvertible: HTTPHeadConvertible { - var httpBody: OpenAPIRuntime.HTTPBody { get } +extension HTTPResponseConvertible { + var httpHeaderFields: HTTPTypes.HTTPFields { [:] } + var httpBody: OpenAPIRuntime.HTTPBody? { nil } } ``` @@ -92,10 +97,11 @@ public struct ErrorMiddleware: ServerMiddleware { do { return try await next(request, body, metadata) } catch let error as ServerError { - if let appError = error.underlyingError as? HTTPStatusConvertible else { - return (HTTPResponse(status: appError.httpStatus), nil) - } else if ... { - + if let appError = error.underlyingError as? HTTPResponseConvertible else { + return (HTTPResponse(status: appError.httpStatus, headerFields: appError.httpHeaderFields), + appError.httpBody) + } else { + throw error } } } @@ -107,9 +113,9 @@ There's no mechanism to prevent the users from inadvertently transforming a thro #### Example usage -1. Create an error type that conforms to one of the error protocol(s) +1. Create an error type that conforms to the error protocol ```swift -extension MyAppError: HTTPStatusConvertible { +extension MyAppError: HTTPResponseConvertible { var httpStatus: HTTPResponse.Status { switch self { case .invalidInputFormat: From c671f5768a4b7d8cb96f4018cbff52dbdf552cf8 Mon Sep 17 00:00:00 2001 From: Gayathri Sairamkrishnan Date: Thu, 10 Oct 2024 15:56:18 +0100 Subject: [PATCH 09/11] Update error handling middleware name --- .../Documentation.docc/Proposals/SOAR-0011.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index 475f2c46..e03b9fd7 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -87,8 +87,10 @@ extension HTTPResponseConvertible { #### Proposed Error Middleware The proposed error middleware in OpenAPIRuntime will convert the application error to the appropriate error response. + It returns 500 for application error(s) that do not conform to HTTPResponseConvertible protocol. + ```swift -public struct ErrorMiddleware: ServerMiddleware { +public struct ErrorHandlingMiddleware: ServerMiddleware { func intercept(_ request: HTTPTypes.HTTPRequest, body: OpenAPIRuntime.HTTPBody?, metadata: OpenAPIRuntime.ServerRequestMetadata, @@ -131,7 +133,7 @@ extension MyAppError: HTTPResponseConvertible { ```swift let handler = try await RequestHandler() -try handler.registerHandlers(on: transport, middlewares: [ErrorMiddleware()]) +try handler.registerHandlers(on: transport, middlewares: [ErrorHandlingMiddleware()]) ``` From 71eb331daba3cc02702f5b3b0577e983bec0f3f4 Mon Sep 17 00:00:00 2001 From: gayathrisairam <168187165+gayathrisairam@users.noreply.github.com> Date: Mon, 11 Nov 2024 09:17:52 +0000 Subject: [PATCH 10/11] Update Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md Co-authored-by: Honza Dvorsky --- .../Documentation.docc/Proposals/SOAR-0011.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index e03b9fd7..1c9182cc 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -6,7 +6,7 @@ Improve error handling by adding the ability for mapping application errors to H - Proposal: SOAR-0011 - Author(s): [Gayathri Sairamkrishnan](https://github.com/gayathrisairam) -- Status: **In Review** +- Status: **Ready for Implementation** - Issue: [apple/swift-openapi-generator#609](https://github.com/apple/swift-openapi-generator/issues/609) - Affected components: - runtime From 55bd604e325618db9067bd6b28eba4514a5220e3 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Fri, 22 Nov 2024 08:03:51 +0100 Subject: [PATCH 11/11] Update Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md --- .../Documentation.docc/Proposals/SOAR-0011.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md index 1c9182cc..1ebe5af7 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0011.md @@ -6,7 +6,7 @@ Improve error handling by adding the ability for mapping application errors to H - Proposal: SOAR-0011 - Author(s): [Gayathri Sairamkrishnan](https://github.com/gayathrisairam) -- Status: **Ready for Implementation** +- Status: **Implemented** - Issue: [apple/swift-openapi-generator#609](https://github.com/apple/swift-openapi-generator/issues/609) - Affected components: - runtime