Better documentation/discussion for error handling

[prabirshrestha]

Would it be possible to add error handling example in https://trillium.rs/ ?

Doc are great by the way for a new project.

[jbr]

Doc are great by the way for a new project.

Thanks!

The absence of error handling is not entirely a mistake, but I should address it directly in the documentation. Currently, the error handling story is the least-developed aspect of Trillium, but we are limited by what the language can express at the moment. In the future, we'll be able to have a

trillium_smol::run(ErrorHandler::new(async move |conn: &Conn| -> Result<(), MyError> {
    Ok(())
}));

… but until the language develops the capability to express the bounds for for<'a> Fn(&'a Conn) -> impl Future<Output=…> + 'a, it's a matter of picking the least-bad workaround, and that's going to end up being outside of trillium core.

There are two macros available to make this easier: trillium::conn_try! and trillium::conn_unwrap!, but I expect there'll be further improvements.

Regardless of how it turns out, error handling is going to end up looking very different from tide. In particular, I think there's a distinction between "returning a non-200 response" and a rust Err, and that the code to translate between them should live entirely in the application. I think most applications will want to have a single thiserror-style enum Error that also implements Handler.

[prabirshrestha]

I was actually looking into some sort of error handling sort of middleware (handler) that auto converts errors to json/html bad requests or 500 or thiserror to some json represntation.

trillium_smol::run(
    Router::new()
        .get("/users", {conn-> Err('blah') ))
        .get("/posts", posts_api),
    ErrorHandler::new(async move |conn: &Conn| -> {
        conn.with_status(500).with_body(json!(conn.err()))
    });
);

[jbr]

It's important to note in this example that .get("/users", |conn: Conn| async move { Err("blah") }) is not how trillium works. Trillium handlers must always return Conn. That's a core aspect of the framework, and is essential to the underlying implementation. If you don't like that approach, tide is probably a better fit for your code

[happysalada]

One question I have regarding error handliing is that if possible I would want to add something like
error!(error.into()) somewhere to actually log the errors on the server.
I don't know if this is possible, but enabling logging would be nice.

[prabirshrestha]

I see that you have error-handling branch. Curious if you have plans to release it? Might be with macro. Might be macros with support for chaining with_* methods could help here too.

#[handler]
async fn hello_world(ctx: &mut ctx) -> Result<(), MyError> {
    ctx.with_status(200).with_body("Hello World");
    Ok(())
}

[prabirshrestha]

I tried a proof of concept and it does seem to work.

Instead of creating a new trait to handle errors we could reuse the same trillium::Handler. This would align rest of trillium where middleware are just handlers in trillium

pub enum MyError {
    FooError,
}

#[async_trait]
impl Handler for MyError {
    async fn run(&self, conn: Conn) -> Conn {
        conn.with_status(500).with_body("Internal Server Error")
    }
}

For the following code.

#[handler]
async fn hello(conn: &mut Conn) -> Result<(), MyError> {
    // conn.set_status(200)
    // conn.set_body("Hello")
    // Ok(())
    Err(MyError::FooError)
}

One could generate the following code via macro.

struct hello;

#[async_trait]
impl Handler for hello {
    async fn run(&self, mut conn: Conn) -> Conn {
        async fn hello(conn: &mut Conn) -> Result<(), MyError> {
            // conn.set_status(200)
            // conn.set_body("Hello")
            // Ok(())
            Err(MyError::FooError)
        }
        match hello(&mut conn).await {
            Ok(_) => conn,
            Err(e) => e.run(conn).await,
        }
    }
}

[jbr]

I would happily review a crate that does this. I'm not personally a fan of macros like this, so I don't intend to write it. The error handler trait is currently blocked on rust adding AsyncFn traits so that we can describe something like for<'a> Fn(&'a mut Conn) -> Future<Output=Result<(), SomeError>> + 'a — GATs are a step in this direction, but not the full story.

[prabirshrestha]

I have published the trillium-error repo. Feel free to send feedback/PRs or suggestions for crate name. I haven't published it yet to crates.io

[prabirshrestha]

I have published trillium-error crate.

• repo: https://github.com/prabirshrestha/trillium-error
• crates: https://crates.io/crates/trillium-error
• docs: Usage can be found at crates at https://docs.rs/trillium-error/latest/trillium_error/

[jalcine]

This crate effectively produces a lot of what I've been doing in my collection of web stuff using trillium (but instead of doing this like this, I have something like the following):

pub trait ErrorRenderer: Clone + 'static {
    fn render_error<R: RenderableErrorExt>(&self, conn: Conn, error_value: R) -> Conn
    where
        Self: Sized;
}

#[derive(Clone, Debug)]
pub struct ErrorRendererState<T: ErrorRenderer>(Arc<T>);

impl<T: ErrorRenderer> ErrorRendererState<T> {
    pub fn new(state: T) -> Self {
        Self(Arc::new(state))
    }
}

impl<T: ErrorRenderer> ErrorRendererState<T> {
    fn render_error<E: RenderableErrorExt>(&self, conn: Conn, error_value: E) -> Conn {
        self.0.render_error(conn, error_value)
    }
}

#[derive(Clone)]
pub struct DefaultErrorRender {}

impl ErrorRenderer for DefaultErrorRender {
    fn render_error<R: RenderableErrorExt>(&self, mut conn: Conn, error_value: R) -> Conn
    where
        Self: Sized,
    {
        match conn.requested_content_type() {
            ContentType::Json => error_value.into().send_as_response(conn).halt(),
            _ => {
                let ctx = conn.context_mut();
                let resolved_context = tera::Context::from_value(error_value.error_json()).unwrap_or_default();
                ctx.extend(resolved_context);
                conn.render(&error_value.template_path()).with_status(error_value.error_status_code()).halt()
            }
        }
    }
}

pub trait RenderableErrorExt: Into<super::Response> + std::error::Error {
    fn default_template_path() -> String;
    fn template_path(&self) -> String;
    fn error_status_code(&self) -> Status { Status::InternalServerError }
    fn error_json(&self) -> serde_json::Value {
        serde_json::json!({})
    }
}

pub trait ErrorRenderingConnExt: NegotigateContentConnExt {
    fn resolve_error_presenter<ER: ErrorRenderer>(
        &self,
    ) -> Option<ErrorRendererState<ER>>
    where
        Self: Sized;

    fn render_error<ER: ErrorRenderer>(self, error: impl RenderableErrorExt) -> Self where Self: Sized;
}

impl ErrorRenderingConnExt for Conn {
    fn resolve_error_presenter<ER>(
        &self
    ) -> Option<ErrorRendererState<ER>>
    where
        Self: Sized, ER: ErrorRenderer
    {
        self.state::<ErrorRendererState<ER>>().cloned()
    }

    fn render_error<ER: ErrorRenderer>(self, error: impl RenderableErrorExt) -> Self where Self: Sized {
        if let Some(presenter) = self.resolve_error_presenter::<ER>() {
        presenter.render_error(self, error)
        } else {
            (DefaultErrorRender {}).render_error(self, error)
        }
        
    }
}

Such that I can write code like this (from https://indieweb.org/Shock, for example):

pub async fn redeem(mut conn: Conn) -> Conn {
    let app: Arc<Application> = conn.state().cloned().unwrap();
    trace!("Attempting to begin to redeem a IndieAuth token.");

    // TODO Add Into<T> for error type to simplify this
    let params = safely_fail_conn!(
        conn,
        qs::from_str::<RedeemParameters>(conn.querystring()).map_err(|e| {
            crate::Error::System(common::Error::Serialization(
                common::error::SerializationError::QueryString(e),
            ))
        })
    );

   /// .... do some work

   conn
}

My hope is to use a singular macro that'd do the handling but still have facilities to overwrite the underlying functionality (so I can change how the error is responded to, do content negotiation, etc) without to create structs to represent a request.