viz-rs / viz

Fast, flexible, lightweight web framework for Rust
https://viz.rs
MIT License
341 stars 20 forks source link

Allow returning custom error types in `Result` #92

Open jonahlund opened 12 months ago

jonahlund commented 12 months ago

Currently it does not work to return a custom error type that implements IntoResponse, it expects the error type to be viz::Error

Example

use std::net::SocketAddr;
use viz::{IntoResponse, Request, Result, Router, Server, ServiceMaker};

#[derive(Debug, thiserror::Error)]
#[error("{message}")]
pub struct MyError {
    message: String,
}

impl IntoResponse for MyError {
    fn into_response(self) -> viz::Response {
        viz::Response::builder()
            .body(viz::Body::from(self.message.to_string()))
            .unwrap()
    }
}

async fn index(_: Request) -> Result<&'static str, MyError> {
    Ok("Hello Viz")
}

#[tokio::main]
async fn main() -> Result<()> {
    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    println!("listening on {addr}");

    let app = Router::new().get("/", index);

    if let Err(err) = Server::bind(&addr).serve(ServiceMaker::from(app)).await {
        println!("{err}");
    }

    Ok(())
}
error[E0271]: expected `impl Future<Output = Result<&str, MyError>>` to be a future that resolves to `Result<_, Error>`, but it resolves to `Result<&str, MyError>`
   --> src/main.rs:27:38
    |
27  |     let app = Router::new().get("/", index);
    |                             ---      ^^^^^ expected `Result<_, Error>`, found `Result<&str, MyError>`
    |                             |
    |                             required by a bound introduced by this call
    |
    = note: expected enum `Result<_, viz::Error>`
               found enum `Result<&'static str, MyError>`

Version 0.4.17

fundon commented 12 months ago

Thanks for the feedback.

It's not currently supported, an async function should return Result<T, Error> in Viz,

But we can make some adjustments:

impl From<MyError> for Error {
    fn from(value: MyError) -> Self {
        Error::Responder(value.into_response())
    }
}

async fn index(_: Request) -> Result<&'static str> {
    Err(MyError {
        message: "my error".to_string(),
    })?;
    Ok("Hello Viz")
}

async fn index_wrapped(req: Request) -> Result<&'static str> {
    Ok(index(req).await?)
}

let app = Router::new().get("/", index_wrapped);

poc

    let app = Router::new().get("/", index.map_custom_error(IntoResponse::into_response));
jonahlund commented 12 months ago

Yes that is indeed a possible solution However in the case that you would want full customization of the possible error messages, having your own error type that implements From<viz::Error> aswell as From<viz::PayloadError> can be very beneficial since it allows you to do things like this:

// custom data structure representing the error body
#[derive(Debug, thiserror::Error)]
pub struct MyError {
  message: String,
}

impl From<viz::Error> for MyError {
  fn from(value: viz::Error) -> Self {
    // return whatever makes sense for viz::Error
    Self {
      message: "Something went wrong. Please try again later".into()
    }   
  }
}

impl From<viz::PayloadError> for MyError {
  fn from(value: viz::PayloadError) -> Self {
    Self {
      message: value.to_string(),
    }
  }
}

impl IntoResponse for MyError { .. }

type ApiResult<T, E = MyError> = Result<T, E>;

async fn index(mut req: viz::Request) -> ApiResult<&'static str> {
  // since MyError implements From<viz::PayloadError> we can use '?'
  let form = req.form::<RegisterForm>().await?;

  Ok("Success")
}

Which gives you a lot of flexibility over your errors

fundon commented 12 months ago

Good idea. 👍

A lot of code needs to be redesigned(handler, router). I will try it. And PR is welcome!

fundon commented 9 months ago

I think we can add a trait TryHandler for this, likes TryFuture.

jonahlund commented 9 months ago

Axum's approach is to allow anything that implements IntoResponse to be returned from a handler, so it doesn't treat Result::Ok or Result::Err differently, they just have to implement IntoResponse, which provides maximum flexibility

fundon commented 9 months ago

https://viz.rs/en/0.4.x/concepts/error-handling

  1. Why not just return Response?

Since in a real-world scenario, you need to determine regular errors in files, IO, DB, etc., returning Result is the most appropriate. It is also possible to use ? operator to return an error early and respond to the client.

  1. Why not just return impl IntoResponse?

Although the IntoResponse feature has been implemented for Result, there is a special case if T = Result<R>, in which fatten is not stable, the result cannot be tied yet, so it cannot be returned properly.

I think, the compromise solution is to add a type Error; on Handler and return Result<Self::Output, Self::Error>.