Error Handling Examples #388
Comments
|
Hi, if I understand correctly you question, to return early with a custom enum MyError {
TeraError(tera::Error)
}
impl warp::reject::Reject for MyError {}
async fn index(tmpl: web::Data<tera::Tera>) -> Result<HttpResponse, Rejection> {
let html = tmpl
.render("index.html", &tera::Context::new())
.map_err(|err| warp::reject::custom(MyError::TeraError(err)))?;
Ok(HttpResponse::Ok().body(html))
}you can then convert this Rejection into a Reply using the |
|
I have/had the same question regarding the |
|
Rejections are meant to say a In the It may be worth adding some pattern to ease that? |
I think this is what I needed to read, I've apparently been going about this incorrectly the whole time.
I would definitely be interested in that, though I'm not sure what it would look like. I've only been using warp for a few weeks (tracking master pre-0.2), and I was initially relatively confused about what the proper way to deal with routes failing was. Rejections seemed silly since I wouldn't want to try the entire rest of the tree just to have it fail to match. Despite that, I've mistakenly (apparently; thanks for the clarification, the repo being small enough to watch fully has been helpful) found myself rejecting with Without rejections + let foo = match get_foo().await {
Ok(v) => v,
Err(e) => return Ok(some_error_response(e)),
}... would be very unpleasant, and even worse than In some ways it feels natural, in others silly, to have the route handler filters (by choice) be |
|
I'm also uncertain how one should handle filters required for a route? If I match a path and decide on its route, but need a database connection to handle the request, I don't want to pointlessly try matching other paths if I fail to acquire one; I want to send a 500 response immediately. A thorough explanation of how what kinds of errors should be and are handled would be very appreciated. |
|
I've been able to get the error handling working with a custom error type by converting my error to a Rejection in the handler and then converting it to a Response in the recover filter, but the ergonomics are not great. For instance, there is a lot of It seems like it's a common pattern to not return a Rejection once you get to the handler, and instead return error responses. It would be nice if there was a way to return a Result in the handlers that implements edit: Would it be possible to add an impl Reply for Result types in warp itself? Or maybe add a custom result type in warp for this? |
|
I've solved this by creating this use http::StatusCode;
use std::error::Error;
use warp::reply::Reply;
use warp::reply::{html, with_status, Response};
const INTERNAL_SERVER_ERROR: &'static str = "Internal server error";
pub struct ReplyResult<T, E>(Result<T, E>);
impl<T: Reply, E: Send + Error> Reply for ReplyResult<T, E> {
fn into_response(self) -> Response {
match self.0 {
Ok(x) => x.into_response(),
Err(e) => {
warn!("Filter failed: {}", e);
with_status(
html(INTERNAL_SERVER_ERROR),
StatusCode::INTERNAL_SERVER_ERROR,
)
.into_response()
}
}
}
}
impl<T, E> From<Result<T, E>> for ReplyResult<T, E> {
fn from(value: Result<T, E>) -> ReplyResult<T, E> {
ReplyResult(value)
}
}
#[inline]
pub fn catch_error<T, E>(result: Result<T, E>) -> ReplyResult<T, E> { ReplyResult(result) }It can be used by simply adding a single map with the function path: use std::io::{Error, ErrorKind};
let route = warp::path!("guess" / isize)
.map(|n| match n {
0 => Ok("Secret unlocked"),
_ => Err(Error::from(ErrorKind::Other))
})
.map(catch_error); |
|
Btw #458 implements Reply for Result in warp and I've been using it successfully in https://github.com/cjbassi/rust-warp-realworld-backend. |
I landed here looking for a pattern where I could use ?; to return an error from my handlers, but not go the rejection route, as there is no sense moving onto another handler, so some pattern to handle this would be very nice. |
|
To add to the other code examples heres: another technique that I've found useful is to create your own pseudo-try macro: macro_rules! warp_try {
($expr:expr) => {
match $expr {
Ok(val) => val,
Err(err) => {
return Ok(err.into_response());
}
}
};
}Then tips for usage:
The quickest of usage: pub async fn my_endpoint(
db: DbPool,
) -> Result<warp::reply::Response, std::convert::Infallible> {
let conn = db.get()
.await
.map_err(MyServerError::DatabaseInvalidConnection);
// Return a 500 error if the database pool is overly contended.
// MyServerError implements `warp::Reply`
let conn = warp_try!(conn);
// ... do something with the connection ...
Ok(warp::reply().into_response())
} |
|
My current solution is to use PR #458. It adds a .map_async in addition to implementing Reply if both sides of Result implement reply. With that just implement reply on your error (that logs and converts it into a user facing error message) and run the handlers with .map_async. Just change the warp line in your Cargo.toml to "warp = { git = "https://github.com/cjbassi/warp.git", branch = "error"}" to start using it now (no major feature losses for now, hoping for it to be merged before any occur). |
|
I agree with what has been mentioned before, it would be nice
Speaking from the experience of using warp for quite a while now (and I love it, great engineering work!). For almost all API routes, once I get past the let a = warp::post() // if this fails, continue with `b`
.and(path!("upload")) // if this fails, continue with `b`
.and(authenticated()) // if this fails, don't continue with any other filter
.and(warp::body::content_length_limit(1024 * 1024 * 10)) // if this fails, don't continue with any other filter
.and(warp::multipart::form()) // if this fails, don't continue with any other filter
.and_then(routes::upload); // if this fails, don't continue with any other filter
let b = warp::get()// ...
a.or(b)If Fortunately, I've found a way to achieve this with built-in filters utilizing the current state of rejects: let a = warp::post().and(
// Everything starting here will be covered by the `recover` below
path!("upload")
// return a custom error if the path did not match so we can uniquely identify this case
.or_else(|_| async { Err(reject::custom(PathMismatch)) })
.and(authenticated())
.and(warp::body::content_length_limit(1024 * 1024 * 10))
.and(warp::multipart::form())
.and_then(routes::upload)
// The recover will handle all rejects that happened starting with `path!`
.recover(recover),
);Here is a reduced version of my async fn recover(err: warp::Rejection) -> Result<impl warp::Reply, warp::Rejection> {
if err.find::<PathMismatch>().is_some() {
// This is the only case where we actually want to reject and continue with the other
// filters in the filter chain. Otherwise, the execution would stop at the first `path`
// that did not match the request.
Err(reject::not_found())
} else if let Some(ref err) = err.find::<error::ClientError>() {
// An example of a custom error type I am using to return structured errors
Ok(
warp::reply::with_status(warp::reply::json(err), StatusCode::BAD_REQUEST)
.into_response(),
)
} else if let Some(ref err) = err.find::<error::ServerError>() {
// Another custom error type, this time for server errors
log::error!("{}", err);
Ok(StatusCode::INTERNAL_SERVER_ERROR.into_response())
} else if err.find::<warp::reject::PayloadTooLarge>().is_some() {
// An example of converting a rejection of a built-in filter
// (`warp::body::content_length_limit` in this case) into a structured error
Ok(warp::reply::with_status(
warp::reply::json(&error::client_error(
"file_size",
"File size limit of 10MB exceeded",
)),
StatusCode::PAYLOAD_TOO_LARGE,
)
.into_response())
} else {
// My business logic always returns either one of my custom error types `ClientError`
// and `ServerError`, so I safely assume that all other rejects are due to filters that
// extract data from the request, which is why I simply return a 404 in those cases.
// It is important to return an `Ok` here, because again, no other filters should be
// executed afterwards.
Ok(StatusCode::NOT_FOUND.into_response())
}
}
#[derive(Debug)]
struct PathMismatch;
impl reject::Reject for PathMismatch {}Maybe this helps others to achieve their intended behaviour. |
The PR #458 delivers both these objectives in a nice and clear way. |
Yes indeed - there is only one thing the PR does not handle that I also wanted to achieve (but this was not highlighted in my bullet point list):
|
Would you be able to skip the or_else(... Path_Mismatch) and corresponding part of the recover() handler by instead pulling the path!() filter out of the inner filter chain? As in: let a = warp::post().and(path!("upload")).and(authenticated().and(...)), where recover() is at the end of that inner filter chain that begins with authenticated() -- rather than a chain straight from path!() to recover(). I agree this subject would be great for more examples. I've gone down the same road of building logic trees with more than two levels of or()/and(), with recover() at the end of the inner "leaf" filter chains. It works, yet that pattern seems not to be emphasized much in the docs/examples. It looks like users may have trouble with internal errors and rejections (and unwanted route re-matching attempts) by virtue of using two-level trees with a single top-level recover(). Wouldn't inner recover()s also mitigate some of the trouble behind #451 (the related issues in its comments)? I'm happy to contribute to examples if this understanding is considered correctly warpy. |
Yes, but only for routes where you don't have parameters inside the path. Once you have parameters, you need this part, so that the parameter correctly contributes to the |
Interesting--yeah when values are extracted from the path, how about: for each route, collapsing the filters into a single chain that ends in recover(handle_only_app_errors), letting warp's internal path rejection fall through to the outer or()? The per-route recover()s can handle only the rejections produced by the per-route filters, like PayloadTooLarge, parsing and custom errors, and path rejections fall out as Err(err). Would that also cover it? |
|
@luke-brown I am afraid this isn't working either, but I am not entirely sure that I understood you correctly. Here is an example of how I understood your example. While the following will work path!("users" / String / "picture")
.and(
authenticated()
.and(warp::body::content_length_limit(1024 * 1024 * 10)) // 10 MB
.and(warp::multipart::form())
.and(state.clone()),
)
.and_then(routes::upload::upload_users_picture)
.recover(recover)adding an additional recover to the non-path filters will not path!("users" / String / "picture")
.and(
authenticated()
.and(warp::body::content_length_limit(1024 * 1024 * 10)) // 10 MB
.and(warp::multipart::form())
.and(state.clone())
.recover(recover),
)
.and_then(routes::upload::upload_users_picture)
.recover(recover)This fails to compile. It looks like adding the recover |
|
@rkusa Here's what I'm thinking should work, annotated with your original comments on what you'd like it to achieve: With the upload-specific rejection handler doing this: With that setup, something like a POST /upload with the wrong content type will result in that 500 "unexpected upload error", while a request with a different path and/or method will fall through to the rest of the routes in the or() filter. It's essentially the same as your solution--I was only adding that I think it can be done this way without needing to add that custom PathMismatch rejection type: by attaching recover() to the filter chain that includes the path(), and handling every rejection in that chain except for the path() rejections. That other solution of moving everything after the path into a wrapped and() filter also seems nice, as long as it's possible to do so, i.e. whether the path() is extracting values. |
|
Oh.. if you might have other methods at the same path, then that works by also passing through errors matching |
@luke-brown I think you are right, that should work, too. To not continue trying other routes if your own handlers returns a 404, You'd only have to make sure to not use https://docs.rs/warp/0.3.0/warp/reject/fn.not_found.html in your own routes and handlers (but this can be replaced with a custom error that is alter converted to a 404 too). |
|
hello I'm trying to handle a reject for websocket upgrade: let subscription = warp::path::full()
.and(warp::ws())
.and(subscription_pools)
.and(subscription_db)
.map(|full_path: warp::path::FullPath, ws: warp::ws::Ws, pools: Pools, db: Storage| {
let key = String::from(rem_first_char(full_path.as_str()));
// TODO: return 400 on invalid key subscription
if key_valid(&key) {
ws.on_upgrade(move |websocket| ws_reader(websocket, key, pools, db))
} else {
let obj: WriteMessage = WriteMessage{
data: String::from("invalid key"),
};
warp::reply::with_status(warp::reply::json(&obj), http::StatusCode::BAD_REQUEST)
}
});this gives the error: is there a way to achieve this? |
|
@benitogf It seems to me that this is not easy to solve at the moment. What is really missing, is an |
|
@benitogf Adding a if key_valid(&key) {
- ws.on_upgrade(move |websocket| ws_reader(websocket, key, pools, db))
+ ws.on_upgrade(move |websocket| ws_reader(websocket, key, pools, db)).into_response()
} else {
let obj: WriteMessage = WriteMessage{
data: String::from("invalid key"),
};
- warp::reply::with_status(warp::reply::json(&obj), http::StatusCode::BAD_REQUEST)
+ warp::reply::with_status(warp::reply::json(&obj), http::StatusCode::BAD_REQUEST).into_response()
} |
this method is not available on version 0.3? Think that @boxdot comment is right, go this error after trying to use a a result and |
It is, you are probably just missing a |
|
thanks @rkusa using |
|
unless I miss something the use of |
- A handler function, instead of returning a Warp reply/rejection, returns a
`Result<Reply, ApiError>.`
- This is because rejections are meant to say "this filter can't handle this
request, but maybe some other can" (see
seanmonstar/warp#388 (comment)).
- A rejection means Warp will fall through to another filter and ultimately
hit a rejection handler, with people reporting rejections take way too long
to process with more routes.
- In our case, the error in our handler function is final and we also would
like to be able to use the ? operator to bail out of the handler if an error
exists, which using a Result type handles for us.
- ApiError knows how to convert itself to an HTTP response + status code
(error-specific), allowing us to implement Reply for ApiError.
- We can't implement `Reply` for `Result<Reply, Reply>` (we didn't make the
`Result` type), so we have to add a final function `into_response` that
converts our `Result` into a Response. We won't need to do this when
seanmonstar/warp#909 is merged:
```
.then(my_handler_func)
.map(into_response)
```
- A handler function, instead of returning a Warp reply/rejection, returns a
`Result<Reply, ApiError>.`
- This is because rejections are meant to say "this filter can't handle this
request, but maybe some other can" (see
seanmonstar/warp#388 (comment)).
- A rejection means Warp will fall through to another filter and ultimately
hit a rejection handler, with people reporting rejections take way too long
to process with more routes.
- In our case, the error in our handler function is final and we also would
like to be able to use the ? operator to bail out of the handler if an error
exists, which using a Result type handles for us.
- ApiError knows how to convert itself to an HTTP response + status code
(error-specific), allowing us to implement Reply for ApiError.
- We can't implement `Reply` for `Result<Reply, Reply>` (we didn't make the
`Result` type), so we have to add a final function `into_response` that
converts our `Result` into a Response. We won't need to do this when
seanmonstar/warp#909 is merged:
```
.then(my_handler_func)
.map(into_response)
```
- A handler function, instead of returning a Warp reply/rejection, returns a
`Result<Reply, ApiError>.`
- This is because rejections are meant to say "this filter can't handle this
request, but maybe some other can" (see
seanmonstar/warp#388 (comment)).
- A rejection means Warp will fall through to another filter and ultimately
hit a rejection handler, with people reporting rejections take way too long
to process with more routes.
- In our case, the error in our handler function is final and we also would
like to be able to use the ? operator to bail out of the handler if an error
exists, which using a Result type handles for us.
- ApiError knows how to convert itself to an HTTP response + status code
(error-specific), allowing us to implement Reply for ApiError.
- We can't implement `Reply` for `Result<Reply, Reply>` (we didn't make the
`Result` type), so we have to add a final function `into_response` that
converts our `Result` into a Response. We won't need to do this when
seanmonstar/warp#909 is merged:
```
.then(my_handler_func)
.map(into_response)
```
- A handler function, instead of returning a Warp reply/rejection, returns a
`Result<Reply, ApiError>.`
- This is because rejections are meant to say "this filter can't handle this
request, but maybe some other can" (see
seanmonstar/warp#388 (comment)).
- A rejection means Warp will fall through to another filter and ultimately
hit a rejection handler, with people reporting rejections take way too long
to process with more routes.
- In our case, the error in our handler function is final and we also would
like to be able to use the ? operator to bail out of the handler if an error
exists, which using a Result type handles for us.
- ApiError knows how to convert itself to an HTTP response + status code
(error-specific), allowing us to implement Reply for ApiError.
- We can't implement `Reply` for `Result<Reply, Reply>` (we didn't make the
`Result` type), so we have to add a final function `into_response` that
converts our `Result` into a Response. We won't need to do this when
seanmonstar/warp#909 is merged:
```
.then(my_handler_func)
.map(into_response)
```
It's possible I'm just not following the rejections example well enough, but what is the correct way of indicating that a handler (called with
and_then()) is fallible, and returning the error from that method in a reply?For an example of the behaviour I'm trying to replicate, in Actix or Rouille, handler functions are defined as returning something resembling a
Result<GoodReply, BadReply>, both of which can be expected to produce an HTTP response.e.g a simple index handler which should generate a 500 if the templating fails:
Reading the docs for Warp, it seems like a
Rejectionshould be used to indicate that this filter could not or should not accept the request, is this the case? And if so, is there an idiomatic way to replicate the behaviour I've sketched above? I can go down the road of matching on results from fallible functions in my handlers, and doing an earlyreturn warp::reply(...), but then I lose out on the ergonomics offered by the?operator inside the function.The text was updated successfully, but these errors were encountered: