feat: add error response mappers

This commit is contained in:
Rob Ede 2023-02-13 23:40:23 +00:00
parent 82eed0ba1a
commit 7b79694b14
No known key found for this signature in database
GPG Key ID: F5E3FCAA33CBF062
2 changed files with 138 additions and 4 deletions

View File

@ -6,7 +6,7 @@ use crate::{HttpResponse, ResponseError};
/// General purpose Actix Web error.
///
/// An Actix Web error is used to carry errors from `std::error` through actix in a convenient way.
/// An Actix Web error is used to carry errors from `std::error` through Actix in a convenient way.
/// It can be created through converting errors with `into()`.
///
/// Whenever it is created from an external object a response error is created for it that can be
@ -14,6 +14,7 @@ use crate::{HttpResponse, ResponseError};
/// you can always get a `ResponseError` reference from it.
pub struct Error {
cause: Box<dyn ResponseError>,
response_mappers: Vec<Box<dyn Fn(HttpResponse) -> HttpResponse>>,
}
impl Error {
@ -29,7 +30,67 @@ impl Error {
/// Shortcut for creating an `HttpResponse`.
pub fn error_response(&self) -> HttpResponse {
self.cause.error_response()
let mut res = self.cause.error_response();
for mapper in &self.response_mappers {
res = (mapper)(res);
}
res
}
/// Adds a function that maps the HTTP response generated for this error.
///
/// Mappers are called in the order they are added each time
/// [`error_response`](Self::error_response) is called. A mapper may receive a response already
/// modified by other mappers, so it should avoid relying on a particular position in the chain.
///
/// Prefer narrowly mutating the provided response. Preserve fields the mapper does not own,
/// insert default headers only when absent, and merge list-valued headers without introducing
/// duplicates. Mappers should also be deterministic and safe to call more than once.
///
/// # Good
///
/// This mapper preserves the error response and adds a default only when it is absent:
///
/// ```
/// use actix_web::{
/// error,
/// http::header::{self, HeaderValue},
/// };
///
/// let mut err = error::ErrorBadRequest("bad request");
/// err.add_response_mapper(|mut res| {
/// if !res.headers().contains_key(header::CACHE_CONTROL) {
/// res.headers_mut()
/// .insert(header::CACHE_CONTROL, HeaderValue::from_static("no-store"));
/// }
///
/// res
/// });
///
/// assert_eq!(
/// err.error_response().headers().get(header::CACHE_CONTROL),
/// Some(&HeaderValue::from_static("no-store")),
/// );
/// ```
///
/// # Bad
///
/// Replacing the response discards the original status, body, headers, extensions, and any
/// changes made by earlier mappers:
///
/// ```
/// use actix_web::{error, HttpResponse};
///
/// let mut err = error::ErrorBadRequest("bad request");
/// err.add_response_mapper(|_| HttpResponse::InternalServerError().finish());
/// ```
pub fn add_response_mapper<F>(&mut self, mapper: F)
where
F: Fn(HttpResponse) -> HttpResponse + 'static,
{
self.response_mappers.push(Box::new(mapper))
}
}
@ -56,13 +117,17 @@ impl<T: ResponseError + 'static> From<T> for Error {
fn from(err: T) -> Error {
Error {
cause: Box::new(err),
response_mappers: Vec::new(),
}
}
}
impl From<Box<dyn ResponseError>> for Error {
fn from(value: Box<dyn ResponseError>) -> Self {
Error { cause: value }
Error {
cause: value,
response_mappers: Vec::new(),
}
}
}

View File

@ -4,8 +4,10 @@ use actix_utils::future::{ok, Ready};
use actix_web::{
dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform},
get,
http::{header, StatusCode},
middleware::{from_fn, Next},
test::{call_service, init_service, TestRequest},
ResponseError,
web, App, Error, HttpResponse, ResponseError,
};
use futures_core::future::LocalBoxFuture;
use futures_util::lock::Mutex;
@ -97,3 +99,70 @@ async fn error_cause_should_be_propagated_to_middlewares() {
let was_error_captured = lock.lock().await.unwrap();
assert!(was_error_captured);
}
async fn inner_error_middleware<B>(
req: ServiceRequest,
next: Next<B>,
) -> Result<ServiceResponse, Error> {
next.call(req).await?;
Err(actix_web::error::ErrorBadRequest("inner middleware error"))
}
async fn cors_like_middleware<B>(
req: ServiceRequest,
next: Next<B>,
) -> Result<ServiceResponse<B>, Error> {
let origin = req.headers().get(header::ORIGIN).cloned();
match next.call(req).await {
Ok(mut res) => {
if let Some(origin) = origin {
res.headers_mut()
.insert(header::ACCESS_CONTROL_ALLOW_ORIGIN, origin);
}
Ok(res)
}
Err(mut err) => {
if let Some(origin) = origin {
err.add_response_mapper(move |mut res| {
res.headers_mut()
.insert(header::ACCESS_CONTROL_ALLOW_ORIGIN, origin.clone());
res
});
}
Err(err)
}
}
}
#[actix_rt::test]
async fn error_response_mapper_allows_cors_like_middleware_to_augment_errors() {
let srv = actix_test::start(|| {
App::new()
// Emulate an inner middleware that propagates an error rather than constructing a
// response. This currently prevents response middleware such as actix-cors from
// augmenting the eventual error response.
.wrap(from_fn(inner_error_middleware))
// Emulate actix-cors as the outer middleware. Successful responses are augmented
// directly; errors carry a mapper that applies the same header during conversion.
.wrap(from_fn(cors_like_middleware))
.route("/", web::get().to(HttpResponse::Ok))
});
let res = srv
.get("/")
.insert_header((header::ORIGIN, "https://example.com"))
.send()
.await
.unwrap();
assert_eq!(res.status(), StatusCode::BAD_REQUEST);
assert_eq!(
res.headers().get(header::ACCESS_CONTROL_ALLOW_ORIGIN),
Some(&header::HeaderValue::from_static("https://example.com")),
);
srv.stop().await;
}