Improve error handling user experience.

[PaulGrandperrin]

Motivation

I started using axum recently (after warp and poem) and I really like its "style", but I was really surprised to see that the project doesn't natively support returning errors from handlers.

I saw the examples code in the source code and the documentation and there are also a few crates available and code snippets on Reddit, but I wasn't happy with them.

I feel that one solution can be good and generic enough to become officially part of axum.

This would reduce friction for new axum users and prevent them for copy-pasting suboptimal code.

Poem handlers can return Poem::Result<T> and it's really nice.

Solution

I wanted to keep things as axumy as possible without chaning any existing API or add any dependencies.

Handlers currently typically return impl IntoReponse so my idea was to be able to change that to impl IntoResultResponse (I don't like Result<impl IntoResponse, MyCustomError>) and then be able to use ? with any kind of errors in the function.

Errors from std, anyhow, eyre etc are all supported and the return type will be converted into a Result<impl IntoResponse, ErrorResponse> where ErrorResponse implements IntoResponse.

ErrorResponse just encapsulates the original error with an associated StatusCode (by default INTERNAL_SERVER_ERROR) and is then converted to (self.status, format!("{:?}", self.error)).

To be able to easily customize the StatusCode I also added a Result extension trait ResultExt that implements Result<T>::err_with_status(self, status: StatusCode) -> ResultResponse<T>.

Resulting user experience

async fn handler() -> impl IntoResultResponse { // instead of IntoResponse
    try_thing_anyhow()?; // by default this will return a StatusCode::INTERNAL_SERVER_ERROR (500) error
    try_thing_anyhow().err_with_status(StatusCode::BAD_REQUEST)?; // Using the `ResultExt` trait to return a StatusCode::BAD_REQUEST (400) error

    try_thing_stderror()?; // Standard errors also work
    Ok(())
}

fn try_thing_anyhow() -> Result<(), anyhow::Error> {
    anyhow::bail!("it failed!");
}

fn try_thing_stderror() -> Result<(), impl std::error::Error> {
    Err(std::fmt::Error::default())
}

Status of PR

Since this PR add some new elements to the API I don't expect it to be readily accepted.
I therefor skipped many things that should be done if it were to be accepted:

• update all relevant doc
• think more about trait bounds (like Send, Sync and 'static)
• integrate better with the project conventions

[jplatte]

I'll think about it but my initial reaction is that this just adds more noise to the public API without much of an actual benefit. Note that instead of Result<impl IntoResponse, MyCustomError>, you can also do

type MyCustomResult<T> = Result<T, MyCustomError>;

or

type Result<T, E = MyCustomError> = std::result::Result<T, E>;

and write -> MyCustomResult<impl IntoResponse> / -> Result<impl IntoResponse>.

[PaulGrandperrin]

The idea is that I guess basically everyone wants to handle errors in handlers so this noise needs to exist somewhere.

Either:

1. It is duplicated into all axum user projects; and with the added cost of requiring all those users to:

• wonder why Results aren't supported by axum natively
• search online for answers
• decide on which code snippet to copy from
• integrate code, not specific to their project, into their code base

2. Or this noise is written once, well documented and integrated, and used by all.
   This spares users from even thinking about this as a problem in the first place.

My feeling is that reducing this kind of friction is what helps giving a good opinion when people try out and evaluate a new crate.

[PaulGrandperrin]

Yes, it's also already possible to write -> ResultResponse<impl IntoResponse> with this PR.

And IntoResultResponse could be removed to trim down on the added API surface.

[PaulGrandperrin]

To avoid extending axums API but still making users solve this problem as easily as possible, maybe it could be put into an official axum-error crate.

I think it's important that something easy and official exists to solve this common issue.

[jplatte]

wonder why Results aren't supported by axum natively

But they are.. And we also have axum::response::Result for quick & dirty error handling if you don't want to define your own error type.

[jplatte]

I guess the problem here is that I don't really understand your motivation / problem statement, or I didn't initially at least.

Is it right that you're looking to make it easier for people to return errors that don't implement IntoResponse? I could see us adding ErrorResponse::from_status_code_and_message(status_code: StatusCode, message: impl Display) and ErrorResponse::internal_server_error(message: impl Error) or somesuch. Then people could use .map_err(ErrorResponse::internal_server_error)?, but the from_status_code_and_message thing would still be pretty verbose to use. Not sure how useful it would be.

[PaulGrandperrin]

I think i went through a quite common struggle when I started using axum.

Very quickly, I needed to ? on functions returning results.

In my case, on a custom function returning an eyre::Result and another returning a reqwest::Result.

Neither types implement IntoResponse and I also can't implement it for them.

I'm therefore stuck.

I know I could come up with some boilerplate code to do the plumbing myself but it feels weird this use case is not taken care of already.

I start searching online and find lot of other people with the same issue and the official example code for anyhow.

Looking deeper, I also find a few semi-maintained crates with the boilerplate code I need.

I'm really not into adding single-unknown-maintainer dependencies but i have to make a choice.

I check all the boilerplate code I find and evaluate them. I also need to dig deeper into axum's types to really understand what I'm doing before adding this code to my project.

After about an hour (or more) I finally write my own custom types and traits, in my project, to solve the problem I encountered 5 minutes into using axum.

[PaulGrandperrin]

I think I went through all the links of the first page of Google result + the example folder in axum + quite many axum document pages, but i don't remember seeing or at least considering axum::Result.

Probably because it can't be used in the use case I described as it doesn't implement conversation traits from regular errors.

And, just like it's impossible to implement IntoResponse for anyhow::Result, it's impossible to implement From<StdError> for axum::Error.

So the user is forced to look for boilerplate code to copy paste as soon as he wants to use ?.

[PaulGrandperrin]

I guess the problem here is that I don't really understand your motivation / problem statement, or I didn't initially at least.

I hope my longer explanation will help!

Is it right that you're looking to make it easier for people to return errors that don't implement IntoResponse? I could see us adding ErrorResponse::from_status_code_and_message(status_code: StatusCode, message: impl Display) and ErrorResponse::internal_server_error(message: impl Error) or somesuch. Then people could use .map_err(ErrorResponse::internal_server_error)?, but the from_status_code_and_message thing would still be pretty verbose to use. Not sure how useful it would be.

Yes, that's exactly the problem.

I don't think forcing the user to prefix all its ? with .map_err(ErrorResponse::internal_server_error) is easy on the keyboard or easy on the eyes.

[jplatte]

I understand your pain, but there's no single obvious solution for turning an arbitrary error type into an http response.

Do you agree that the most common need is to return an internal server error? What do you think about my suggested solution, .map_err(ErrorResponse::internal_server_error)??

[PaulGrandperrin]

ErrorResponse::from_status_code_and_message(status_code: StatusCode, message: impl Display) and ErrorResponse::internal_server_error(message: impl Error)

Why not

impl<E: Into<Box<dyn Error>>> From<E> for axum::Error

?

That's the central piece that makes things work natively without having to use mappings.

[PaulGrandperrin]

I understand your pain, but there's no single obvious solution for turning an arbitrary error type into an http response.

I think that's exactly what this PR does.

Do you agree that the most common need is to return an internal server error? What do you think about my suggested solution, .map_err(ErrorResponse::internal_server_error)??

I'm not a fan because people would need to copy paste this snippet over and over again for no real value when this could be avoided.

axum::Error just needs to implement a trait to be automatically converted from any error by ?.

What the issue with making ? work directly?

I don't understand the downside of this solution.

I'll refactor this PR to remove ErrorResponse and ResultResponse and directly use axum's existing corresponding versions.

Then this PR won't add any new types, which is much better and I hope you'll reconsider 🙂

[jplatte]

I'm sorry, but making ErrorResponse convertible from any E: Error is not on the table. You can release your own crate that has a type like that, but we intentionally did not do it that way because implicitly producing internal server errors and exposing the underlying error message whenever a handler hits an error in some sub-routine is just bad form. I would consider a solution that is slightly less verbose than .map_err(ErrorResponse::internal_server_error), but nothing that makes it completely implicit.

[PaulGrandperrin]

I absolutely understand that, and I wouldn't do this on a service exposed to the web too.

That's why this PR didn't make it implicit by default (even though similar projects like poem does it by default).

In this PR, nothing changes by default, and no errors are implicitly converted.

One has to opt-in with impl IntoResultResponse or Result<impl IntoResponse, ErrorResponse> to get the implicit conversion.

The idea is that:

• by default, the errors are not implicitly converted to response (because IntoResponse doesn't change)
• but new users or people working on internal projects can easily opt-in the automatic conversion, without having to spend an hour or so looking for boiler place code or random crates online.

If that doesn't seem sensible to you, I won't bother refactoring this PR so please tell me!

[jplatte]

Hm, I don't think using ErrorResponse or something similar in the handler signature is a strong enough opt-in honestly.

Hope it's okay if I ping a few people for opinions again: @mladedav @yanns @tottoto @programatik29 @SabrinaJewson @lilyball (if you don't want to be pinged let me know)

[PaulGrandperrin]

Hm, I don't think using ErrorResponse or something similar in the handler signature is a strong enough opt-in honestly.

You're right, the name should probably be more a lot more explicit, like ErrorInBodyResponse or ExposedErrorResponseor maybe put an emphasis on this type being tailored for quick and easy handling, in the same spirit as anyhow.

[PaulGrandperrin]

In fact, when opting into this, there are three things that can be done when an error is returned:

1. print the error in the response body
2. trace the error with tracing
3. do both

This PR does 3. but probably it should only do 2.

This is probably what most people need anyway, and this way, there are no concerns with inadvertently leaking internal errors to clients.

For reference, poem does 3. by default and implicitly... And yes, to be honest, I was surprised and concerned by such a decision.

[jplatte]

Oh, I didn't even check the code yet, only your PR description. I actually think the exact opposite, that it should be returned but not logged. In my mind, this is purely a quick-prototyping thing. But that's certainly something that could be discussed more as well. I think it makes sense to create some issues from this PR soon, to have better places for the individual bits of the discussion. But I'm not completely certain yet what we need dedicates issues for :)

[yanns]

In fact, when opting into this, there are three things that can be done when an error is returned:

1. print the error in the response body

2. trace the error with `tracing`

3. do both

This PR does 3. but probably it should only do 2.

This is probably what most people need anyway, and this way, there are no concerns with inadvertently leaking internal errors to clients.

For me, print the error in the response body is something quite dangerous. Maybe I don't get the details here.
My typical error handler for non expected error:

1. create a correlation Id if there's none existing
2. trace the error and the correlation ID with tracing
3. send a 500 error with a generic response body like "there was an unexpected error. Please contact support with id {correlation_id}". I'd never write the error into the body.

Are you trying to cover this use-case of non-expected errors?

[PaulGrandperrin]

@jplatte Ohh ahah, indeed I think this should be split into many different bits.

Here's the different "subjects" I see:

1. Should implicit error conversions be provided by axum or left to the user to code himself?
2. How to opt-in? How explicit and verbose this opt-in should be?
3. How to handle the error? print in body, trace or both?
4. Should axum provide different ways to handle those errors, or just settle on one?

[PaulGrandperrin]

Are you trying to cover this use-case of non-expected errors?

Not in particular no, on the contrary, I liked the idea of being able to easily attach the StatusCode of my choice before ?ing the error.

Like:

async fn get_user(Path(user_id): Path<String>) -> impl IntoResultReponse {
  let u = Db::get(user_id).err_with_status(StatusCode::NOT_FOUND)?;
  ...
}

EDIT: this works thanks to the ResultExt trait from this PR.

[PaulGrandperrin]

And my first goal was to get the error in the logs, but I also find it convenient to get it back in the body when testing.

Returning errors in the HTTP body should probably be a "Debug"-only feature or something similarly gated away from normal usage.

[jplatte]

I've created #2671 for the potential improvements to the existing ErrorResponse type.

Regarding the ErrorResponse type from this PR, I wonder: Did you try this sort of error type in a few different places? I just realized that while it is convenient for using ? on methods that can return std::error::Errors, it does not work for types that implement IntoResponse but not std::error::Error, for example method_returning_option().ok_or(StatusCode::NOT_FOUND)? wouldn't work.

[mladedav]

My two cents. I personally like that I have to handle errors explicitly. I wouldn't want to make a mistake by putting a question mark somewhere it doesn't belong and not having axum reject the code.

I do believe any way to opt into something like that should be way more explicit like Result<impl IntoResponse, IntoInternalServerError>.

---

I'm not convinced that it is needed either though, if you need a quick and dirty way to prototype and get errors logged, why not just unwrap/expect? Panics are handled as an internal server error connection drop and if you're using stderr and don't change the panic handler, you get a textual representation of what happened.

[jplatte]

Actually, panics are handled as closing the tcp connection, unless you add a middleware that catches the panic and returns a response.

[PaulGrandperrin]

@mladedav removing the IntoResultResponse from this PR would satisfy what you mention.

The user would have to very explicitly write Result<impl IntoResponse, IntoInternalServerError>.

My goal is that IntoInternalServerError is provided by axum instead of being copy-pasted from the example code.

[PaulGrandperrin]

My feeling is that panics, even for testing releases, are very difficult to deal with in async code.

The "raison-d'etre" of this PR is very similar to anyhow, no as precise as a custom error, but way better than panics!

[Ptrskay3]

Just my 0.02$.

In my experience, this whole class of problem is solved by thiserror. We tend to use this pattern:

#[derive(thiserror::Error, Debug)]
enum MyError {
    #[error("an internal server error occurred")]
    Reqwest(#[from] reqwest::Error)
}

impl IntoResponse for MyError {
    fn into_response(self) -> axum::response::Response {
        match self {
             MyError::Reqwest(ref e) => {
                tracing::error!("Reqwest error: {:?}", e);
            }
     }
    // just a quick example, don't return 500 for everything in real code
     (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()).into_response()
}

and then look how cleanly it composes with axum handlers:

async fn handler() -> Result<String, MyError> {
    let content = reqwest::get("example.com").await?.text().await?;
    Ok(content)
}

You have the chance to log the error in the IntoResponse implementation.
I don't consider this boilerplate code at all — you can tailor this to your specific application explicitly, and don't need to fight with an existing implementation that's in the library.

[PaulGrandperrin]

@Ptrskay3 yes, that's basically what my PR is.

Here's the crux of the PR:

/// Here, we can wrap any kind of error, and also attach a `StatusCode` to it.
/// The StatusCode will be returned when the error happens.
#[derive(Debug)]
pub struct ErrorResponse {
  status: StatusCode,
  error: Box<dyn Error>,
}

// can be created from any errors, using the same technique as `anyhow` and `eyre`
impl<E: Into<Box<dyn Error>>> From<E> for ErrorResponse
{
  fn from(error: E) -> Self {
    Self::new(StatusCode::INTERNAL_SERVER_ERROR, error) // by default we use a 500 error
  }
}

// can be converted to Response
impl IntoResponse for ErrorResponse {
  fn into_response(self) -> Response {
    let error = format!("{:?}", self.error);

    #[cfg(feature = "tracing")]
    tracing::error!(error = %error); // we trace when tracing is enabled
     
    // this part should be behind an explicit flag for testing only
    (self.status, error).into_response()
    
    // for regular releases, it should do this:
    self.status.into_response()
  }
}

It's very similar to your code. The main difference is that my code handles all the errors, not just reqwest::Error.
Since my code is generic over any error, it doesn't need to be adapted for each and every new error types.

I know very well the debate between thiserror and anyhow, and so many discussion and blog post have been written about the advantages of one over the other.

I personally use both kind of error handling, sometimes custom errors, sometimes anyhow like errors.

Like many others, I think both have their place. dtolney, one of the most important contributor to the rust project and compiler, and who wrote anyhow thinks this way too.

I got the inspiration for this code from reading his source code in anyhow and also from eyre, like this: https://docs.rs/eyre/latest/src/eyre/error.rs.html#490-498

And finally, in avery similar project, poem, which I was using before, it works exactly like my PR does. You can return poem::Result<...> from your handlers and that works with any errors. No need to write anything particular.

[PaulGrandperrin]

I really got my fair share of people assuming I didn't know how to write custom errors or that I didn't know how to use thiserror.

I started using Rust shortly after the 0.6 release and saw the evolution of the std::Error trait and the RFCs and the history of error handling crates:

• error-chain
• custom_error
• failure
• anyhow
• thiserror
• eyre
• and more...

I wrote my own errors and used all those error handling crates too.

I probably read all the blog posts about error handling from, boat/dtolney, niko matsakis, yaahc, steve klabnik, aaron turon, carol nichols and many others, and in 10 years, I can tell you, that's a lot of them!

The path to finding a good error handling story for Rust has been very tumultuous in the first years after the 1.0 release.

I feel that I'm opening this almost 10 years old debate again now.

Many people in the rust community got burned out in those debates.

I just wanted to contribute something I wrote that I found useful, but yes, this PR does too many things at once, even though it was just a Draft.

I don't want to defend it anymore.

I shouldn't have spent that much time here already, I'll go back to other projects.

Sorry to have made everyone loose time here.

[mladedav]

I really am sorry if I or anyone else made you feel unwelcome or feel that your experience was unfairly dismissed.

I for one don't think the time was wasted at least for me because it did give me another perspective of how people use or want to use errors in handlers.

[PaulGrandperrin]

@mladedav Thanks for the kind words.

I didn't want to force people to use implicit error. I thought my PR reflected that. My PR didn't change any behavior by default.

All my code was opt-in.

I know rust developers often argue with people using "weaker" languages about how great rust explicitness is. But I'm a rust developer too.

Because of Rust, I stopped using C++ and forgot about it so quickly that I then couldn't find dev jobs because Rust wasn't popular yet.

This PR only wanted to ADD a feature, not change or remove anything.

For info, it's the same guy who wrote both thiserror (explicit handling) and anyhow (implicit handling), and he and others wrote many articles about why both techniques make sense. This guy is dtolnay, one of the most important contributor to rust and rustc.

[PaulGrandperrin]

One fun fact, in 2017, I applied to Google and got invited to their headquarters for the final 5 SWE+SRE on-site interviews.

By that time, I was already so into Rust, that I started forgetting about C++ but Rust wasn't popular yet so I had to do my interviews in C++.

I totally failed one of the interview because, with the stress, I totally forgot about C++ exceptions.
My head was absolutely stuck thinking about explicit error handling only.

I looked very stupid to the interviewers and because of this massive fail, it got me rejected even though my other interviews went well.

[jplatte]

My thanks for your contributions too. Even though we seem to disagree not just on the solution but also in some parts the problem(s) to solve here, I think something productive has already come out of this PR.

In case you haven't seen it, maybe you're interested in voicing your opinion on the alternatives listed in #2671 about improving the existing ErrorResponse. I also welcome you to create a new issue for discussion on other potential ways to make returning non-IntoResponse errors simpler.