Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a flexible concrete type for HTTPResponseError #602

Open
alephao opened this issue Nov 12, 2024 · 1 comment
Open

Provide a flexible concrete type for HTTPResponseError #602

alephao opened this issue Nov 12, 2024 · 1 comment

Comments

@alephao
Copy link
Contributor

alephao commented Nov 12, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants