Improve the custom rejection experience

[jplatte]

Currently, rejections from extractors in axum and axum-extra turn into responses with plain-text bodies. In some cases this is not a problem but many people are using axum to implement API servers, and at least for rejections that are caused by invalid API usage (rather than server implementation errors), you would most often want a machine-readable error instead.

This means that many extractors such as Json or Query are rarely suitable for production – instead you have to write your own¹. We have examples for this, but it's really unfortunate you can't just do the obvious thing of using the provided ones. There have been many requests for being able to customize the rejections somehow, but we have not found a design that's clearly better than requiring people to create their own extractors.

These are the ideas we had so far, and why they were rejected:

1. Add a defaulted generic const parameter to the extractors, something like

struct Json<T, const F: fn(serde_json::Error) -> Response = default_error_to_response>(T);

• Rejected because it isn't possible on stable (and nightly right now, actually, though it will likely become possible at some point), and makes the type rather unwieldy in the documentation

2. Add a defaulted generic type parameter to the extractors, something like

struct Json<T, R = PlainTextJsonRejection>(T);

where R has to implement both From<serde_json::Error> and IntoResponse

• Rejected because it isn't possible with Rust right now, generic type parameters on a struct need to be mentioned in one of the fields, but adding a PhantomData<R> breaks usage of Json(value) as a pattern and constructor

3. Add some config mechanism, the most basic form of which would be

my_router.layer(Extension(JsonConfig {
    serialize_error: |e: serde_json::Error| /* turn e into axum::response::Response */,
    ..Default::default()
}))

• Rejected because it is non-local and has an unnecessary runtime cost (this really seems like something that should be configured at compile-time, not at runtime)

¹ these can sometimes delegate to the provided ones, but not always: for example if you want to include line / column as machine-readable fields of a json rejection, you have to write JSON extractor "from scratch" – though this is hardly more complex than delegating to axum::Json

[simon-amadeus]

Maybe this is a good place to mention, that I have to implement custom rejections for Json and Query.

I am doing this, because the default rejections return error messages that contain internal specifics such as module paths on an public rest-api, e.g.:

Failed to deserialize query string. Expected something of type 'registration_service::routes::registrations::RequestParams'. Error: missing field 'email'.

Nothing serious, but my stomach tells me to avoid exposing internal details of any kind in public environments, at least as a default.

Btw, working with axum has been a pleasure so far and i find it very well documented. Especially the examples have been very helpful and it is great to have a reliable alternative to actix. So thank you @davidpdrsn and all the others for this awesome work!

[davidpdrsn]

Nothing serious, but my stomach tells me to avoid exposing internal details of any kind in public environments, at least as a default.

Yep I agree with that. I considered just logging the errors and return empty responses but that makes it way harder to discover and will likely cause more confusion. And since you'll probably customize the rejections anyway I figured it was fine to include those things directly in the response.

[simon-amadeus]

I see... Well, as this example covers the details, it is easy to implement anyway, but maybe it should be mentioned in the docs somehow?

I just found out about it randomly, when sending a request on the command line with curl. My integration tests missed it, as I did not test for empty response bodies in "failure" cases.

Somehow I did not expect this behavior, but maybe it boils down to writing better tests 😄

[davidpdrsn]

I still think its the right default. I'm certain we're gonna get so many questions about it otherwise. People don't always setup tracing to begin with.

I'd accept a PR that mentions it in the docs though (:

[davidpdrsn]

I've written a crate that explores strategy 2 and 3. You can find it here https://github.com/davidpdrsn/axum-extractor-config

Some learnings

• Doing the conversion to a custom rejection with an async fn would be cool because then you can run extractors. However I cannot get that working with extensions (2). You run into lifetime issues where the future returned from a boxed function needs to borrow from one of the inputs which you cannot do:

  struct JsonConfig {
      rejection_handler: Arc<
        dyn for<'a> Fn(JsonRejection, &'a mut RequestParts<B>) -> BoxFuture<'a, Response>
                                                                            ^^ cannot use 'a here
      >,
  }
  

  Not sure if this is a showstopper but its a little annoying for sure.
• Doing async stuff is possible with phantom types (3), however you cannot use type aliases as constructors so this doesn't work:

  type Json<T> = axum_extractor_config::Json<T, CustomRejection>;
  
  async fn handler(Json(value, _): Json<Value>) {}
                   ^^^^ cannot use type alias as constructor
  
  // so you have to do this everywhere =(
  async fn handler(Json(value, _): Json<Value, CustomRejection>) {}
  
  // or this which is super weird
  async fn handler(axum_extractor_config::Json(value, _): Json<Value>) {}
  

  I think this is a showstopper. Having to write Json<Value, CustomRejection> everywhere is pretty annoying.

[jplatte]

you cannot use type aliases as constructors

Have you checked whether there are any bug reports about this on rust-lang/rust? It looks more like a compiler limitation to me rather than an intentional design decision.

[davidpdrsn]

Have you checked whether there are any bug reports about this on rust-lang/rust? It looks more like a compiler limitation to me rather than an intentional design decision.

No I haven't checked but yeah does feel like something that should work.

I've been thinking that we could probably improve things a bit by making it possible to override the rejection in #[derive(FromRequest)] like so:

#[derive(FromRequest)]
#[from_request(
  via(axum::Json),
  rejection(MyRejection),
)]
struct Json<T>(T);

struct MyRejection { ... }

impl From<JsonRejection> for MyRejection {
  ...
}

impl IntoResponse for MyRejection {
  ...
}

While its not an ideal solution it does remove most of the boilerplate. I think we should support this regardless but might be good enough until the necessary features stabilize.

[davidpdrsn]

I've implemented what I posted above in https://github.com/tokio-rs/axum/pull/1256. Still not an ideal solution but better than before.

[Altair-Bueno]

2. Add a defaulted generic type parameter to the extractors, something like

struct Json<T, R = PlainTextJsonRejection>(T);

where R has to implement both From<serde_json::Error> and IntoResponse

• Rejected because it isn't possible with Rust right now, generic type parameters on a struct need to be mentioned in one of the fields, but adding a PhantomData<R> breaks usage of Json(value) as a pattern and constructor

I think I'm a bit late, but there is one other alternative: Instead of breaking the current extractors, add an "extractor of extractors". Something like this:

use std::{
    error::Error, marker::PhantomData,
};

use axum::{
    async_trait,
    extract::{FromRequest, RequestParts},
    http::StatusCode,
    response::IntoResponse,
    routing::get,
    Json, Router, Server,
};
use serde_json::{json, Value};

// Generic extractor
struct ExtractWith<E, S>(pub E, pub PhantomData<S>);

#[async_trait]
impl<B, E, S> FromRequest<B> for ExtractWith<E, S>
where
    B: Send,
    E: FromRequest<B>,
    E::Rejection: Error,
    S: Strategy<E::Rejection>,
{
    type Rejection = S::Response;

    async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
        // Applies extractor. If it fails, applies strategy
        match req.extract::<E>().await {
            Ok(x) => Ok(ExtractWith(x, Default::default())),
            Err(x) => Err(S::apply(x)),
        }
    }
}

pub trait Strategy<E: Error> {
    type Response: IntoResponse;

    fn apply(err: E) -> Self::Response;
}

// User implements its own strategy, Format the error as they like. Different
// endpoints have different error responses.
pub struct MyStrategy;

impl<E: Error> Strategy<E> for MyStrategy {
    type Response = (StatusCode, Json<Value>);

    fn apply(err: E) -> Self::Response {
        let value = json!({
            "err": err.to_string()
        });
        (StatusCode::INTERNAL_SERVER_ERROR, Json(value))
    }
}

async fn handler(
    ExtractWith(Json(json), _): ExtractWith<Json<()>, MyStrategy>,
) -> impl IntoResponse {
    ()
}

#[tokio::main]
async fn main() {
    let app = Router::new().route("/", get(handler));

    Server::bind(&([127, 0, 0, 1], 8080).into())
        .serve(app.into_make_service())
        .await
        .unwrap()
}

This code compiles today, and is far more customisable.

Pros

• Works with almost all already existing extractors (even external libraries!). Most rejections already implement std::error::Error. There is no need to change the ecosystem, no breaking changes
• Easy mix and reformat. If you ever plan on replacing your strategy or combining multiple, you can use type aliases incrementally
• Performant. Compile time monomorphization
• MVC Separation of concerns. Model doesn't get mixed with axum

Cons

• Ugly Phantomdata gets extracted too
• No runtime configuration. What you compile is what you will get
• No internal state. Because we never instantiate the strategy, there is no internal state
• One more trait: Not sure if it could be replace with an std trait
• Requires std::error::Error
• The type gets a bit verbose. Type alias should fix this. type ApiWithStratety<E> = WithStrategy<E,MyRejectStrategy>

[jplatte]

I like that, I think we can easily add it to axum-extra, although I think it we should name things differently, and not impose a std::error::Error constraint (it can still exist on an impl like you showed even if it the trait isn't constrained):

struct WithRejection<E, R>(pub E, pub PhantomData<R>);

impl<E, R> WithRejection<E, R> {
    fn into_inner(self) -> E { self.0 }
}

// impl Deref, DerefMut

#[async_trait]
impl<B, E, R> FromRequest<B> for ExtractWith<E, R>
where
    E: FromRequest<B>,
    E::Rejection: Into<T>,
{
    type Rejection = R;

    async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
        match req.extract::<E>().await {
            Ok(v) => Ok(ExtractWith(v, PhantomData)),
            Err(r) => Err(r.into()),
        }
    }
}

[Altair-Bueno]

I think it we should name things differently

Yea im terrible at naming things haha

and not impose a std::error::Error constraint (it can still exist on an impl like you showed even if it the trait isn't constrained)

Good catch!

[davidpdrsn]

Thats cool! Adding something like that to axum-extra gets a 👍 from me.

[Altair-Bueno]

Is anyone working on this? I can open a pull request with this change, if you are okay with it.

I only have one small concern with @jplatte version, regarding impl Deref and DerefMut. As stated by its docs:

On the other hand, the rules regarding Deref and DerefMut were designed specifically to accommodate smart pointers. Because of this, Deref should only be implemented for smart pointers to avoid confusion. ^1

I think AsRef and AsMut are the correct traits

[davidpdrsn]

Go for it!

I think it should implement Deref and DerefMut, like the other extractors in axum. Afaik that guideline is generally considered not best practice anymore. I believe even std itself violates it in a few places (don't remember where)

[jplatte]

Hehe yeah, the rules for Deref and DerefMut are not really fully settled yet. A while ago (should be fixed now), there was actually a circular definition kind of situation, with some official docs saying that Deref and DerefMut are (only) supposed to be implemented by "smart pointers" without defining that, and a different part of official docs defining smart pointers as types that implement Deref¹ 😄

¹ I find this definition quite absurd, it doesn't even match my intutition for "smart pointer" even for basic examples such as Vec, but Cow and AssertUnwindSafe are worse offenders for not even being pointers IMO (so how can they be smart pointers)

[Altair-Bueno]

Thanks for the explanation! I will look up more info about this topic, looks interesting

[Altair-Bueno]

I've been playing a bit more with WithRejection today and I think i've discovered a nice feature that I believe is worth mentioning somewhere.

Because WithRejection uses From<E::Rejection>, it is possible to use the thiserror crate to transform extractor rejections the same way it is used to transform Err variants. This means that you can now write one single enum and handle all rejections with it!

use axum::http::StatusCode;
use axum::response::IntoResponse;
use axum::routing::post;
use axum::*;
use axum_extra::extract::WithRejection;
use chrono::{DateTime, Utc};
use serde::Serialize;
use serde_json::Value;

async fn handler(
    WithRejection(json, _): WithRejection<Json<Value>, ApiError>,
) -> impl IntoResponse {
    json
}

#[tokio::main]
async fn main() {
    let app = Router::new().route("/", post(handler));

    Server::bind(&([127, 0, 0, 1], 8080).into())
        .serve(app.into_make_service())
        .await
        .unwrap()
}

// Defines all the possible error responses from the API, including extractor responses
#[derive(thiserror::Error, Debug)]
#[non_exhaustive]
enum ApiError {
    #[error(transparent)]
    JsonDeserialization(#[from] axum::extract::rejection::JsonRejection),
    // More extractor errors and API errors
}

#[derive(Debug, Serialize)]
struct ApiErrorPayload {
    message: String,
    timestamp: DateTime<Utc>,
    docs: String,
}

impl IntoResponse for ApiError {
    fn into_response(self) -> response::Response {
        let now = Utc::now();
        let (code, value) = match self {
			// Some sample responses you might see on production REST apis
            Self::JsonDeserialization(x) => (
                StatusCode::BAD_REQUEST,
                ApiErrorPayload {
                    message: x.to_string(),
                    timestamp: now,
                    docs: "https://example.org/errors#JsonDeserialization".into(),
                },
            ),
            _ => (
                StatusCode::INTERNAL_SERVER_ERROR,
                ApiErrorPayload {
                    message: "Unknown error".into(),
                    timestamp: now,
                    docs: "https://example.org/contact".into(),
                },
            ),
        };
        (code, Json(value)).into_response()
    }
}

Cargo.toml for anyone interested on trying this now

[package]
name = "sample"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
axum = { git = "https://github.com/tokio-rs/axum", rev="fb215616" }
axum-extra = { git = "https://github.com/tokio-rs/axum", rev="fb215616" }
axum-macros = { git = "https://github.com/tokio-rs/axum", rev="fb215616" }
tokio = { version = "1.20.1", features = ["full"] }
serde = { version = "1.0.143", features = ["derive"] }
serde_json = "1.0.83"
thiserror = "1.0.32"
chrono = { version = "0.4.22", features = ["serde"] }

Imho we could close this issue for now, given how ergonomic/powerful combining WithRejection + thiserror is.

Maybe we should include a link to this comment somewhere on the docs so people can find this example easily?

[davidpdrsn]

Yeah thats pretty cool. Takes most of the boilerplate out of it.

Imho we could close this issue for now, given how ergonomic/powerful combining WithRejection + thiserror is.

I think we should keep it open. I don't think having to write in all your handlers WithRejection(Json(person), _): WithRejection<Json<Person>, MyRejection> is ideal. Its quite foreign looking syntax, even though it does work.

Ideally const generics would be more powerful so we could do Json(person): Json<Person, MyRejection> and type Json<T> = axum::Json<T, MyRejection> but thats not possible today, hence the S-blocked label.

[davidpdrsn]

@Altair-Bueno however I think we can probably extend https://github.com/tokio-rs/axum/blob/main/examples/customize-extractor-error/src/main.rs to show the three solutions:

• Manually making a new extractor, i.e. what the example currently shows
• Using #[derive(FromRequest)] like shown here
• Using WithRejection perhaps combined with thiserror.

I think if we do that we can remove the 0.6 milestone from this issue. Not sure we have many other options and I'm okay with these options.

Do you wanna work on that?

[Altair-Bueno]

Do you wanna work on that?

Sure!

however I think we can probably extend main/examples/customize-extractor-error/src/main.rs to show the three solutions:

You mean one crate per solution, right? We are gonna need some good self-explanatory names

While we are at it, we might also refactor some examples to make things easier:

• customize-extractor-error and customize-path-rejection: Choose one of them and rename it to custom-extractor(?). I don't think is worth it to maintain those two at the same time, they are too alike.
• error-handling-and-dependency-injection: Break down this example
  • error-handling(?): Using thiserror, WithRejection and some custom error. Include links to custom-extractor and derive-from-request with small summary highlighting the cons and pros of each approach
  • dependency-injection: Just the dependency injection
• derive-from-request(?): An example showcasing #[derive(FromRequest)], including the error handling feature

[davidpdrsn]

You mean one crate per solution, right? We are gonna need some good self-explanatory names

I don't think we need three separate crates. We can just do three modules in one crate.

[davidpdrsn]

I don't think we should change other examples.

[Altair-Bueno]

I don't think we should change other examples.

Roger, i've created another crate for #1276. I still think there is too much overlapping between examples to my taste, but is something we judge later

[davidpdrsn]

With https://github.com/tokio-rs/axum/pull/1276 merged (thanks @Altair-Bueno) I think we can remove this from the 0.6 milestone.

[heat1q]

I've been following this issue for a while now I must say that the options for implementing custom rejections already improved greatly! In particular WithRejection and #[derive(FromRequest)] make it much easier to use custom API errors as rejections.

What I am still missing (or rather what would be nice to have) is a way to derive a From<Rejection> for my custom API error. It's really cool that you can already do this with thiserror when your error is an enum, but you still have to manually deconstruct the rejection if you want to use a different structure.

Often you simply want to take whatever status and body the axum rejection has and put it in your custom type. The axum rejections already categorizes the different error cases nicely (status code and message) and you might want to keep them as is.

Based on this example the derive signature could look something like

#[derive(FromRejection)]
#[from_rejection(via(JsonRejection))]
struct ApiError {
    #[rejection(status_code)]
    code: StatusCode,
    #[rejection(message)]
    message: String,
}

where you simply tell where the rejection status_code and message (aka body_text) should go. Similar to #[derive(FromRequest)] one could also add #[from_rejection(via(...),...) to specify for which rejection a From implementation should be derived.

I've already implemented a rough draft of this derive macro and found it quite handy. Now I'm wondering if this would also be of use to others and included into axum-macros. What do you think?

[Altair-Bueno]

You don't have to deconstruct the rejection, just use a struct instead of an enum and impl From<RejectionType> for MyCustomRejection

If you still want to go the macro route, i think a declarative macro is better suited as they are more performant and easier to read

Full cargo project

pub async fn handler(
    WithRejection(Json(value), _): WithRejection<Json<Value>, CustomRejection>,
) -> impl IntoResponse {
    Json(dbg!(value))
}

#[derive(Debug)]
pub struct CustomRejection {
    message: String,
    code: StatusCode,
}

// We implement `IntoResponse` so ApiError can be used as a response
impl IntoResponse for CustomRejection {
    fn into_response(self) -> axum::response::Response {
        let payload = json!({
            "message": self.message,
            "origin": "with_rejection"
        });
        (self.code, Json(payload)).into_response()
    }
}

// Manual impl
impl From<JsonRejection> for CustomRejection {
    fn from(value: JsonRejection) -> Self {
        Self {
            code: value.status(),
            message: value.body_text(),
        }
    }
}

// Generate a bunch with a simple declarative macro, cheaper than proc_macro
macro_rules! gen_from_rejection {
    ($from:ty, $rejection:ty ) => {
        impl From<$from> for $rejection {
            fn from(value: $from) -> Self {
                Self {
                    code: value.status(),
                    message: value.body_text(),
                }
            }
        }
    };
}

gen_from_rejection!(FormRejection, CustomRejection);
gen_from_rejection!(BytesRejection, CustomRejection);

Another solution (that would require breaking changes to axum itself) would be to introduce another trait (AxumRejection) that would make the following code possible. This would leverage monomorphization instead of macros

trait AxumRejection {
    fn status(&self) -> StatusCode;
    fn body_text(&self) -> String;
}

// Manual impl
impl<T: AxumRejection> From<T> for CustomRejection {
    fn from(value: T) -> Self {
        Self {
            code: value.status(),
            message: value.body_text(),
        }
    }
}

Note: IntoResponse cannot be used as the compiler yields "conflicting implementations of trait From<CustomRejection> for type CustomRejection"

As a final note, I strongly believe that proc/derive macros shouldn't be used unless you have a really good reason. And even then, it's probably a bad idea.

[heat1q]

Thanks for the answer, really appreciate the feedback. I understand the cost of proc macros, it was just an idea I wanted to explore. I like your suggestion of a simple declarative macro that I can easily apply to my use cases.

[davidpdrsn]

As a final note, I strongly believe that proc/derive macros shouldn't be used unless you have a really good reason. And even then, it's probably a bad idea.

While I don't agree with the sentiment that proc-macros are always a bad idea (#[derive(Serialize, Deserialize)] 👀) I do agree that we probably don't need a proc-macro for this.

[saiintbrisson]

I'm unsure if this has come up yet, but I've added the rejection as a response extension with about 15 lines of code, just changing how the define_rejection macro implements IntoResponse. No clones or copies were required, nor were any checks.

It allows the developer to customize it however it wants if it wants to. My company is switching to Axum and having a global way to handle rejections is a decently important thing for us.

I'll open a PR to gather comments and opinions about it, but here's a general idea:

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
}

[kurtbuilds]

Out of the 3 options for customizing errors in the examples (https://github.com/tokio-rs/axum/blob/main/examples/customize-extractor-error/src/main.rs?rgh-link-date=2022-08-18T12%3A26%3A54Z), I went with #3.

However, the examples appear out of date. (Were they written for axum 0.5?)

Here's the updated example:

pub struct Json<T>(pub T);

// optional, but I find this helpful.
impl<T> std::ops::Deref for Json<T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

#[async_trait]
impl<T, S, B> FromRequest<S, B> for Json<T>
    where
        T: DeserializeOwned,
        B: HttpBody + Send + 'static,
        B::Data: Send,
        B::Error: Into<BoxError>,
        S: Send + Sync,
{
    type Rejection = ErrorResponse;

    async fn from_request(req: Request<B>, state: &S) -> Result<Self, Self::Rejection> {
        match axum::Json::<T>::from_request(req, state).await {
            Ok(axum::Json(value)) => Ok(Self(value)),
            Err(rejection) => Err(ErrorResponse {
                code: rejection.status(),
                error: "Client Error".to_string(),
                message: rejection.to_string(),
            })
        }
    }
}