Handling Errors in Route Handlers

[mgattozzi]

Feature Request

It would be great if we could handle errors, such as being unable to get a database connection, with a route of return type Result<Response, MyErrorType>.

Detailed Description

Lot's of things need I/O in order to do things, such as database connections! The common pattern in Rust is to use ? to pass the error up the stack of function calls and then handle it at a single point. My idea is something like this:

1. User requests something from an endpoint
2. The endpoint calls out to the database to grab stuff, but this doesn't work because the Database is down.
3. The call to get a connection fails and gets passed up to the route handler.
4. Middleware for handling the error then takes the error and then the user handles it (whether it be logging or whatever is up to them). They then send a response back (like 500 or whatever leave it up to the user).

This feels a bit more ergonomic to me and was the first thing I tried to do upon trying out Tide.

Context

It's important, because I want to write code that feels like Rust. I don't want to twist my code to fit a single framework. This feels counter intuitive. In fact this was the thinking behind async/await. Rust code that looks synchronous, but just happens to be asynchronous.

I think this would be a great ergonomics win and would likely be something they expect to happen and from my talk with @yoshuawuyts at the Berlin All Hands (2019) this seems something they are inline with.

Possible Implementation

I think I covered a general idea above, but I don't have much else to flesh this out more

[petejodo]

My understanding is that this is possible as long as MyErrorType implements intoResponse

Edit: I see what you meant. By the time you get to handle the error in the middleware layer, it's already a response, losing import context for error logging and such

[yoshuawuyts]

I've been thinking about error handling a bunch, and I think there are generally two things people might want to do:

1. attach http status codes & error messages to existing errors
2. create new errors with status codes

There's probably people that will want to do this structurally (analogous to implementing custom structs with failures' Fail trait), but many others will be happy to create errors on the fly (failure's bail! and ensure! macros), and add upgrade existing errors to add context (failure's Context struct and context method).

I think we could follow in the footsteps here of koa's context.throw / context.assert, and include it for Tide:

API Sketch

Shorthands to create new errors

fn foo(req: Request, cx: Context) ->Result<TryInto<Response>, _> {
    // create a new error, and return manually
    return Err(http_format_err!(500, "oh no"));

    // quickly throw a new error, equivalent to failure's bail (which was inherited from error-chain)
    http_throw!(500);
    http_throw!(500, "name required");

    // non-panic assertion, equal to failure's ensure!
    http_ensure!(400, cx.foo, "please login");
}

Manually add context to an error

fn foo(req: Request, cx: Context) -> Result<TryInto<Response>, _> {
    // only include a status code, no error message
    foob.parse()
        .http_status(400)?;

    // include a status code and error message
    foob.parse()
        .http_context(500, "Server error")?;
}

Use context groups to add context in bulk

/// Parse a foo.
#[http_context(500, "Server error")]
fn foo(req: Request, cx: Context) ->Result<TryInto<Response>, _> {
    let bar = foo.parse()?;

    #[http_context(400, "Accessing the blargs")]
    try {
        bar.baz()?.beep()?.boop()?
    }?
}

---

This would change our model to not try and coerce everything to a Response inside the handlers, but would allow us to use Try operators throughout, and still be able to deliver valid HTTP responses when returning Result::Err.

This also ties into @prasannavl's point made in #156 (comment) I think. I think this model would probably work best to then have a final request handler that can convert these errors into valid response types. Though I haven't thought it through how that one works already, so I'll hold off here.

[aturon]

The revamp in #156 added an error module that provides a bunch of goodies along these lines!

Here were my goals:

• Make it easy to use ? in endpoints
• Make it possible for middleware to analyze errors
• Make it easy to control the exact response being generated, including the status code

Here's what the revamp does to achieve these goals:

The tide::error::Error type provides a generic error type that most endpoints can use. In addition, EndpointResult is a handy alias to bake this in.

But we don't provide direct conversions from arbitrary errors to tide errors. Instead, we have the following extension methods on any Result value:

/// Extends the `Result` type with convenient methods for constructing Tide errors.
pub trait ResultExt<T>: Sized {
    /// Convert to an `EndpointResult`, treating the `Err` case as a client
    /// error (response code 400).
    fn client_err(self) -> EndpointResult<T> {
        self.with_err_status(400)
    }

    /// Convert to an `EndpointResult`, treating the `Err` case as a server
    /// error (response code 500).
    fn server_err(self) -> EndpointResult<T> {
        self.with_err_status(500)
    }

    /// Convert to an `EndpointResult`, wrapping the `Err` case with a custom
    /// response status.
    fn with_err_status<S>(self, status: S) -> EndpointResult<T>
    where
        StatusCode: HttpTryFrom<S>;
}

This leads to endpoint code like the following:

fn foo(cx: Context<MyData>) -> EndpointResult {
    // if the id in the URL can't parse to a u64, that's the client's fault
    let id: u64 = cx.param("id").client_err()?;

    // if the body can't be read, or it can't deserialize as json, that's the client's fault
    let data: Whatever = await!(cx.body_json()).client_err()?;

    // if the database falls over, that's a server error
    cx.app_data().database.do_a_thing(id, data).server_err()?;
}

I feel pretty good about this approach so far, and I think it handles the original concerns @mgattozzi was raising, but I'd love to hear what y'all think!

[yoshuawuyts]

@aturon something I'm still missing in the current state is to provide:

1. error messages; knowing what went wrong and why, particularly with validation, is super useful
2. have custom formatters for the errors. If for example Stripe decides to use Tide, they'll likely want to return errors with a different (JSON) formatting then when Zalando uses it.

Also I'm not entirely confident in the mapping of "client_err" to 400, and "server_err" to 500. They're indeed errors in that category, but for example so are 403 and 402 error codes.

What I've seen in Go in the past is using the full error name as the method handle. E.g. being able to do .bad_request() instead of .client_err() or .with_err_status(400). I don't know if that's ideal, but it might work nicely.

Refs

• Stripe API Reference -- I find this to be a useful reference. If we can implement what they need conveniently, we're probably in an okay spot.

[mgattozzi]

I agree with @yoshuawuyts that having the error automatically 400 or 500 is a bit limiting. Being able to pass in a value like 404 would be ideal, or even better a set of consts representing those numbers to let users use both (i.e. NOT_FOUND or 404).

I will say I like this approach for methods to map things to end point errors in order to turn them into something that works inside tide as an error. Being able to provide context for the error would be good, like we got a 500 error cause we couldn't get a connection to the database.

[jonathanKingston]

Maybe it makes sense to handle this requirement at the IntoResponse implementation in the result type for the error returned?

I'm already handling HTML this way to prepend a HTML header for example.

pub struct HTMLResult {
    status: StatusCode,
    //body: DOMTree<String> isn't send
    body: String,
}

impl HTMLResult {
    pub fn ok(body: DOMTree<String>) -> HTMLResult {
        let body = format!("{}", body);
        HTMLResult {
            status: StatusCode::OK,
            body,
        }
    }
    pub fn client_error(body: DOMTree<String>) -> HTMLResult {
        let body = format!("{}", body);
        HTMLResult {
            status: StatusCode::BAD_REQUEST,
            body,
        }
    }
}

impl IntoResponse for HTMLResult {
    fn into_response(self) -> Response {
        let head = r#"<!DOCTYPE html>
        <html lang="en">
          <head>
            <script src="/static/app.js"></script>
          </head>
          <body>"#;
        let foot = r#"</body></html>"#;
        let html = format!("{}{}{}", head, self.body, foot);
        http::Response::builder()
            .status(self.status)
            .header("Content-Type", "text/html; charset=utf-8")
            .body(Body::from(html))
            .unwrap()
    }
}

However I currently have to hand craft the param handling which is getting a little error prone:

async fn maybe_product_page(cx: AppContext) -> HTMLResult {
    let path: String = cx.param("path").unwrap();
    let num = path.parse::<usize>();
...

In @aturon's example I could just:

async fn maybe_product_page(cx: AppContext) -> HTMLResult {
    let path: usize = cx.param("path").client_error()?;
...

So I would then:

impl From<ClientError> for HTMLResponse {
    fn from(error: ClientError) -> Self {
        HTMLResponse::client_error(html!{
          <h1>"oh noes!"</h1>
          <div>{error.reason}</div>
        });
    }
}

With an optional:

async fn maybe_product_page(cx: AppContext) -> HTMLResult {
    let path: usize = cx.param("path").client_error_code(StatusCode::NOT_FOUND)?;
...

Either way I agree that param handling is a point of contention.