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

Allowing Handlers to not return IntoResponse #3229

Open
mladedav opened this issue Feb 20, 2025 · 0 comments
Open

Allowing Handlers to not return IntoResponse #3229

mladedav opened this issue Feb 20, 2025 · 0 comments
Labels
C-musings Category: musings about a better world

Comments

@mladedav
Copy link
Collaborator

Motivation

There have been requests to convert users' errors into responses in a middleware with access to state (#3211) or propagating errors without converting them into responses first (#2587).

Another request was to make sure every route uses an authorization extractor (#3221). This could be solved by this by having all handlers return an AuthorizedIntoResponse which could be created only with one of the authorization extractors.

Other things this should allow for is returning objects that impl Serialize and converting them to Responses based on Accept-Encoding.

I've also sometimes wanted to be able to transform the returned value before converting it into an HTTP response.

Current State

Currently, each Handler must return impl IntoResponse and the response is automatically immediately converted into Response which can then be acted upon by middlewares.

This architecture lets people to transform only already serialized textual or binary data inside an HTTP response. This is good for things like adding HTTP headers or compression but less so for other transformations like adding a common field to JSON payloads.

Similarly, MethodRouter expects each of its services to return Response and PathRouter (and RouterInner) expects each of its inner services (usually MethodRouters) to also return Response.

Proposal

We can allow Handlers to return any type. Similarly MethodRouter, PathRouter, and Router can then allow any type to be created from a Request. In the end we want to call into_response on whatever is eventually returned, but that can happen as late as in Serve which would require that the Router's response type implements IntoResponse.

Instead of generating Handler implementation like this:

impl<F, Fut, S, Res, M, $($ty,)* $last> Handler<(M, $($ty,)* $last,), S> for F
where
    F: FnOnce($($ty,)* $last,) -> Fut + Clone + Send + 'static,
    Fut: Future<Output = Res> + Send,
    S: Send + Sync + 'static,
    Res: IntoResponse,
    $( $ty: FromRequestParts<S> + Send, )*
    $last: FromRequest<S, M> + Send,
{
    type Future = Pin<Box<dyn Future<Output = Response> + Send>>;

    // ...
}

we can generate:

// Added new generic type to `Handler` signifying return type of `Handler::call`
impl<F, Fut, S, Res, M, $($ty,)* $last> Handler<(M, $($ty,)* $last,), S, Result<Res, Response>> for F
where
    F: FnOnce($($ty,)* $last,) -> Fut + Clone + Send + Sync + 'static,
    Fut: Future<Output = Res> + Send,
    S: Send + Sync + 'static,
    Res: 'static, // The returned value does not need implement `IntoResponse`
    $( $ty: FromRequestParts<S> + Send, )*
    $last: FromRequest<S, M> + Send,
{
    // `Handler::call` will return a future that would resolve to `Res` if all extractors run
    // successfully or to `Response` created from an extractor rejection.
    type Future = Pin<Box<dyn Future<Output = Result<Res, Response>> + Send>>;

    // ...
}

Of note is that the returned future resolves Result<Res, Response> instead of just Res. This is because extractors can fail and we need to return something. There are other options for the Err variant but this is the simplest solution for the prototype I made.

Similarly, we can add another generic parameter to MethodRouter and Router signifying what type will the responses be.

We would require all handlers in a given MethodRouter to resolve to the same type and all Services inside a Router to also resolve to the same type. That type can be changed dynamically though using mapping layers.

Drawbacks

Requiring that services under all router paths may be a bit restrictive and push people to just convert everything into Responses anyway. This would probably also introduce new hard-to-debug type errors.

I also think this would be an advanced feature that most users would not need to use often.

So while we can have an AdvancedRouter<S = (), Res = Response> we can still expose Router<S = ()> which would work the same way as the Router we have today. That way users can decide between the simpler more user-friendly version or the advanced more powerful version. This would make the API surface we need to maintain larger, but the functionality itself would be concentrated only in the general versions of the routers.

Open Questions

How to handle extractor rejections. Converting them to Responses is simple but we could also return an enum containing a variant for each extractor or something like Box<dyn Rejection>.

How to handle default fallbacks. If Router returns Responses, we have decided that returning an empty 404 response is a sane default. There may not be similar sane defaults for other types. We might require explicit fallbacks.

Example

I've created a version of this (without having the current and advanced versions side by side) in this branch, where there's also a bit contrived example returning a custom struct which gets turned into Response in a later middleware.

Alternatives

Some of the unlocked functionality can be achieved with response extensions. This would however allow users to use typechecking to make sure they did not overlook a handler where they forget to set the extension.

@mladedav mladedav added the C-musings Category: musings about a better world label Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-musings Category: musings about a better world
Projects
None yet
Development

No branches or pull requests

1 participant