apple / apple/swift-openapi-generator
Automatically throw error if HTTP response represents a failure
- Dominant language
- Swift
- Stars
- 2k
- Forks
- 182
- Avg merge
- 13h 28m
- Merged PRs (30d)
- 5
Description
# Motivation
Hi! I am the developer of the [`swift-http-error-handling`](https://swiftpackageindex.com/fumoboy007/swift-http-error-handling) package, which helps with interpreting HTTP responses and handling failures. I am looking to apply some of the concepts from that package to my usage of `swift-openapi-generator`, but I am running into some issues.
## Overview of `swift-http-error-handling`
A core part of `swift-http-error-handling` is interpreting an HTTP response as a success or a failure.
### Representing an HTTP Application Failure
The package introduces an [`HTTPApplicationError`](https://fumoboy007.github.io/swift-http-error-handling/documentation/httperrorhandling/httpapplicationerror) type to represent a failure. This allows the failure to be propagated and handled using Swift’s standard try-catch error handling mechanism. The error type contains the following information:
- The `HTTPResponse` value.
- A generically-typed value representing the response body, if desired. Some HTTP servers add additional details about a failure to the response body.
- Whether the failure is transient. This is important for the package’s [retry feature](https://fumoboy007.github.io/swift-http-error-handling/documentation/httperrorhandling#Retrying-HTTP-Requests).
### Interpreting HTTP Responses
The package also extends `HTTPResponse` to add two methods: [`throwIfFailed(successStatuses:transientFailureStatuses:)`](https://fumoboy007.github.io/swift-http-error-handling/documentation/httperrorhandling/httptypes/httpresponse/throwiffailed(successstatuses:transientfailurestatuses:)) and [`throwIfFailed(successStatuses:transientFailureStatuses:makeResponseBody:)`](https://fumoboy007.github.io/swift-http-error-handling/documentation/httperrorhandling/httptypes/httpresponse/throwiffailed(successstatuses:transientfailurestatuses:makeresponsebody:)).
The methods have [default values](https://fumoboy007.github.io/swift-http-error-handling/documentation/httperrorhandling/httptypes/httpresponse/status) for the `successStatuses` and `transientFailureStatuses` parameters so that the methods can be called conveniently:
```swift
try response.throwIfFailed()
```
The `successStatuses` and `transientFailureStatuses` parameters allow for a customized interpretation if needed:
```swift
try response.throwIfFailed(
successStatuses: [.created],
transientFailureStatuses: HTTPResponse.Status.transientFailures.union([.conflict])
)
```
The second `throwIfFailed` method accepts an `async` closure to attach a response body to the error:
```swift
try await response.throwIfFailed {
return try await deserializeFailureDetails(from: responseBody)
}
```
The response body can be accessed later like so:
```swift
do {
try await performRequest()
} catch let error as HTTPApplicationError {
let failureDetails = error.responseBody
doSomething(with: failureDetails)
}
```
## Shortcomings of the OpenAPI-Generated Client
If I wanted to interpret an OpenAPI-generated response as a success or failure, I might do the following:
```swift
let response = try await client.getGreeting()
doSomething(with: try response.ok.body.json)
```
If the HTTP response status was not `.ok`, then the `response.ok` property access would throw `RuntimeError.unexpectedResponseStatus`. This seems like it works for our use case; however, there are a number of issues with this approach.
### Unsafe
A developer needs to remember to check the response status. Not checking the response status introduces a subtle bug.
This is obvious for experienced developers who have used Swift HTTP libraries like `Foundation` or `AsyncHTTPClient` but may be non-intuitive for other developers. Even experienced developers may forget to check the response status in cases where they do not intend to access the response body.
### Inconvenient
Consider the case where a response can have multiple successful statuses (e.g. `.ok`, `.notFound`, and `.gone` for a delete request). The developer would have to do one of two things, both of which are inconvenient:
```swift
let response = try await client.deleteSomething()
do {
_ = try response.ok
} catch {
do {
_ = try response.notFound
} catch {
_ = try response.gone
}
}
```
```swift
let response = try await client.deleteSomething()
switch response {
case .ok, .notFound, .gone:
break
default:
throw MyCustomError() // need to define our own error type
```
### Limited Error Handling Ability
The ability to handle such an error is limited by the following:
- The error type is not public.
- The error does not contain the response body.
### Duplicate Logic in Middleware
Some middleware also needs to interpret the response as a success or failure (e.g. to retry, to log failures, etc.). Now, the response interpretation logic is duplicated across the caller and the middleware, which is both inconvenient and could cause inconsistency across the two implementations, causing bugs.
# Proposed Solution
I propose a breaking change for `swift-openapi-generator` version `2.0.0` to address all of the current shortcomings.
I propose that the client automatically interprets responses and throws `HTTPApplicationError` when the response represents a failure.
Since the response deserialization code is inside of the client, the client could deserialize the response body, if any, and attach it to the error automatically. Error handling code would be able to catch the error using its concrete type `HTTPApplicationError` or as an instance of [`HTTPApplicationErrorProtocol`](https://fumoboy007.github.io/swift-http-error-handling/documentation/httperrorhandling/httpapplicationerrorprotocol) if the code does not care about the response body.
The generated client could expose properties to customize the response interpretation. The generated client methods could also have additional parameters to customize the response interpretation per operation.
Finally, it would be important for the client to interpret the response before passing the response to the middleware so that the middleware do not have to do their own response interpretation.
Putting it all together, it would look something like the following:
```swift
let client = Client(serverURL: URL(string: "http://localhost:8080/api")!,
transport: URLSessionTransport(),
// The middleware will handle HTTP application failures using the standard
// try-catch mechanism.
middlewares: [RetryMiddleware(), LoggingMiddleware()])
do {
// Safer than before.
_ = try await client.doSomething()
// More convenient than before.
_ = try await client.deleteSomething(successStatuses: [.ok, .notFound, .gone])
} catch let error as any HTTPApplicationErrorProtocol {
// The error type is public, allowing for more sophisticated error handling.
doSomething(with: error)
}
```
# Alternatives Considered
## Alternative 1: Add middleware that interprets the response
Instead of building the response interpretation into the client directly, `swift-openapi-runtime` or a third-party library could expose a hypothetical `ResponseInterpretationMiddleware` that does the response interpretation. Developers would add `ResponseInterpretationMiddleware` to the list of middleware when creating the client.
The first issue with this alternative is that middleware do not have access to the response deserialization code, so `ResponseInterpretationMiddleware` would not be able to attach the deserialized response body to the error.
Second, since the `ResponseInterpretationMiddleware` is optional, other middleware would not be able to rely on its availability and would need to do their own response interpretation, duplicating logic.
Finally, this alternative is less safe compared to the proposed solution since the developer would need to remember to add the `ResponseInterpretationMiddleware` to the list of middleware.
## Alternative 2: Add a property to the generated response that returns the `HTTPResponse` value
In this alternative, the generated response would have a property to access the `HTTPResponse` value. The caller could then interpret the response themselves.
This alternative does not solve the safety issue since the developer still needs to remember to interpret the response.
This alternative also does not solve the middleware code duplication issue since the middleware would still need to do their own response interpretation.
# Additional Information
If the proposed solution is accepted, feel free to add a dependency to my `swift-http-error-handling` package to help with implementation. You would gain access to the `HTTPApplicationError` type, the `throwIfFailed` methods, the default set of success statuses, and the default set of transient failure statuses.
I promise I won’t do anything sketchy with my package. 😆 But if it makes you feel better, I am also considering creating a proposal to merge my package into the `swift-http-types` package. Let me know if that would be a prerequisite. If it is, I’ll get right on it!
Contributor guide
Research direction
Start by tracing the generated client’s response deserialization flow and how it passes responses to middleware. Review the proposed HTTPApplicationError and HTTPApplicationErrorProtocol behavior, including Operations.myOperation.Output, and determine how customizable success statuses and response bodies should work. Done means the design resolves automatic failure interpretation, middleware ordering, and the breaking-change scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- openapi, swift
- Domain
- api, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100