hummingbird-project / hummingbird

Lightweight, flexible HTTP server framework written in Swift
https://hummingbird.codes/
Apache License 2.0
1.21k stars 53 forks source link

Provide a flexible concrete type for HTTPResponseError #602

Open alephao opened 3 days ago

alephao commented 3 days ago

In hummingbird we can throw Swift Errors to halt a request handler. There are multiple instances of this pattern in hummingbird-examples repo. Hummingbird provides HTTPError as the default way to throw a valid HTTP Response, but the current implementation of HTTPType doesn't let us create a custom response body, the only possible format is to return a JSON with the structure {"message": "..."}.

The issue is that in real world projects, you'll probably want to return an HTML response, a custom JSON or something else, but to achieve this currently in hummingbird, you'll need to implement your own type conforming to HTTPResponseError or conform Response to HTTPResponseError.

So since using errors to halt execution of handlers is so common, the proposal here is for to hummingbird provide a way create a completely custom Response that can be thrown. Some options below:

Option 1. Conform Response to HTTPResponseError.

This was discussed briefly in the discord, and it seemed like a no-no, but I'm leaving it here for reference.

The idea is to just conform Response to HTTPResponseError so you can throw responses like throw Response(status: .badRequest) or throw SomeCodableType().response(...)

Option 2. Use a proxy type

We could create a proxy type for Response and conform it to HTTPResponseError. This provides the same flexibility of the Response type, but with a separation between a throwable and a non throwable type. We could initialize this type exactly the same as we would initialize a Response:

// Use same constructor as the Response type
throw ResponseError(status: .badRequest, headers: [:], body: nil)

// Pass a Response type
throw ResponseError(Response(/*....*/))

3. Expand HTTPError to accept a custom body

This is similar to option 2, but instead of creating a new type, we could expand the HTTPError type. I believe this would require some gymnastics to prevent breaking changes on 2.x, so would have to be looked into more carefully.

adam-fowler commented 2 days ago

Originally I setup the HTTP error handling to provide a simple error type (HTTPError) but give the user the ability to provide their own error type (by conforming to HTTPResponseError).

I'm concerned adding an additional type confuses the API. Suddenly we supply two error types and it isn't overly clear why you would choose one over the other.

Adding a custom body to HTTPError might work, but I'm not sure what advantage is giving the user the chance to add a ByteBuffer to the error instead of an message string. Given the amount of additional work they would have to do to generate that ByteBuffer (for every time they create the error), they might as well just create their own error type.