Skip to content

Commit 8301a71

Browse files
committed
Add error handling / propagation
- 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) ```
1 parent 3cd7135 commit 8301a71

File tree

2 files changed

+83
-0
lines changed

2 files changed

+83
-0
lines changed

src/frontend/http_utils.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Warp error handling and propagation
2+
// Courtesy of https://github.com/seanmonstar/warp/pull/909#issuecomment-1184854848
3+
//
4+
// Usage:
5+
//
6+
// 1) A handler function, instead of returning a Warp reply/rejection, returns a
7+
// `Result<Reply, ApiError>.`
8+
//
9+
// This is because rejections are meant to say "this filter can't handle this request, but maybe
10+
// some other can" (see https://github.com/seanmonstar/warp/issues/388#issuecomment-576453485).
11+
// A rejection means Warp will fall through to another filter and ultimately hit a rejection
12+
// handler, with people reporting rejections take way too long to process with more routes.
13+
//
14+
// In our case, the error in our handler function is final and we also would like to be able
15+
// to use the ? operator to bail out of the handler if an error exists, which using a Result type
16+
// handles for us.
17+
//
18+
// 2) ApiError knows how to convert itself to an HTTP response + status code (error-specific), allowing
19+
// us to implement Reply for ApiError.
20+
//
21+
// 3) We can't implement Reply for Result<Reply, Reply> (we don't control Result), so we have to
22+
// add a final function `into_response` that converts our Result into a Response. We won't need
23+
// to do this when https://github.com/seanmonstar/warp/pull/909 is merged:
24+
//
25+
// ```
26+
// .then(my_handler_func)
27+
// .map(into_response)
28+
// ```
29+
//
30+
31+
use datafusion::error::DataFusionError;
32+
33+
use warp::hyper::{Body, Response, StatusCode};
34+
use warp::Reply;
35+
36+
#[derive(Debug)]
37+
pub enum ApiError {
38+
Forbidden,
39+
DataFusionError(DataFusionError),
40+
HashMismatch(String, String),
41+
NotReadOnlyQuery,
42+
}
43+
44+
// Wrap DataFusion errors so that we can automagically return an
45+
// `ApiError(DataFusionError)` by using the `?` operator
46+
impl From<DataFusionError> for ApiError {
47+
fn from(err: DataFusionError) -> Self {
48+
ApiError::DataFusionError(err)
49+
}
50+
}
51+
52+
impl ApiError {
53+
fn status_code_body(self: ApiError) -> (StatusCode, String) {
54+
match self {
55+
ApiError::Forbidden => (StatusCode::FORBIDDEN, "FORBIDDEN".to_string()),
56+
// TODO: figure out which DF errors to propagate, we have ones that are the server's fault
57+
// here too (e.g. ResourcesExhaused) and potentially some that leak internal information
58+
// (e.g. ObjectStore?)
59+
ApiError::DataFusionError(e) => (StatusCode::BAD_REQUEST, e.to_string()),
60+
// Mismatched hash
61+
ApiError::HashMismatch(expected, got) => (StatusCode::BAD_REQUEST, format!("Invalid hash: expected {0:?}, got {1:?}. Resend your query with {0:?}", expected, got)),
62+
ApiError::NotReadOnlyQuery => (StatusCode::METHOD_NOT_ALLOWED, "NOT_READ_ONLY_QUERY".to_string()),
63+
}
64+
}
65+
}
66+
67+
impl Reply for ApiError {
68+
fn into_response(self) -> Response<Body> {
69+
let (status, body) = self.status_code_body();
70+
Response::builder()
71+
.status(status)
72+
.body(body.into())
73+
.expect("Could not construct Response")
74+
}
75+
}
76+
77+
pub fn into_response<S: Reply, E: Reply>(reply_res: Result<S, E>) -> Response<Body> {
78+
match reply_res {
79+
Ok(resp) => resp.into_response(),
80+
Err(err) => err.into_response(),
81+
}
82+
}

src/frontend/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
pub mod http;
2+
pub mod http_utils;
23
#[cfg(feature = "frontend-postgres")]
34
pub mod postgres;

0 commit comments

Comments
 (0)