feat(rejection): add rejection as a response extension

[saiintbrisson]

Motivation

There's no current way to handle rejections in a global manner, which adds a certain layer of
complexity requiring the developer to create its own extractor, or use WithRejection every
single time.

It'd be nice to have a way to do this without needing to configure every route separately, to
behave in a way that is expected from all of them.

This is a follow-up to my #1116 (comment).

Solution

The idea was to insert the rejection into the Response extensions when calling IntoResponse.
This is done via modifying the define_rejection macro from axum-core. No clones are required
as self is never consumed inside this macro.

To simplify the process of checking the rejection type, I've added an optional composite meta to
the macro invocation, if present, the rejection is inserted as its composite type, e.g., JsonSyntaxError
is inserted as JsonRejection::JsonSyntaxError.

This allows a middleware to check whether a Response contains rejection information or not, and
respond accordingly:

async fn handle_rejection<B>(req: Request<B>, next: Next<B>) -> Response {
    let resp = next.run(req).await;

    if let Some(rejection) = resp.extensions().get::<JsonRejection>() {
        let payload = json!({
            "message": rejection.body_text(),
            "origin": "response_extension"
        });

        return (resp.status(), axum::Json(payload)).into_response();
    }

    resp
}

This solution doesn't provide a ready way to change how the rejection behaves but gives the devs
more information, and they are allowed to do whatever they want with it. Extensions seem like the
right place as a rejection is metadata regarding a response.

Other ideas

We can even go as far as creating a RejectionHandler<Rejection> layer which receives a fn(Rejection) -> Response, but I don't think it is necessary.

[jplatte]

While this doesn't have to clone the rejection, it still has to allocate for the extension typemap insert operation. Also overwriting the already-serialized response doesn't seem like an ideal solution to me. I think if we add this, it should be behind a Cargo feature that's off by default.

[noghartt]

LGTM

[davidpdrsn]

Thanks for the PR!

I think this is an interesting idea and is something I've considered in the past. However there are a few things I don't like about it:

I main gripe I have is that you have to remember to go update a middleware when you introduce a new rejection. If you forget you might accidentally be sending a wrong rejection response. You could argue that rejections are quite implicit already, which is true.

That is why the current ways to customize rejections are all "local to the handler". You just have to look at the handler to know that you're handling the rejections correctly, maybe via your own wrapper types. You can then use something like clippy's disallowed_types to ensure you're always using your wrappers.

It's also not clear how we should handle rejections that are part of multiple composite rejections, such as BytesRejection. I suppose we would just have to document that FormRejection::BytesRejection and RawFormRejection::BytesRejection will never actually appear in a response extension, but instead the BytesRejection directly. That's a code smell imo.

You could also consider not inserting the rejection directly but maybe something that has the status and body, but that's already available on the response itself, and you loose typed data such as PathRejection's ErrorKind.

[saiintbrisson]

Hi! Thank you both for your insight.

While this doesn't have to clone the rejection, it still has to allocate for the extension typemap insert operation

I see rejections as uncommon cases, having this allocation occur only when rejection takes place doesn't seem
as a performance hit most devs would be worried about.

I main gripe I have is that you have to remember to go update a middleware when you introduce a new rejection.

This looks pretty similar to the process of creating a new custom extractor and remembering to add it to clippy's
disallowed types, if not simpler.

It's also not clear how we should handle rejections that are part of multiple composite rejections

BytesRejection hits me as a rejection on its own, I can't think of a reason to handle it differently for each content-type.
But I see where you are coming from, the same problem occurs with InvalidFormContentType (there could be a
InvalidRawFormContentType counterpart, but it would indeed be awkward).

I could try inserting the composite rejection inside composite_rejection without messing with the define_rejection,
but that would require something clunky like implementing IntoResponse for &'a JsonSyntaxError.

That is why the current ways to customize rejections are all "local to the handler"

I agree that this might be the best approach for small to medium-sized projects. For large workspaces, however,
I don't think it's uncommon for different services to use a common crate when preparing their routers, like for
setting up telemetry, panic handlers, etc. Once you set it up, future services don't really have to worry about
how they handle rejections. This is my personal experience and may not be as common as I think, though.

[davidpdrsn]

This looks pretty similar to the process of creating a new custom extractor and remembering to add it to clippy's
disallowed types, if not simpler.

That's true. Though wrappers have the advantage that you don't pay for the middleware or the extension unless you opt-in to using them. The downside is that it's a bit of boilerplate but probably not much different from a middleware.

BytesRejection hits me as a rejection on its own, I can't think of a reason to handle it differently for each content-type.
But I see where you are coming from, the same problem occurs with InvalidFormContentType (there could be a
InvalidRawFormContentType counterpart, but it would indeed be awkward).

BytesRejection was just the first thing I thought off. There are probably others and regardless it is inconsistent.

I don't think it's uncommon for different services to use a common crate when preparing their routers, like for
setting up observability, panic handlers, etc. Once you set it up, future services don't really have to worry about
how they handle rejections

Yeah I think you're right that that is a common way to get consistency across services, but you could just as well share wrapper types with custom rejections.

Overall I think it's an interesting idea but I'm not sure how to make the judgement call. Might have to spend some time thinking about it and comparing the alternatives.

[saiintbrisson]

Thank you for the quick response nonetheless, I'll leave the decision to close this up to you!

[davidpdrsn]

I'm still not sure this is a great or that much better than the alternatives, so I think I'l err on the side of caution and not merge this, for now at least.

[saiintbrisson]

Thank you for the consideration nonetheless!