Replies: 12 comments
-
Do you mean move what is in error_page?
|
Beta Was this translation helpful? Give feedback.
-
Sorry I think I misunderstood and since deleted my comment. |
Beta Was this translation helpful? Give feedback.
-
I think will be good to provide request on error_response. |
Beta Was this translation helpful? Give feedback.
-
You should include all details required to generate error page in the error struct or enum variant. |
Beta Was this translation helpful? Give feedback.
-
I don't even see how get the data of the enum I provide. In |
Beta Was this translation helpful? Give feedback.
-
You may need to change the structure of your error type to have an error kind as some other metadata about the request. Eg: #[derive(Debug, Display)]
pub enum ErrKind {
CoreError {
source: CoreErr,
},
UrlError {
source: UrlGenerationError,
}
// etc...
}
struct ServerErr {
kind: ErrKind,
tennant: String,
}
// ...
fn extract_tenant(req: &HttpRequest) -> String {
// ...
}
async route_handler(req: HttpRequest) -> Result<HttpResponse, ServerErr> {
fn_that_can_error()
.map_err(|err|, ServerErr { kind: err, tenant: extract_tenant(&req) })
} This way you can still match on the err enum contents inside error response builders without having to include common metadata in every variant. |
Beta Was this translation helpful? Give feedback.
-
But that only catch some errors. Exist many errors that could be outside the handlers where I don't have a way to react to them, except on |
Beta Was this translation helpful? Give feedback.
-
I think a concrete example of a sitution where this pattern CAN NOT be used to cleanly handle an error should be provided to clearly demonstrate the deficiencies in this error handling strategy. |
Beta Was this translation helpful? Give feedback.
-
Ok, this could solve the issue. But I need to .map_err everywhere (after I do the rewrite I hope it also catch errors outside my code). I think will be more clean to unify the way errors are handled to be alike regular handlers, with access to HttpRequest, contexts, etc. |
Beta Was this translation helpful? Give feedback.
-
I’ve found that the need for map_err is reduced with From impls which can be derived using the From derive from derive_more. |
Beta Was this translation helpful? Give feedback.
-
Yep. i already use it: #[derive(From, Debug, Display)]
pub enum ServerErr { But it can't be used to extract data from request (like tenant) because it not have that info. So still demand to manually map, and is a lot in my case :(. |
Beta Was this translation helpful? Give feedback.
-
I have modified the middleware for error handling so I can get the headers, this partially alleviate the problem: #![allow(clippy::redundant_allocation)]
use std::rc::Rc;
use std::task::{Context, Poll};
use actix_service::{Service, Transform};
use actix_web::dev::{ServiceRequest, ServiceResponse};
use actix_web::http::HeaderMap;
use actix_web::Error;
use actix_web::Result;
use futures::future::{ok, FutureExt, LocalBoxFuture, Ready};
/// Error handler response
pub enum ErrorHandlerResponse<B> {
/// New http response got generated
Response(ServiceResponse<B>),
/// Result is a future that resolves to a new http response
Future(LocalBoxFuture<'static, Result<ServiceResponse<B>, Error>>),
}
pub type ErrorHandler<B> =
dyn Fn(ServiceResponse<B>, &HeaderMap) -> Result<ErrorHandlerResponse<B>>;
pub struct ErrorHandlers<B> {
handler: Rc<Box<ErrorHandler<B>>>,
}
impl<B> ErrorHandlers<B> {
/// Construct new `ErrorHandlers` instance
pub fn new(handler: Box<ErrorHandler<B>>) -> Self {
let handler = Rc::new(handler);
ErrorHandlers { handler }
}
}
impl<S, B> Transform<S> for ErrorHandlers<B>
where
S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
S::Future: 'static,
B: 'static,
{
type Request = ServiceRequest;
type Response = ServiceResponse<B>;
type Error = Error;
type Transform = ErrorHandlersMiddleware<S, B>;
type InitError = ();
type Future = Ready<Result<Self::Transform, Self::InitError>>;
fn new_transform(&self, service: S) -> Self::Future {
ok(ErrorHandlersMiddleware {
service,
handler: self.handler.clone(),
})
}
}
#[doc(hidden)]
pub struct ErrorHandlersMiddleware<S, B> {
service: S,
handler: Rc<Box<ErrorHandler<B>>>,
}
impl<S, B> Service for ErrorHandlersMiddleware<S, B>
where
S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
S::Future: 'static,
B: 'static,
{
type Request = ServiceRequest;
type Response = ServiceResponse<B>;
type Error = Error;
type Future = LocalBoxFuture<'static, Result<Self::Response, Self::Error>>;
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
self.service.poll_ready(cx)
}
fn call(&mut self, req: ServiceRequest) -> Self::Future {
let headers = req.headers().clone();
let handler = self.handler.clone();
let fut = self.service.call(req);
async move {
let res = fut.await?;
if res.status().is_client_error() || res.status().is_server_error() {
match handler(res, &headers) {
Ok(ErrorHandlerResponse::Response(res)) => Ok(res),
Ok(ErrorHandlerResponse::Future(fut)) => fut.await,
Err(e) => Err(e),
}
} else {
Ok(res)
}
}
.boxed_local()
}
}
pub fn is_ui_request(headers: &HeaderMap) -> bool {
//true
match headers.get("Content-Type") {
Some(t) => {
let tt = t.to_str().unwrap_or_else(|_| "");
!tt.contains("application/json")
}
None => true,
}
}
fn error_page<B>(
res: dev::ServiceResponse<B>,
headers: &HeaderMap,
) -> Result<ErrorHandlerResponse<B>> {
let status = res.status();
dbg!("error", &status);
if is_ui_request(headers) {
let company = get_req_company(&res.request());
let page = match admin::error_page(company, status) {
Ok(page) => page,
Err(err) => HttpResponse::InternalServerError().body(format!("{}", err)),
};
Ok(ErrorHandlerResponse::Response(
res.into_response(page.into_body()),
))
} else {
Ok(ErrorHandlerResponse::Response(res.into_response(
HttpResponse::build(status).json(false).into_body(),
)))
}
}
.wrap(ErrorHandlers::new(Box::new(error_page))) P.D: I collapse all errors in one single handler, bc I think is easier this way |
Beta Was this translation helpful? Give feedback.
-
Specially in development, is nicer to have a error page (404, 500, etc) not only show "is an error 404" but also whatever else help in debug problem. I need to check in the console to see what exactly happened.
I have implemented a custom error responder, but the extra context only work for json. How get it the html templates?:
P.D: One of the most nicer things django have apart of the auto-admin, is the error pages, at least in debug mode, are great for debugging. How hard to have something like that?
Beta Was this translation helpful? Give feedback.
All reactions