From bb01b691b9265fbebd4b0fcafa1678b6c3421806 Mon Sep 17 00:00:00 2001 From: fakeshadow <24548779@qq.com> Date: Mon, 25 Jan 2021 09:08:33 +0800 Subject: [PATCH] replace services with 500 error when data_factory construction failed --- src/app.rs | 54 +++++++++++---------- src/app_service.rs | 118 +++++++++++++++++++++++++++------------------ 2 files changed, 99 insertions(+), 73 deletions(-) diff --git a/src/app.rs b/src/app.rs index 9c5b806ea..c24e44bdd 100644 --- a/src/app.rs +++ b/src/app.rs @@ -11,11 +11,10 @@ use actix_service::{ apply, apply_fn_factory, IntoServiceFactory, ServiceFactory, ServiceFactoryExt, Transform, }; -use futures_util::future::FutureExt; use crate::app_service::{AppEntry, AppInit, AppRoutingFactory}; use crate::config::ServiceConfig; -use crate::data::{Data, DataFactory, FnDataFactory}; +use crate::data::{Data, FnDataFactory}; use crate::dev::ResourceDef; use crate::error::Error; use crate::resource::Resource; @@ -114,22 +113,19 @@ where E: std::fmt::Debug, { self.data_factories.push(Box::new(move || { - { - let fut = data(); - async move { - match fut.await { - Err(e) => { - log::error!("Can not construct data instance: {:?}", e); - Err(()) - } - Ok(data) => { - let data: Box = Box::new(Data::new(data)); - Ok(data) - } + let fut = data(); + Box::pin(async move { + match fut.await { + Err(e) => { + log::error!("Can not construct data instance: {:?}", e); + Err(()) + } + Ok(data) => { + let data = Box::new(Data::new(data)) as _; + Ok(data) } } - } - .boxed_local() + }) })); self } @@ -453,7 +449,7 @@ where fn into_factory(self) -> AppInit { AppInit { async_data_factories: self.data_factories.into_boxed_slice().into(), - endpoint: self.endpoint, + endpoint: Rc::new(self.endpoint), services: Rc::new(RefCell::new(self.services)), external: RefCell::new(self.external), default: self.default, @@ -473,9 +469,7 @@ mod tests { use crate::http::{header, HeaderValue, Method, StatusCode}; use crate::middleware::DefaultHeaders; use crate::service::ServiceRequest; - use crate::test::{ - call_service, init_service, read_body, try_init_service, TestRequest, - }; + use crate::test::{call_service, init_service, read_body, TestRequest}; use crate::{web, HttpRequest, HttpResponse}; #[actix_rt::test] @@ -546,13 +540,21 @@ mod tests { #[actix_rt::test] async fn test_data_factory_errors() { - let srv = - try_init_service(App::new().data_factory(|| err::(())).service( - web::resource("/").to(|_: web::Data| HttpResponse::Ok()), - )) - .await; + let mut srv = init_service( + App::new().data_factory(|| err::(())).service( + web::resource("/") + .route(web::get().to(|_: web::Data| HttpResponse::Ok())), + ), + ) + .await; - assert!(srv.is_err()); + let req = TestRequest::get().uri("/").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let req = TestRequest::post().uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); } #[actix_rt::test] diff --git a/src/app_service.rs b/src/app_service.rs index 8a00f59eb..7b22c3f14 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -26,14 +26,14 @@ type HttpNewService = BoxServiceFactory<(), ServiceRequest, ServiceResponse, Err pub struct AppInit where T: ServiceFactory< - ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), - >, + ServiceRequest, + Config = (), + Response = ServiceResponse, + Error = Error, + InitError = (), + > + 'static, { - pub(crate) endpoint: T, + pub(crate) endpoint: Rc, pub(crate) extensions: RefCell>, pub(crate) async_data_factories: Rc<[FnDataFactory]>, pub(crate) services: Rc>>>, @@ -45,12 +45,12 @@ where impl ServiceFactory for AppInit where T: ServiceFactory< - ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), - >, + ServiceRequest, + Config = (), + Response = ServiceResponse, + Error = Error, + InitError = (), + > + 'static, T::Future: 'static, { type Response = ServiceResponse; @@ -77,38 +77,21 @@ where .into_iter() .for_each(|mut srv| srv.register(&mut config)); - let mut rmap = ResourceMap::new(ResourceDef::new("")); - - let (config, services) = config.into_services(); - - // complete pipeline creation. - *self.factory_ref.borrow_mut() = Some(AppRoutingFactory { - default, - services: services - .into_iter() - .map(|(mut rdef, srv, guards, nested)| { - rmap.add(&mut rdef, nested); - (rdef, srv, RefCell::new(guards)) - }) - .collect::>() - .into_boxed_slice() - .into(), - }); - - // external resources - for mut rdef in std::mem::take(&mut *self.external.borrow_mut()) { - rmap.add(&mut rdef, None); - } - - // complete ResourceMap tree creation - let rmap = Rc::new(rmap); - rmap.finish(rmap.clone()); + let (config, mut services) = config.into_services(); // construct all async data factory futures let factory_futs = join_all(self.async_data_factories.iter().map(|f| f())); - // construct app service and middleware service factory future. - let endpoint_fut = self.endpoint.new_service(()); + // clone factory_ref and endpoint to use them in async block. + let factory_ref = self.factory_ref.clone(); + let endpoint = self.endpoint.clone(); + + // construct resource map. it would move to async block afterwards. + let mut rmap = ResourceMap::new(ResourceDef::new("")); + // add external resources + for mut rdef in std::mem::take(&mut *self.external.borrow_mut()) { + rmap.add(&mut rdef, None); + } // take extensions or create new one as app data container. let mut app_data = self @@ -118,15 +101,56 @@ where .unwrap_or_else(Extensions::new); Box::pin(async move { - // async data factories - let async_data_factories = factory_futs + // construct async data factories. + let res = factory_futs .await .into_iter() - .collect::, _>>() - .map_err(|_| ())?; + .collect::, _>>(); - // app service and middleware - let service = endpoint_fut.await?; + let async_data_factories = match res { + Ok(async_data_factories) => async_data_factories, + Err(_) => { + // error occur producing app_data. + // replace services with 500 error response + services = services + .into_iter() + .map(|(rdef, _, guards, nested)| { + let srv = boxed::factory(fn_service( + |req: ServiceRequest| async { + Ok(req.into_response( + Response::InternalServerError().finish(), + )) + }, + )); + (rdef, srv, guards, nested) + }) + .collect(); + + // throw away async_data. + Vec::new() + } + }; + + // complete pipeline creation. + *factory_ref.borrow_mut() = Some(AppRoutingFactory { + default, + services: services + .into_iter() + .map(|(mut rdef, srv, guards, nested)| { + rmap.add(&mut rdef, nested); + (rdef, srv, RefCell::new(guards)) + }) + .collect::>() + .into_boxed_slice() + .into(), + }); + + // complete ResourceMap tree creation + let rmap = Rc::new(rmap); + rmap.finish(rmap.clone()); + + // construct app service and middleware service. + let service = endpoint.new_service(()).await?; // populate app data container from (async) data factories. async_data_factories.iter().for_each(|factory| {