From 532f7b9923720df8f9057ff7941f7f310c5d3ccd Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 17 Jun 2021 17:57:58 +0100 Subject: [PATCH] refined error model (#2253) --- .cargo/config.toml | 5 +- CHANGES.md | 3 + actix-files/Cargo.toml | 1 + actix-http/CHANGES.md | 4 + actix-http/Cargo.toml | 3 +- actix-http/examples/echo.rs | 6 +- actix-http/examples/echo2.rs | 6 +- actix-http/examples/hello-world.rs | 14 +- actix-http/examples/streaming-error.rs | 40 +++ actix-http/src/body/body.rs | 13 +- actix-http/src/body/body_stream.rs | 80 +++++- actix-http/src/body/mod.rs | 6 +- actix-http/src/body/sized_stream.rs | 33 ++- actix-http/src/builder.rs | 45 ++-- actix-http/src/client/error.rs | 40 +-- actix-http/src/encoding/encoder.rs | 30 ++- actix-http/src/error.rs | 324 +++++++++++-------------- actix-http/src/h1/dispatcher.rs | 92 +++---- actix-http/src/h1/encoder.rs | 21 +- actix-http/src/h1/service.rs | 76 +++--- actix-http/src/h1/utils.rs | 13 +- actix-http/src/h2/dispatcher.rs | 26 +- actix-http/src/h2/service.rs | 66 ++--- actix-http/src/helpers.rs | 16 +- actix-http/src/lib.rs | 2 +- actix-http/src/response.rs | 67 +++-- actix-http/src/response_builder.rs | 23 +- actix-http/src/service.rs | 118 ++++----- actix-http/src/ws/dispatcher.rs | 7 +- actix-http/src/ws/mod.rs | 79 +++--- actix-http/tests/test_client.rs | 22 +- actix-http/tests/test_openssl.rs | 44 ++-- actix-http/tests/test_rustls.rs | 50 ++-- actix-http/tests/test_server.rs | 91 +++---- actix-http/tests/test_ws.rs | 45 +++- actix-test/src/lib.rs | 71 +++++- actix-web-actors/src/ws.rs | 9 +- awc/examples/client.rs | 4 +- awc/src/error.rs | 4 - awc/src/frozen.rs | 26 +- awc/src/lib.rs | 3 +- awc/src/request.rs | 29 +-- awc/src/sender.rs | 37 ++- src/data.rs | 3 +- src/error/error.rs | 76 ++++++ src/error/internal.rs | 105 ++++---- src/error/macros.rs | 109 +++++++++ src/error/mod.rs | 8 +- src/error/response_error.rs | 144 +++++++++++ src/extract.rs | 6 +- src/handler.rs | 6 +- src/helpers.rs | 25 ++ src/lib.rs | 21 +- src/middleware/compat.rs | 12 +- src/middleware/compress.rs | 2 +- src/middleware/err_handlers.rs | 2 +- src/request.rs | 7 +- src/request_data.rs | 3 +- src/resource.rs | 20 +- src/responder.rs | 13 +- src/response/builder.rs | 19 +- src/response/response.rs | 16 +- src/route.rs | 18 +- src/server.rs | 48 +++- src/service.rs | 24 +- src/types/form.rs | 2 +- src/types/json.rs | 4 +- src/types/path.rs | 10 +- src/types/query.rs | 2 +- 69 files changed, 1498 insertions(+), 901 deletions(-) create mode 100644 actix-http/examples/streaming-error.rs create mode 100644 src/error/error.rs create mode 100644 src/error/macros.rs create mode 100644 src/error/response_error.rs create mode 100644 src/helpers.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index 0bab205cd..0cf09f710 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,7 +1,8 @@ [alias] -chk = "hack check --workspace --all-features --tests --examples" -lint = "hack --clean-per-run clippy --workspace --tests --examples" +chk = "check --workspace --all-features --tests --examples --bins" +lint = "clippy --workspace --tests --examples" ci-min = "hack check --workspace --no-default-features" ci-min-test = "hack check --workspace --no-default-features --tests --examples" ci-default = "hack check --workspace" ci-full = "check --workspace --bins --examples --tests" +ci-test = "test --workspace --all-features --no-fail-fast" diff --git a/CHANGES.md b/CHANGES.md index ae1d435cc..5d33e6dd1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,8 @@ * Update `language-tags` to `0.3`. * `ServiceResponse::take_body`. [#2201] * `ServiceResponse::map_body` closure receives and returns `B` instead of `ResponseBody` types. [#2201] +* All error trait bounds in server service builders have changed from `Into` to `Into>`. [#2253] +* All error trait bounds in message body and stream impls changed from `Into` to `Into>`. [#2253] * `HttpServer::{listen_rustls(), bind_rustls()}` now honor the ALPN protocols in the configuation parameter. [#2226] * `middleware::normalize` now will not try to normalize URIs with no valid path [#2246] @@ -21,6 +23,7 @@ [#2200]: https://github.com/actix/actix-web/pull/2200 [#2201]: https://github.com/actix/actix-web/pull/2201 +[#2253]: https://github.com/actix/actix-web/pull/2253 [#2246]: https://github.com/actix/actix-web/pull/2246 diff --git a/actix-files/Cargo.toml b/actix-files/Cargo.toml index b97badd3e..6cff9b263 100644 --- a/actix-files/Cargo.toml +++ b/actix-files/Cargo.toml @@ -18,6 +18,7 @@ path = "src/lib.rs" [dependencies] actix-web = { version = "4.0.0-beta.6", default-features = false } +actix-http = "3.0.0-beta.6" actix-service = "2.0.0" actix-utils = "3.0.0" diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 99953ff26..f25e14254 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -13,12 +13,15 @@ ### Changed * The `MessageBody` trait now has an associated `Error` type. [#2183] +* All error trait bounds in server service builders have changed from `Into` to `Into>`. [#2253] +* All error trait bounds in message body and stream impls changed from `Into` to `Into>`. [#2253] * Places in `Response` where `ResponseBody` was received or returned now simply use `B`. [#2201] * `header` mod is now public. [#2171] * `uri` mod is now public. [#2171] * Update `language-tags` to `0.3`. * Reduce the level from `error` to `debug` for the log line that is emitted when a `500 Internal Server Error` is built using `HttpResponse::from_error`. [#2201] * `ResponseBuilder::message_body` now returns a `Result`. [#2201] +* Remove `Unpin` bound on `ResponseBuilder::streaming`. [#2253] * `HttpServer::{listen_rustls(), bind_rustls()}` now honor the ALPN protocols in the configuation parameter. [#2226] ### Removed @@ -37,6 +40,7 @@ [#2201]: https://github.com/actix/actix-web/pull/2201 [#2205]: https://github.com/actix/actix-web/pull/2205 [#2215]: https://github.com/actix/actix-web/pull/2215 +[#2253]: https://github.com/actix/actix-web/pull/2253 [#2244]: https://github.com/actix/actix-web/pull/2244 diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 1a0a32599..ea338fec1 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -84,7 +84,8 @@ trust-dns-resolver = { version = "0.20.0", optional = true } actix-server = "2.0.0-beta.3" actix-http-test = { version = "3.0.0-beta.4", features = ["openssl"] } actix-tls = { version = "3.0.0-beta.5", features = ["openssl"] } -criterion = "0.3" +async-stream = "0.3" +criterion = { version = "0.3", features = ["html_reports"] } env_logger = "0.8" rcgen = "0.8" serde = { version = "1.0", features = ["derive"] } diff --git a/actix-http/examples/echo.rs b/actix-http/examples/echo.rs index 54a71a106..6cfe3a675 100644 --- a/actix-http/examples/echo.rs +++ b/actix-http/examples/echo.rs @@ -5,14 +5,13 @@ use actix_server::Server; use bytes::BytesMut; use futures_util::StreamExt as _; use http::header::HeaderValue; -use log::info; #[actix_rt::main] async fn main() -> io::Result<()> { env_logger::init_from_env(env_logger::Env::new().default_filter_or("info")); Server::build() - .bind("echo", "127.0.0.1:8080", || { + .bind("echo", ("127.0.0.1", 8080), || { HttpService::build() .client_timeout(1000) .client_disconnect(1000) @@ -22,7 +21,8 @@ async fn main() -> io::Result<()> { body.extend_from_slice(&item?); } - info!("request body: {:?}", body); + log::info!("request body: {:?}", body); + Ok::<_, Error>( Response::build(StatusCode::OK) .insert_header(( diff --git a/actix-http/examples/echo2.rs b/actix-http/examples/echo2.rs index 3974cf20b..db195d65b 100644 --- a/actix-http/examples/echo2.rs +++ b/actix-http/examples/echo2.rs @@ -5,7 +5,6 @@ use actix_http::{Error, HttpService, Request, Response}; use actix_server::Server; use bytes::BytesMut; use futures_util::StreamExt as _; -use log::info; async fn handle_request(mut req: Request) -> Result, Error> { let mut body = BytesMut::new(); @@ -13,7 +12,8 @@ async fn handle_request(mut req: Request) -> Result, Error> { body.extend_from_slice(&item?) } - info!("request body: {:?}", body); + log::info!("request body: {:?}", body); + Ok(Response::build(StatusCode::OK) .insert_header(("x-head", HeaderValue::from_static("dummy value!"))) .body(body)) @@ -24,7 +24,7 @@ async fn main() -> io::Result<()> { env_logger::init_from_env(env_logger::Env::new().default_filter_or("info")); Server::build() - .bind("echo", "127.0.0.1:8080", || { + .bind("echo", ("127.0.0.1", 8080), || { HttpService::build().finish(handle_request).tcp() })? .run() diff --git a/actix-http/examples/hello-world.rs b/actix-http/examples/hello-world.rs index d51de6f4e..9a593c66a 100644 --- a/actix-http/examples/hello-world.rs +++ b/actix-http/examples/hello-world.rs @@ -1,28 +1,28 @@ -use std::io; +use std::{convert::Infallible, io}; use actix_http::{http::StatusCode, HttpService, Response}; use actix_server::Server; -use actix_utils::future; use http::header::HeaderValue; -use log::info; #[actix_rt::main] async fn main() -> io::Result<()> { env_logger::init_from_env(env_logger::Env::new().default_filter_or("info")); Server::build() - .bind("hello-world", "127.0.0.1:8080", || { + .bind("hello-world", ("127.0.0.1", 8080), || { HttpService::build() .client_timeout(1000) .client_disconnect(1000) - .finish(|_req| { - info!("{:?}", _req); + .finish(|req| async move { + log::info!("{:?}", req); + let mut res = Response::build(StatusCode::OK); res.insert_header(( "x-head", HeaderValue::from_static("dummy value!"), )); - future::ok::<_, ()>(res.body("Hello world!")) + + Ok::<_, Infallible>(res.body("Hello world!")) }) .tcp() })? diff --git a/actix-http/examples/streaming-error.rs b/actix-http/examples/streaming-error.rs new file mode 100644 index 000000000..3988cbac2 --- /dev/null +++ b/actix-http/examples/streaming-error.rs @@ -0,0 +1,40 @@ +//! Example showing response body (chunked) stream erroring. +//! +//! Test using `nc` or `curl`. +//! ```sh +//! $ curl -vN 127.0.0.1:8080 +//! $ echo 'GET / HTTP/1.1\n\n' | nc 127.0.0.1 8080 +//! ``` + +use std::{convert::Infallible, io, time::Duration}; + +use actix_http::{body::BodyStream, HttpService, Response}; +use actix_server::Server; +use async_stream::stream; +use bytes::Bytes; + +#[actix_rt::main] +async fn main() -> io::Result<()> { + env_logger::init_from_env(env_logger::Env::new().default_filter_or("info")); + + Server::build() + .bind("streaming-error", ("127.0.0.1", 8080), || { + HttpService::build() + .finish(|req| async move { + log::info!("{:?}", req); + let res = Response::ok(); + + Ok::<_, Infallible>(res.set_body(BodyStream::new(stream! { + yield Ok(Bytes::from("123")); + yield Ok(Bytes::from("456")); + + actix_rt::time::sleep(Duration::from_millis(1000)).await; + + yield Err(io::Error::new(io::ErrorKind::Other, "")); + }))) + }) + .tcp() + })? + .run() + .await +} diff --git a/actix-http/src/body/body.rs b/actix-http/src/body/body.rs index 3fda8ae11..f04837d07 100644 --- a/actix-http/src/body/body.rs +++ b/actix-http/src/body/body.rs @@ -76,7 +76,9 @@ impl MessageBody for AnyBody { // TODO: MSRV 1.51: poll_map_err AnyBody::Message(body) => match ready!(body.as_pin_mut().poll_next(cx)) { - Some(Err(err)) => Poll::Ready(Some(Err(err.into()))), + Some(Err(err)) => { + Poll::Ready(Some(Err(Error::new_body().with_cause(err)))) + } Some(Ok(val)) => Poll::Ready(Some(Ok(val))), None => Poll::Ready(None), }, @@ -162,9 +164,10 @@ impl From for AnyBody { } } -impl From> for AnyBody +impl From> for AnyBody where - S: Stream> + 'static, + S: Stream> + 'static, + E: Into> + 'static, { fn from(s: SizedStream) -> Body { AnyBody::from_message(s) @@ -174,7 +177,7 @@ where impl From> for AnyBody where S: Stream> + 'static, - E: Into + 'static, + E: Into> + 'static, { fn from(s: BodyStream) -> Body { AnyBody::from_message(s) @@ -222,7 +225,7 @@ impl MessageBody for BoxAnyBody { ) -> Poll>> { // TODO: MSRV 1.51: poll_map_err match ready!(self.0.as_mut().poll_next(cx)) { - Some(Err(err)) => Poll::Ready(Some(Err(err.into()))), + Some(Err(err)) => Poll::Ready(Some(Err(Error::new_body().with_cause(err)))), Some(Ok(val)) => Poll::Ready(Some(Ok(val))), None => Poll::Ready(None), } diff --git a/actix-http/src/body/body_stream.rs b/actix-http/src/body/body_stream.rs index ebe872022..f726f4475 100644 --- a/actix-http/src/body/body_stream.rs +++ b/actix-http/src/body/body_stream.rs @@ -1,4 +1,5 @@ use std::{ + error::Error as StdError, pin::Pin, task::{Context, Poll}, }; @@ -7,8 +8,6 @@ use bytes::Bytes; use futures_core::{ready, Stream}; use pin_project_lite::pin_project; -use crate::error::Error; - use super::{BodySize, MessageBody}; pin_project! { @@ -24,7 +23,7 @@ pin_project! { impl BodyStream where S: Stream>, - E: Into, + E: Into> + 'static, { pub fn new(stream: S) -> Self { BodyStream { stream } @@ -34,9 +33,9 @@ where impl MessageBody for BodyStream where S: Stream>, - E: Into, + E: Into> + 'static, { - type Error = Error; + type Error = E; fn size(&self) -> BodySize { BodySize::Stream @@ -56,7 +55,7 @@ where let chunk = match ready!(stream.poll_next(cx)) { Some(Ok(ref bytes)) if bytes.is_empty() => continue, - opt => opt.map(|res| res.map_err(Into::into)), + opt => opt, }; return Poll::Ready(chunk); @@ -66,9 +65,16 @@ where #[cfg(test)] mod tests { - use actix_rt::pin; + use std::{convert::Infallible, time::Duration}; + + use actix_rt::{ + pin, + time::{sleep, Sleep}, + }; use actix_utils::future::poll_fn; - use futures_util::stream; + use derive_more::{Display, Error}; + use futures_core::ready; + use futures_util::{stream, FutureExt as _}; use super::*; use crate::body::to_bytes; @@ -78,7 +84,7 @@ mod tests { let body = BodyStream::new(stream::iter( ["1", "", "2"] .iter() - .map(|&v| Ok(Bytes::from(v)) as Result), + .map(|&v| Ok::<_, Infallible>(Bytes::from(v))), )); pin!(body); @@ -103,9 +109,63 @@ mod tests { let body = BodyStream::new(stream::iter( ["1", "", "2"] .iter() - .map(|&v| Ok(Bytes::from(v)) as Result), + .map(|&v| Ok::<_, Infallible>(Bytes::from(v))), )); assert_eq!(to_bytes(body).await.ok(), Some(Bytes::from("12"))); } + #[derive(Debug, Display, Error)] + #[display(fmt = "stream error")] + struct StreamErr; + + #[actix_rt::test] + async fn stream_immediate_error() { + let body = BodyStream::new(stream::once(async { Err(StreamErr) })); + assert!(matches!(to_bytes(body).await, Err(StreamErr))); + } + + #[actix_rt::test] + async fn stream_delayed_error() { + let body = + BodyStream::new(stream::iter(vec![Ok(Bytes::from("1")), Err(StreamErr)])); + assert!(matches!(to_bytes(body).await, Err(StreamErr))); + + #[pin_project::pin_project(project = TimeDelayStreamProj)] + #[derive(Debug)] + enum TimeDelayStream { + Start, + Sleep(Pin>), + Done, + } + + impl Stream for TimeDelayStream { + type Item = Result; + + fn poll_next( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll> { + match self.as_mut().get_mut() { + TimeDelayStream::Start => { + let sleep = sleep(Duration::from_millis(1)); + self.as_mut().set(TimeDelayStream::Sleep(Box::pin(sleep))); + cx.waker().wake_by_ref(); + Poll::Pending + } + + TimeDelayStream::Sleep(ref mut delay) => { + ready!(delay.poll_unpin(cx)); + self.set(TimeDelayStream::Done); + cx.waker().wake_by_ref(); + Poll::Pending + } + + TimeDelayStream::Done => Poll::Ready(Some(Err(StreamErr))), + } + } + } + + let body = BodyStream::new(TimeDelayStream::Start); + assert!(matches!(to_bytes(body).await, Err(StreamErr))); + } } diff --git a/actix-http/src/body/mod.rs b/actix-http/src/body/mod.rs index 11aff039e..8a08dbd2b 100644 --- a/actix-http/src/body/mod.rs +++ b/actix-http/src/body/mod.rs @@ -191,11 +191,15 @@ mod tests { } #[actix_rt::test] - async fn test_box() { + async fn test_box_and_pin() { let val = Box::new(()); pin!(val); assert_eq!(val.size(), BodySize::Empty); assert!(poll_fn(|cx| val.as_mut().poll_next(cx)).await.is_none()); + + let mut val = Box::pin(()); + assert_eq!(val.size(), BodySize::Empty); + assert!(poll_fn(|cx| val.as_mut().poll_next(cx)).await.is_none()); } #[actix_rt::test] diff --git a/actix-http/src/body/sized_stream.rs b/actix-http/src/body/sized_stream.rs index 4af132389..b6ceb32fe 100644 --- a/actix-http/src/body/sized_stream.rs +++ b/actix-http/src/body/sized_stream.rs @@ -1,4 +1,5 @@ use std::{ + error::Error as StdError, pin::Pin, task::{Context, Poll}, }; @@ -7,15 +8,13 @@ use bytes::Bytes; use futures_core::{ready, Stream}; use pin_project_lite::pin_project; -use crate::error::Error; - use super::{BodySize, MessageBody}; pin_project! { /// Known sized streaming response wrapper. /// - /// This body implementation should be used if total size of stream is known. Data get sent as is - /// without using transfer encoding. + /// This body implementation should be used if total size of stream is known. Data is sent as-is + /// without using chunked transfer encoding. pub struct SizedStream { size: u64, #[pin] @@ -23,20 +22,22 @@ pin_project! { } } -impl SizedStream +impl SizedStream where - S: Stream>, + S: Stream>, + E: Into> + 'static, { pub fn new(size: u64, stream: S) -> Self { SizedStream { size, stream } } } -impl MessageBody for SizedStream +impl MessageBody for SizedStream where - S: Stream>, + S: Stream>, + E: Into> + 'static, { - type Error = Error; + type Error = E; fn size(&self) -> BodySize { BodySize::Sized(self.size as u64) @@ -66,6 +67,8 @@ where #[cfg(test)] mod tests { + use std::convert::Infallible; + use actix_rt::pin; use actix_utils::future::poll_fn; use futures_util::stream; @@ -77,7 +80,11 @@ mod tests { async fn skips_empty_chunks() { let body = SizedStream::new( 2, - stream::iter(["1", "", "2"].iter().map(|&v| Ok(Bytes::from(v)))), + stream::iter( + ["1", "", "2"] + .iter() + .map(|&v| Ok::<_, Infallible>(Bytes::from(v))), + ), ); pin!(body); @@ -103,7 +110,11 @@ mod tests { async fn read_to_bytes() { let body = SizedStream::new( 2, - stream::iter(["1", "", "2"].iter().map(|&v| Ok(Bytes::from(v)))), + stream::iter( + ["1", "", "2"] + .iter() + .map(|&v| Ok::<_, Infallible>(Bytes::from(v))), + ), ); assert_eq!(to_bytes(body).await.ok(), Some(Bytes::from("12"))); diff --git a/actix-http/src/builder.rs b/actix-http/src/builder.rs index 660cd9817..4e68dc920 100644 --- a/actix-http/src/builder.rs +++ b/actix-http/src/builder.rs @@ -1,19 +1,16 @@ -use std::marker::PhantomData; -use std::rc::Rc; -use std::{fmt, net}; +use std::{error::Error as StdError, fmt, marker::PhantomData, net, rc::Rc}; use actix_codec::Framed; use actix_service::{IntoServiceFactory, Service, ServiceFactory}; -use crate::body::MessageBody; -use crate::config::{KeepAlive, ServiceConfig}; -use crate::error::Error; -use crate::h1::{Codec, ExpectHandler, H1Service, UpgradeHandler}; -use crate::h2::H2Service; -use crate::request::Request; -use crate::response::Response; -use crate::service::HttpService; -use crate::{ConnectCallback, Extensions}; +use crate::{ + body::{AnyBody, MessageBody}, + config::{KeepAlive, ServiceConfig}, + h1::{self, ExpectHandler, H1Service, UpgradeHandler}, + h2::H2Service, + service::HttpService, + ConnectCallback, Extensions, Request, Response, +}; /// A HTTP service builder /// @@ -34,7 +31,7 @@ pub struct HttpServiceBuilder { impl HttpServiceBuilder where S: ServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, >::Future: 'static, { @@ -57,13 +54,13 @@ where impl HttpServiceBuilder where S: ServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, >::Future: 'static, X: ServiceFactory, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, - U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, + U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U::Error: fmt::Display, U::InitError: fmt::Debug, { @@ -123,7 +120,7 @@ where where F: IntoServiceFactory, X1: ServiceFactory, - X1::Error: Into, + X1::Error: Into>, X1::InitError: fmt::Debug, { HttpServiceBuilder { @@ -145,8 +142,8 @@ where /// and this service get called with original request and framed object. pub fn upgrade(self, upgrade: F) -> HttpServiceBuilder where - F: IntoServiceFactory)>, - U1: ServiceFactory<(Request, Framed), Config = (), Response = ()>, + F: IntoServiceFactory)>, + U1: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U1::Error: fmt::Display, U1::InitError: fmt::Debug, { @@ -181,7 +178,7 @@ where where B: MessageBody, F: IntoServiceFactory, - S::Error: Into, + S::Error: Into>, S::InitError: fmt::Debug, S::Response: Into>, { @@ -203,12 +200,12 @@ where pub fn h2(self, service: F) -> H2Service where F: IntoServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { let cfg = ServiceConfig::new( self.keep_alive, @@ -226,12 +223,12 @@ where pub fn finish(self, service: F) -> HttpService where F: IntoServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { let cfg = ServiceConfig::new( self.keep_alive, diff --git a/actix-http/src/client/error.rs b/actix-http/src/client/error.rs index d27363456..34833503b 100644 --- a/actix-http/src/client/error.rs +++ b/actix-http/src/client/error.rs @@ -1,15 +1,16 @@ -use std::io; +use std::{error::Error as StdError, fmt, io}; use derive_more::{Display, From}; #[cfg(feature = "openssl")] use actix_tls::accept::openssl::SslError; -use crate::error::{Error, ParseError, ResponseError}; -use crate::http::{Error as HttpError, StatusCode}; +use crate::error::{Error, ParseError}; +use crate::http::Error as HttpError; /// A set of errors that can occur while connecting to an HTTP host #[derive(Debug, Display, From)] +#[non_exhaustive] pub enum ConnectError { /// SSL feature is not enabled #[display(fmt = "SSL is not supported")] @@ -64,6 +65,7 @@ impl From for ConnectError { } #[derive(Debug, Display, From)] +#[non_exhaustive] pub enum InvalidUrl { #[display(fmt = "Missing URL scheme")] MissingScheme, @@ -82,6 +84,7 @@ impl std::error::Error for InvalidUrl {} /// A set of errors that can occur during request sending and response reading #[derive(Debug, Display, From)] +#[non_exhaustive] pub enum SendRequestError { /// Invalid URL #[display(fmt = "Invalid URL: {}", _0)] @@ -115,25 +118,17 @@ pub enum SendRequestError { /// Error sending request body Body(Error), + + /// Other errors that can occur after submitting a request. + #[display(fmt = "{:?}: {}", _1, _0)] + Custom(Box, Box), } impl std::error::Error for SendRequestError {} -/// Convert `SendRequestError` to a server `Response` -impl ResponseError for SendRequestError { - fn status_code(&self) -> StatusCode { - match *self { - SendRequestError::Connect(ConnectError::Timeout) => { - StatusCode::GATEWAY_TIMEOUT - } - SendRequestError::Connect(_) => StatusCode::BAD_REQUEST, - _ => StatusCode::INTERNAL_SERVER_ERROR, - } - } -} - /// A set of errors that can occur during freezing a request #[derive(Debug, Display, From)] +#[non_exhaustive] pub enum FreezeRequestError { /// Invalid URL #[display(fmt = "Invalid URL: {}", _0)] @@ -142,15 +137,20 @@ pub enum FreezeRequestError { /// HTTP error #[display(fmt = "{}", _0)] Http(HttpError), + + /// Other errors that can occur after submitting a request. + #[display(fmt = "{:?}: {}", _1, _0)] + Custom(Box, Box), } impl std::error::Error for FreezeRequestError {} impl From for SendRequestError { - fn from(e: FreezeRequestError) -> Self { - match e { - FreezeRequestError::Url(e) => e.into(), - FreezeRequestError::Http(e) => e.into(), + fn from(err: FreezeRequestError) -> Self { + match err { + FreezeRequestError::Url(err) => err.into(), + FreezeRequestError::Http(err) => err.into(), + FreezeRequestError::Custom(err, msg) => SendRequestError::Custom(err, msg), } } } diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index 6adde9be2..36509b371 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -133,9 +133,7 @@ where }, EncoderBodyProj::BoxedStream(ref mut b) => { match ready!(b.as_pin_mut().poll_next(cx)) { - Some(Err(err)) => { - Poll::Ready(Some(Err(EncoderError::Boxed(err.into())))) - } + Some(Err(err)) => Poll::Ready(Some(Err(EncoderError::Boxed(err)))), Some(Ok(val)) => Poll::Ready(Some(Ok(val))), None => Poll::Ready(None), } @@ -337,7 +335,7 @@ pub enum EncoderError { Body(E), #[display(fmt = "boxed")] - Boxed(Error), + Boxed(Box), #[display(fmt = "blocking")] Blocking(BlockingError), @@ -346,19 +344,19 @@ pub enum EncoderError { Io(io::Error), } -impl StdError for EncoderError { +impl StdError for EncoderError { fn source(&self) -> Option<&(dyn StdError + 'static)> { - None - } -} - -impl> From> for Error { - fn from(err: EncoderError) -> Self { - match err { - EncoderError::Body(err) => err.into(), - EncoderError::Boxed(err) => err, - EncoderError::Blocking(err) => err.into(), - EncoderError::Io(err) => err.into(), + match self { + EncoderError::Body(err) => Some(err), + EncoderError::Boxed(err) => Some(&**err), + EncoderError::Blocking(err) => Some(err), + EncoderError::Io(err) => Some(err), } } } + +impl From> for crate::Error { + fn from(err: EncoderError) -> Self { + crate::Error::new_encoder().with_cause(err) + } +} diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index 92efd572d..d9e1a1ed2 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -1,174 +1,155 @@ //! Error and Result module -use std::{ - error::Error as StdError, - fmt, - io::{self, Write as _}, - str::Utf8Error, - string::FromUtf8Error, -}; +use std::{error::Error as StdError, fmt, io, str::Utf8Error, string::FromUtf8Error}; -use bytes::BytesMut; use derive_more::{Display, Error, From}; -use http::{header, uri::InvalidUri, StatusCode}; -use serde::de::value::Error as DeError; +use http::{uri::InvalidUri, StatusCode}; -use crate::{body::Body, helpers::Writer, Response}; +use crate::{ + body::{AnyBody, Body}, + ws, Response, +}; pub use http::Error as HttpError; -/// General purpose actix web error. -/// -/// 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 used to create an HTTP response from it this means that -/// if you have access to an actix `Error` you can always get a -/// `ResponseError` reference from it. pub struct Error { - cause: Box, + inner: Box, +} + +pub(crate) struct ErrorInner { + #[allow(dead_code)] + kind: Kind, + cause: Option>, } impl Error { - /// Returns the reference to the underlying `ResponseError`. - pub fn as_response_error(&self) -> &dyn ResponseError { - self.cause.as_ref() + fn new(kind: Kind) -> Self { + Self { + inner: Box::new(ErrorInner { kind, cause: None }), + } } - /// Similar to `as_response_error` but downcasts. - pub fn as_error(&self) -> Option<&T> { - ::downcast_ref(self.cause.as_ref()) + pub(crate) fn new_http() -> Self { + Self::new(Kind::Http) + } + + pub(crate) fn new_parse() -> Self { + Self::new(Kind::Parse) + } + + pub(crate) fn new_payload() -> Self { + Self::new(Kind::Payload) + } + + pub(crate) fn new_body() -> Self { + Self::new(Kind::Body) + } + + pub(crate) fn new_send_response() -> Self { + Self::new(Kind::SendResponse) + } + + // TODO: remove allow + #[allow(dead_code)] + pub(crate) fn new_io() -> Self { + Self::new(Kind::Io) + } + + pub(crate) fn new_encoder() -> Self { + Self::new(Kind::Encoder) + } + + pub(crate) fn new_ws() -> Self { + Self::new(Kind::Ws) + } + + pub(crate) fn with_cause(mut self, cause: impl Into>) -> Self { + self.inner.cause = Some(cause.into()); + self } } -/// Errors that can generate responses. -pub trait ResponseError: fmt::Debug + fmt::Display { - /// Returns appropriate status code for error. - /// - /// A 500 Internal Server Error is used by default. If [error_response](Self::error_response) is - /// also implemented and does not call `self.status_code()`, then this will not be used. - fn status_code(&self) -> StatusCode { - StatusCode::INTERNAL_SERVER_ERROR - } +impl From for Response { + fn from(err: Error) -> Self { + let status_code = match err.inner.kind { + Kind::Parse => StatusCode::BAD_REQUEST, + _ => StatusCode::INTERNAL_SERVER_ERROR, + }; - /// Creates full response for error. - /// - /// By default, the generated response uses a 500 Internal Server Error status code, a - /// `Content-Type` of `text/plain`, and the body is set to `Self`'s `Display` impl. - fn error_response(&self) -> Response { - let mut resp = Response::new(self.status_code()); - let mut buf = BytesMut::new(); - let _ = write!(Writer(&mut buf), "{}", self); - resp.headers_mut().insert( - header::CONTENT_TYPE, - header::HeaderValue::from_static("text/plain; charset=utf-8"), - ); - resp.set_body(Body::from(buf)) + Response::new(status_code).set_body(Body::from(err.to_string())) } - - downcast_get_type_id!(); } -downcast!(ResponseError); +#[derive(Debug, Clone, Copy, PartialEq, Eq, Display)] +pub enum Kind { + #[display(fmt = "error processing HTTP")] + Http, -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.cause, f) - } + #[display(fmt = "error parsing HTTP message")] + Parse, + + #[display(fmt = "request payload read error")] + Payload, + + #[display(fmt = "response body write error")] + Body, + + #[display(fmt = "send response error")] + SendResponse, + + #[display(fmt = "error in WebSocket process")] + Ws, + + #[display(fmt = "connection error")] + Io, + + #[display(fmt = "encoder error")] + Encoder, } impl fmt::Debug for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", &self.cause) + // TODO: more detail + f.write_str("actix_http::Error") } } -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - None +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.inner.cause.as_ref() { + Some(err) => write!(f, "{}: {}", &self.inner.kind, err), + None => write!(f, "{}", &self.inner.kind), + } } } -impl From<()> for Error { - fn from(_: ()) -> Self { - Error::from(UnitError) +impl StdError for Error { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + self.inner.cause.as_ref().map(|err| err.as_ref()) } } impl From for Error { - fn from(_: std::convert::Infallible) -> Self { - // hint that an error that will never happen - unreachable!() + fn from(err: std::convert::Infallible) -> Self { + match err {} } } -/// Convert `Error` to a `Response` instance -impl From for Response { - fn from(err: Error) -> Self { - Response::from_error(err) +impl From for Error { + fn from(err: ws::ProtocolError) -> Self { + Self::new_ws().with_cause(err) } } -/// `Error` for any error that implements `ResponseError` -impl From for Error { - fn from(err: T) -> Error { - Error { - cause: Box::new(err), - } +impl From for Error { + fn from(err: HttpError) -> Self { + Self::new_http().with_cause(err) } } -#[derive(Debug, Display, Error)] -#[display(fmt = "Unknown Error")] -struct UnitError; - -impl ResponseError for Box {} - -/// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`UnitError`]. -impl ResponseError for UnitError {} - -/// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`actix_tls::accept::openssl::SslError`]. -#[cfg(feature = "openssl")] -impl ResponseError for actix_tls::accept::openssl::SslError {} - -/// Returns [`StatusCode::BAD_REQUEST`] for [`DeError`]. -impl ResponseError for DeError { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST - } -} - -/// Returns [`StatusCode::BAD_REQUEST`] for [`Utf8Error`]. -impl ResponseError for Utf8Error { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST - } -} - -/// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`HttpError`]. -impl ResponseError for HttpError {} - -/// Inspects the underlying [`io::ErrorKind`] and returns an appropriate status code. -/// -/// If the error is [`io::ErrorKind::NotFound`], [`StatusCode::NOT_FOUND`] is returned. If the -/// error is [`io::ErrorKind::PermissionDenied`], [`StatusCode::FORBIDDEN`] is returned. Otherwise, -/// [`StatusCode::INTERNAL_SERVER_ERROR`] is returned. -impl ResponseError for io::Error { - fn status_code(&self) -> StatusCode { - match self.kind() { - io::ErrorKind::NotFound => StatusCode::NOT_FOUND, - io::ErrorKind::PermissionDenied => StatusCode::FORBIDDEN, - _ => StatusCode::INTERNAL_SERVER_ERROR, - } - } -} - -/// Returns [`StatusCode::BAD_REQUEST`] for [`header::InvalidHeaderValue`]. -impl ResponseError for header::InvalidHeaderValue { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST +impl From for Error { + fn from(err: ws::HandshakeError) -> Self { + Self::new_ws().with_cause(err) } } @@ -218,13 +199,6 @@ pub enum ParseError { Utf8(Utf8Error), } -/// Return `BadRequest` for `ParseError` -impl ResponseError for ParseError { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST - } -} - impl From for ParseError { fn from(err: io::Error) -> ParseError { ParseError::Io(err) @@ -263,14 +237,23 @@ impl From for ParseError { } } +impl From for Error { + fn from(err: ParseError) -> Self { + Self::new_parse().with_cause(err) + } +} + +impl From for Response { + fn from(err: ParseError) -> Self { + Error::from(err).into() + } +} + /// A set of errors that can occur running blocking tasks in thread pool. #[derive(Debug, Display, Error)] #[display(fmt = "Blocking thread pool is gone")] pub struct BlockingError; -/// `InternalServerError` for `BlockingError` -impl ResponseError for BlockingError {} - /// A set of errors that can occur during payload parsing. #[derive(Debug, Display)] #[non_exhaustive] @@ -344,16 +327,9 @@ impl From for PayloadError { } } -/// `PayloadError` returns two possible results: -/// -/// - `Overflow` returns `PayloadTooLarge` -/// - Other errors returns `BadRequest` -impl ResponseError for PayloadError { - fn status_code(&self) -> StatusCode { - match *self { - PayloadError::Overflow => StatusCode::PAYLOAD_TOO_LARGE, - _ => StatusCode::BAD_REQUEST, - } +impl From for Error { + fn from(err: PayloadError) -> Self { + Self::new_payload().with_cause(err) } } @@ -362,13 +338,19 @@ impl ResponseError for PayloadError { #[non_exhaustive] pub enum DispatchError { /// Service error - Service(Error), + // FIXME: display and error type + #[display(fmt = "Service Error")] + Service(#[error(not(source))] Response), + + /// Body error + // FIXME: display and error type + #[display(fmt = "Body Error")] + Body(#[error(not(source))] Box), /// Upgrade service error Upgrade, - /// An `io::Error` that occurred while trying to read or write to a network - /// stream. + /// An `io::Error` that occurred while trying to read or write to a network stream. #[display(fmt = "IO error: {}", _0)] Io(io::Error), @@ -434,12 +416,6 @@ mod content_type_test_impls { } } -impl ResponseError for ContentTypeError { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST - } -} - #[cfg(test)] mod tests { use super::*; @@ -448,42 +424,36 @@ mod tests { #[test] fn test_into_response() { - let resp: Response = ParseError::Incomplete.error_response(); + let resp: Response = ParseError::Incomplete.into(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); let err: HttpError = StatusCode::from_u16(10000).err().unwrap().into(); - let resp: Response = err.error_response(); + let resp: Response = Error::new_http().with_cause(err).into(); assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } #[test] fn test_as_response() { let orig = io::Error::new(io::ErrorKind::Other, "other"); - let e: Error = ParseError::Io(orig).into(); - assert_eq!(format!("{}", e.as_response_error()), "IO error: other"); - } - - #[test] - fn test_error_cause() { - let orig = io::Error::new(io::ErrorKind::Other, "other"); - let desc = orig.to_string(); - let e = Error::from(orig); - assert_eq!(format!("{}", e.as_response_error()), desc); + let err: Error = ParseError::Io(orig).into(); + assert_eq!( + format!("{}", err), + "error parsing HTTP message: IO error: other" + ); } #[test] fn test_error_display() { let orig = io::Error::new(io::ErrorKind::Other, "other"); - let desc = orig.to_string(); - let e = Error::from(orig); - assert_eq!(format!("{}", e), desc); + let err = Error::new_io().with_cause(orig); + assert_eq!("connection error: other", err.to_string()); } #[test] fn test_error_http_response() { let orig = io::Error::new(io::ErrorKind::Other, "other"); - let e = Error::from(orig); - let resp: Response = e.into(); + let err = Error::new_io().with_cause(orig); + let resp: Response = err.into(); assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } @@ -535,14 +505,4 @@ mod tests { from!(httparse::Error::TooManyHeaders => ParseError::TooLarge); from!(httparse::Error::Version => ParseError::Version); } - - #[test] - fn test_error_casting() { - let err = PayloadError::Overflow; - let resp_err: &dyn ResponseError = &err; - let err = resp_err.downcast_ref::().unwrap(); - assert_eq!(err.to_string(), "Payload reached size limit."); - let not_err = resp_err.downcast_ref::(); - assert!(not_err.is_none()); - } } diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs index c81d0b3bc..b4adde638 100644 --- a/actix-http/src/h1/dispatcher.rs +++ b/actix-http/src/h1/dispatcher.rs @@ -1,5 +1,6 @@ use std::{ collections::VecDeque, + error::Error as StdError, fmt, future::Future, io, mem, net, @@ -17,19 +18,19 @@ use futures_core::ready; use log::{error, trace}; use pin_project::pin_project; -use crate::body::{Body, BodySize, MessageBody}; -use crate::config::ServiceConfig; -use crate::error::{DispatchError, Error}; -use crate::error::{ParseError, PayloadError}; -use crate::http::StatusCode; -use crate::request::Request; -use crate::response::Response; -use crate::service::HttpFlow; -use crate::OnConnectData; +use crate::{ + body::{AnyBody, BodySize, MessageBody}, + config::ServiceConfig, + error::{DispatchError, ParseError, PayloadError}, + service::HttpFlow, + OnConnectData, Request, Response, StatusCode, +}; -use super::codec::Codec; -use super::payload::{Payload, PayloadSender, PayloadStatus}; -use super::{Message, MessageType}; +use super::{ + codec::Codec, + payload::{Payload, PayloadSender, PayloadStatus}, + Message, MessageType, +}; const LW_BUFFER_SIZE: usize = 1024; const HW_BUFFER_SIZE: usize = 1024 * 8; @@ -50,13 +51,13 @@ bitflags! { pub struct Dispatcher where S: Service, - S::Error: Into, + S::Error: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -72,13 +73,13 @@ where enum DispatcherState where S: Service, - S::Error: Into, + S::Error: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -91,13 +92,13 @@ where struct InnerDispatcher where S: Service, - S::Error: Into, + S::Error: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -136,13 +137,13 @@ where X: Service, B: MessageBody, - B::Error: Into, + B::Error: Into>, { None, ExpectCall(#[pin] X::Future), ServiceCall(#[pin] S::Future), SendPayload(#[pin] B), - SendErrorPayload(#[pin] Body), + SendErrorPayload(#[pin] AnyBody), } impl State @@ -152,7 +153,7 @@ where X: Service, B: MessageBody, - B::Error: Into, + B::Error: Into>, { fn is_empty(&self) -> bool { matches!(self, State::None) @@ -170,14 +171,14 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into, + S::Error: Into>, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -231,14 +232,14 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into, + S::Error: Into>, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -334,7 +335,7 @@ where fn send_error_response( mut self: Pin<&mut Self>, message: Response<()>, - body: Body, + body: AnyBody, ) -> Result<(), DispatchError> { let size = self.as_mut().send_response_inner(message, &body)?; let state = match size { @@ -379,7 +380,7 @@ where // send_response would update InnerDispatcher state to SendPayload or // None(If response body is empty). // continue loop to poll it. - self.as_mut().send_error_response(res, Body::Empty)?; + self.as_mut().send_error_response(res, AnyBody::Empty)?; } // return with upgrade request and poll it exclusively. @@ -399,7 +400,7 @@ where // send service call error as response Poll::Ready(Err(err)) => { - let res = Response::from_error(err); + let res: Response = err.into(); let (res, body) = res.replace_body(()); self.as_mut().send_error_response(res, body)?; } @@ -438,7 +439,7 @@ where } Poll::Ready(Some(Err(err))) => { - return Err(DispatchError::Service(err.into())) + return Err(DispatchError::Body(err.into())) } Poll::Pending => return Ok(PollResponse::DoNothing), @@ -473,7 +474,7 @@ where } Poll::Ready(Some(Err(err))) => { - return Err(DispatchError::Service(err)) + return Err(DispatchError::Service(err.into())) } Poll::Pending => return Ok(PollResponse::DoNothing), @@ -496,7 +497,7 @@ where // send expect error as response Poll::Ready(Err(err)) => { - let res = Response::from_error(err); + let res: Response = err.into(); let (res, body) = res.replace_body(()); self.as_mut().send_error_response(res, body)?; } @@ -546,7 +547,7 @@ where // to notify the dispatcher a new state is set and the outer loop // should be continue. Poll::Ready(Err(err)) => { - let res = Response::from_error(err); + let res: Response = err.into(); let (res, body) = res.replace_body(()); return self.send_error_response(res, body); } @@ -566,7 +567,7 @@ where Poll::Pending => Ok(()), // see the comment on ExpectCall state branch's Ready(Err(err)). Poll::Ready(Err(err)) => { - let res = Response::from_error(err); + let res: Response = err.into(); let (res, body) = res.replace_body(()); self.send_error_response(res, body) } @@ -772,7 +773,7 @@ where trace!("Slow request timeout"); let _ = self.as_mut().send_error_response( Response::with_body(StatusCode::REQUEST_TIMEOUT, ()), - Body::Empty, + AnyBody::Empty, ); this = self.project(); this.flags.insert(Flags::STARTED | Flags::SHUTDOWN); @@ -909,14 +910,14 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into, + S::Error: Into>, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -1067,16 +1068,17 @@ mod tests { } } - fn ok_service() -> impl Service, Error = Error> { + fn ok_service() -> impl Service, Error = Error> + { fn_service(|_req: Request| ready(Ok::<_, Error>(Response::ok()))) } fn echo_path_service( - ) -> impl Service, Error = Error> { + ) -> impl Service, Error = Error> { fn_service(|req: Request| { let path = req.path().as_bytes(); ready(Ok::<_, Error>( - Response::ok().set_body(Body::from_slice(path)), + Response::ok().set_body(AnyBody::from_slice(path)), )) }) } diff --git a/actix-http/src/h1/encoder.rs b/actix-http/src/h1/encoder.rs index eaabcb687..254981123 100644 --- a/actix-http/src/h1/encoder.rs +++ b/actix-http/src/h1/encoder.rs @@ -6,14 +6,15 @@ use std::{cmp, io}; use bytes::{BufMut, BytesMut}; -use crate::body::BodySize; -use crate::config::ServiceConfig; -use crate::header::{map::Value, HeaderName}; -use crate::helpers; -use crate::http::header::{CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCODING}; -use crate::http::{HeaderMap, StatusCode, Version}; -use crate::message::{ConnectionType, RequestHeadType}; -use crate::response::Response; +use crate::{ + body::BodySize, + config::ServiceConfig, + header::{map::Value, HeaderMap, HeaderName}, + header::{CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCODING}, + helpers, + message::{ConnectionType, RequestHeadType}, + Response, StatusCode, Version, +}; const AVERAGE_HEADER_SIZE: usize = 30; @@ -287,7 +288,7 @@ impl MessageType for RequestHeadType { let head = self.as_ref(); dst.reserve(256 + head.headers.len() * AVERAGE_HEADER_SIZE); write!( - helpers::Writer(dst), + helpers::MutWriter(dst), "{} {} {}", head.method, head.uri.path_and_query().map(|u| u.as_str()).unwrap_or("/"), @@ -420,7 +421,7 @@ impl TransferEncoding { *eof = true; buf.extend_from_slice(b"0\r\n\r\n"); } else { - writeln!(helpers::Writer(buf), "{:X}\r", msg.len()) + writeln!(helpers::MutWriter(buf), "{:X}\r", msg.len()) .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; buf.reserve(msg.len() + 2); diff --git a/actix-http/src/h1/service.rs b/actix-http/src/h1/service.rs index 1ab85cbf3..dbad8cfac 100644 --- a/actix-http/src/h1/service.rs +++ b/actix-http/src/h1/service.rs @@ -1,7 +1,11 @@ -use std::marker::PhantomData; -use std::rc::Rc; -use std::task::{Context, Poll}; -use std::{fmt, net}; +use std::{ + error::Error as StdError, + fmt, + marker::PhantomData, + net, + rc::Rc, + task::{Context, Poll}, +}; use actix_codec::{AsyncRead, AsyncWrite, Framed}; use actix_rt::net::TcpStream; @@ -11,17 +15,15 @@ use actix_service::{ use actix_utils::future::ready; use futures_core::future::LocalBoxFuture; -use crate::body::MessageBody; -use crate::config::ServiceConfig; -use crate::error::{DispatchError, Error}; -use crate::request::Request; -use crate::response::Response; -use crate::service::HttpServiceHandler; -use crate::{ConnectCallback, OnConnectData}; +use crate::{ + body::{AnyBody, MessageBody}, + config::ServiceConfig, + error::DispatchError, + service::HttpServiceHandler, + ConnectCallback, OnConnectData, Request, Response, +}; -use super::codec::Codec; -use super::dispatcher::Dispatcher; -use super::{ExpectHandler, UpgradeHandler}; +use super::{codec::Codec, dispatcher::Dispatcher, ExpectHandler, UpgradeHandler}; /// `ServiceFactory` implementation for HTTP1 transport pub struct H1Service { @@ -36,7 +38,7 @@ pub struct H1Service { impl H1Service where S: ServiceFactory, - S::Error: Into, + S::Error: Into>, S::InitError: fmt::Debug, S::Response: Into>, B: MessageBody, @@ -61,21 +63,21 @@ impl H1Service where S: ServiceFactory, S::Future: 'static, - S::Error: Into, + S::Error: Into>, S::InitError: fmt::Debug, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { /// Create simple tcp stream service @@ -110,16 +112,16 @@ mod openssl { where S: ServiceFactory, S::Future: 'static, - S::Error: Into, + S::Error: Into>, S::InitError: fmt::Debug, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory< @@ -128,7 +130,7 @@ mod openssl { Response = (), >, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { /// Create openssl based service @@ -170,16 +172,16 @@ mod rustls { where S: ServiceFactory, S::Future: 'static, - S::Error: Into, + S::Error: Into>, S::InitError: fmt::Debug, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory< @@ -188,7 +190,7 @@ mod rustls { Response = (), >, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { /// Create rustls based service @@ -217,7 +219,7 @@ mod rustls { impl H1Service where S: ServiceFactory, - S::Error: Into, + S::Error: Into>, S::Response: Into>, S::InitError: fmt::Debug, B: MessageBody, @@ -225,7 +227,7 @@ where pub fn expect(self, expect: X1) -> H1Service where X1: ServiceFactory, - X1::Error: Into, + X1::Error: Into>, X1::InitError: fmt::Debug, { H1Service { @@ -268,21 +270,21 @@ where S: ServiceFactory, S::Future: 'static, - S::Error: Into, + S::Error: Into>, S::Response: Into>, S::InitError: fmt::Debug, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { type Response = (); @@ -338,17 +340,17 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into, + S::Error: Into>, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, { type Response = (); type Error = DispatchError; diff --git a/actix-http/src/h1/utils.rs b/actix-http/src/h1/utils.rs index 90e44daa4..523e652fd 100644 --- a/actix-http/src/h1/utils.rs +++ b/actix-http/src/h1/utils.rs @@ -81,7 +81,9 @@ where let _ = this.body.take(); } let framed = this.framed.as_mut().as_pin_mut().unwrap(); - framed.write(Message::Chunk(item))?; + framed.write(Message::Chunk(item)).map_err(|err| { + Error::new_send_response().with_cause(err) + })?; } Poll::Pending => body_ready = false, } @@ -92,7 +94,10 @@ where // flush write buffer if !framed.is_write_buf_empty() { - match framed.flush(cx)? { + match framed + .flush(cx) + .map_err(|err| Error::new_send_response().with_cause(err))? + { Poll::Ready(_) => { if body_ready { continue; @@ -106,7 +111,9 @@ where // send response if let Some(res) = this.res.take() { - framed.write(res)?; + framed + .write(res) + .map_err(|err| Error::new_send_response().with_cause(err))?; continue; } diff --git a/actix-http/src/h2/dispatcher.rs b/actix-http/src/h2/dispatcher.rs index baff20e51..ea149b1e0 100644 --- a/actix-http/src/h2/dispatcher.rs +++ b/actix-http/src/h2/dispatcher.rs @@ -1,5 +1,6 @@ use std::{ cmp, + error::Error as StdError, future::Future, marker::PhantomData, net, @@ -18,15 +19,12 @@ use http::header::{HeaderValue, CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCOD use log::{error, trace}; use pin_project_lite::pin_project; -use crate::body::{BodySize, MessageBody}; -use crate::config::ServiceConfig; -use crate::error::Error; -use crate::message::ResponseHead; -use crate::payload::Payload; -use crate::request::Request; -use crate::response::Response; -use crate::service::HttpFlow; -use crate::OnConnectData; +use crate::{ + body::{AnyBody, BodySize, MessageBody}, + config::ServiceConfig, + service::HttpFlow, + OnConnectData, Payload, Request, Response, ResponseHead, +}; const CHUNK_SIZE: usize = 16_384; @@ -66,12 +64,12 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into, + S::Error: Into>, S::Future: 'static, S::Response: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, { type Output = Result<(), crate::error::DispatchError>; @@ -106,7 +104,7 @@ where let res = match fut.await { Ok(res) => handle_response(res.into(), tx, config).await, Err(err) => { - let res = Response::from_error(err.into()); + let res: Response = err.into(); handle_response(res, tx, config).await } }; @@ -133,7 +131,7 @@ where enum DispatchError { SendResponse(h2::Error), SendData(h2::Error), - ResponseBody(Error), + ResponseBody(Box), } async fn handle_response( @@ -143,7 +141,7 @@ async fn handle_response( ) -> Result<(), DispatchError> where B: MessageBody, - B::Error: Into, + B::Error: Into>, { let (res, body) = res.replace_body(()); diff --git a/actix-http/src/h2/service.rs b/actix-http/src/h2/service.rs index 3a6d535d9..09e24045b 100644 --- a/actix-http/src/h2/service.rs +++ b/actix-http/src/h2/service.rs @@ -1,8 +1,12 @@ -use std::future::Future; -use std::marker::PhantomData; -use std::pin::Pin; -use std::task::{Context, Poll}; -use std::{net, rc::Rc}; +use std::{ + error::Error as StdError, + future::Future, + marker::PhantomData, + net, + pin::Pin, + rc::Rc, + task::{Context, Poll}, +}; use actix_codec::{AsyncRead, AsyncWrite}; use actix_rt::net::TcpStream; @@ -13,16 +17,16 @@ use actix_service::{ use actix_utils::future::ready; use bytes::Bytes; use futures_core::{future::LocalBoxFuture, ready}; -use h2::server::{handshake, Handshake}; +use h2::server::{handshake as h2_handshake, Handshake as H2Handshake}; use log::error; -use crate::body::MessageBody; -use crate::config::ServiceConfig; -use crate::error::{DispatchError, Error}; -use crate::request::Request; -use crate::response::Response; -use crate::service::HttpFlow; -use crate::{ConnectCallback, OnConnectData}; +use crate::{ + body::{AnyBody, MessageBody}, + config::ServiceConfig, + error::DispatchError, + service::HttpFlow, + ConnectCallback, OnConnectData, Request, Response, +}; use super::dispatcher::Dispatcher; @@ -37,12 +41,12 @@ pub struct H2Service { impl H2Service where S: ServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { /// Create new `H2Service` instance with config. pub(crate) fn with_config>( @@ -68,12 +72,12 @@ impl H2Service where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { /// Create plain TCP based service pub fn tcp( @@ -107,12 +111,12 @@ mod openssl { where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { /// Create OpenSSL based service pub fn openssl( @@ -153,12 +157,12 @@ mod rustls { where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { /// Create Rustls based service pub fn rustls( @@ -197,12 +201,12 @@ where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { type Response = (); type Error = DispatchError; @@ -237,7 +241,7 @@ where impl H2ServiceHandler where S: Service, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Future: 'static, S::Response: Into> + 'static, B: MessageBody + 'static, @@ -260,11 +264,11 @@ impl Service<(T, Option)> for H2ServiceHandler, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Future: 'static, S::Response: Into> + 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { type Response = (); type Error = DispatchError; @@ -288,7 +292,7 @@ where Some(self.cfg.clone()), addr, on_connect_data, - handshake(io), + h2_handshake(io), ), } } @@ -305,7 +309,7 @@ where Option, Option, OnConnectData, - Handshake, + H2Handshake, ), } @@ -313,7 +317,7 @@ pub struct H2ServiceHandlerResponse where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Future: 'static, S::Response: Into> + 'static, B: MessageBody + 'static, @@ -325,11 +329,11 @@ impl Future for H2ServiceHandlerResponse where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Future: 'static, S::Response: Into> + 'static, B: MessageBody, - B::Error: Into, + B::Error: Into>, { type Output = Result<(), DispatchError>; diff --git a/actix-http/src/helpers.rs b/actix-http/src/helpers.rs index 34bb989f9..cba94d9b8 100644 --- a/actix-http/src/helpers.rs +++ b/actix-http/src/helpers.rs @@ -27,7 +27,9 @@ pub(crate) fn write_status_line(version: Version, n: u16, buf: &mut B buf.put_u8(b' '); } -/// NOTE: bytes object has to contain enough space +/// Write out content length header. +/// +/// Buffer must to contain enough space or be implicitly extendable. pub fn write_content_length(n: u64, buf: &mut B) { if n == 0 { buf.put_slice(b"\r\ncontent-length: 0\r\n"); @@ -41,11 +43,15 @@ pub fn write_content_length(n: u64, buf: &mut B) { buf.put_slice(b"\r\n"); } -// TODO: bench why this is needed vs Buf::writer -/// An `io` writer for a `BufMut` that should only be used once and on an empty buffer. -pub(crate) struct Writer<'a, B>(pub &'a mut B); +/// An `io::Write`r that only requires mutable reference and assumes that there is space available +/// in the buffer for every write operation or that it can be extended implicitly (like +/// `bytes::BytesMut`, for example). +/// +/// This is slightly faster (~10%) than `bytes::buf::Writer` in such cases because it does not +/// perform a remaining length check before writing. +pub(crate) struct MutWriter<'a, B>(pub(crate) &'a mut B); -impl<'a, B> io::Write for Writer<'a, B> +impl<'a, B> io::Write for MutWriter<'a, B> where B: BufMut, { diff --git a/actix-http/src/lib.rs b/actix-http/src/lib.rs index 7c2c3b4e3..9f94faaa5 100644 --- a/actix-http/src/lib.rs +++ b/actix-http/src/lib.rs @@ -54,7 +54,7 @@ pub mod ws; pub use self::builder::HttpServiceBuilder; pub use self::config::{KeepAlive, ServiceConfig}; -pub use self::error::{Error, ResponseError}; +pub use self::error::Error; pub use self::extensions::Extensions; pub use self::header::ContentEncoding; pub use self::http_message::HttpMessage; diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index bcfa65732..2aa38c153 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -8,7 +8,7 @@ use std::{ use bytes::{Bytes, BytesMut}; use crate::{ - body::{Body, MessageBody}, + body::{AnyBody, MessageBody}, error::Error, extensions::Extensions, http::{HeaderMap, StatusCode}, @@ -22,13 +22,13 @@ pub struct Response { pub(crate) body: B, } -impl Response { +impl Response { /// Constructs a new response with default body. #[inline] - pub fn new(status: StatusCode) -> Response { + pub fn new(status: StatusCode) -> Self { Response { head: BoxedResponseHead::new(status), - body: Body::Empty, + body: AnyBody::Empty, } } @@ -43,40 +43,29 @@ impl Response { /// Constructs a new response with status 200 OK. #[inline] - pub fn ok() -> Response { + pub fn ok() -> Self { Response::new(StatusCode::OK) } /// Constructs a new response with status 400 Bad Request. #[inline] - pub fn bad_request() -> Response { + pub fn bad_request() -> Self { Response::new(StatusCode::BAD_REQUEST) } /// Constructs a new response with status 404 Not Found. #[inline] - pub fn not_found() -> Response { + pub fn not_found() -> Self { Response::new(StatusCode::NOT_FOUND) } /// Constructs a new response with status 500 Internal Server Error. #[inline] - pub fn internal_server_error() -> Response { + pub fn internal_server_error() -> Self { Response::new(StatusCode::INTERNAL_SERVER_ERROR) } // end shortcuts - - /// Constructs a new response from an error. - #[inline] - pub fn from_error(error: impl Into) -> Response { - let error = error.into(); - let resp = error.as_response_error().error_response(); - if resp.head.status == StatusCode::INTERNAL_SERVER_ERROR { - debug!("Internal Server Error: {:?}", error); - } - resp - } } impl Response { @@ -209,7 +198,6 @@ impl Response { impl fmt::Debug for Response where B: MessageBody, - B::Error: Into, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let res = writeln!( @@ -235,7 +223,9 @@ impl Default for Response { } } -impl>, E: Into> From> for Response { +impl>, E: Into> From> + for Response +{ fn from(res: Result) -> Self { match res { Ok(val) => val.into(), @@ -244,13 +234,19 @@ impl>, E: Into> From> for Response for Response { +impl From for Response { fn from(mut builder: ResponseBuilder) -> Self { builder.finish() } } -impl From<&'static str> for Response { +impl From for Response { + fn from(val: std::convert::Infallible) -> Self { + match val {} + } +} + +impl From<&'static str> for Response { fn from(val: &'static str) -> Self { Response::build(StatusCode::OK) .content_type(mime::TEXT_PLAIN_UTF_8) @@ -258,7 +254,7 @@ impl From<&'static str> for Response { } } -impl From<&'static [u8]> for Response { +impl From<&'static [u8]> for Response { fn from(val: &'static [u8]) -> Self { Response::build(StatusCode::OK) .content_type(mime::APPLICATION_OCTET_STREAM) @@ -266,7 +262,7 @@ impl From<&'static [u8]> for Response { } } -impl From for Response { +impl From for Response { fn from(val: String) -> Self { Response::build(StatusCode::OK) .content_type(mime::TEXT_PLAIN_UTF_8) @@ -274,7 +270,7 @@ impl From for Response { } } -impl<'a> From<&'a String> for Response { +impl<'a> From<&'a String> for Response { fn from(val: &'a String) -> Self { Response::build(StatusCode::OK) .content_type(mime::TEXT_PLAIN_UTF_8) @@ -282,7 +278,7 @@ impl<'a> From<&'a String> for Response { } } -impl From for Response { +impl From for Response { fn from(val: Bytes) -> Self { Response::build(StatusCode::OK) .content_type(mime::APPLICATION_OCTET_STREAM) @@ -290,7 +286,7 @@ impl From for Response { } } -impl From for Response { +impl From for Response { fn from(val: BytesMut) -> Self { Response::build(StatusCode::OK) .content_type(mime::APPLICATION_OCTET_STREAM) @@ -301,7 +297,6 @@ impl From for Response { #[cfg(test)] mod tests { use super::*; - use crate::body::Body; use crate::http::header::{HeaderValue, CONTENT_TYPE, COOKIE}; #[test] @@ -316,7 +311,7 @@ mod tests { #[test] fn test_into_response() { - let resp: Response = "test".into(); + let resp: Response = "test".into(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), @@ -325,7 +320,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().get_ref(), b"test"); - let resp: Response = b"test".as_ref().into(); + let resp: Response = b"test".as_ref().into(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), @@ -334,7 +329,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().get_ref(), b"test"); - let resp: Response = "test".to_owned().into(); + let resp: Response = "test".to_owned().into(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), @@ -343,7 +338,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); assert_eq!(resp.body().get_ref(), b"test"); - let resp: Response = (&"test".to_owned()).into(); + let resp: Response = (&"test".to_owned()).into(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), @@ -353,7 +348,7 @@ mod tests { assert_eq!(resp.body().get_ref(), b"test"); let b = Bytes::from_static(b"test"); - let resp: Response = b.into(); + let resp: Response = b.into(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), @@ -363,7 +358,7 @@ mod tests { assert_eq!(resp.body().get_ref(), b"test"); let b = Bytes::from_static(b"test"); - let resp: Response = b.into(); + let resp: Response = b.into(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), @@ -373,7 +368,7 @@ mod tests { assert_eq!(resp.body().get_ref(), b"test"); let b = BytesMut::from("test"); - let resp: Response = b.into(); + let resp: Response = b.into(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get(CONTENT_TYPE).unwrap(), diff --git a/actix-http/src/response_builder.rs b/actix-http/src/response_builder.rs index df9079d70..e46d9a28c 100644 --- a/actix-http/src/response_builder.rs +++ b/actix-http/src/response_builder.rs @@ -2,6 +2,7 @@ use std::{ cell::{Ref, RefMut}, + error::Error as StdError, fmt, future::Future, pin::Pin, @@ -13,7 +14,7 @@ use bytes::Bytes; use futures_core::Stream; use crate::{ - body::{Body, BodyStream}, + body::{AnyBody, BodyStream}, error::{Error, HttpError}, header::{self, IntoHeaderPair, IntoHeaderValue}, message::{BoxedResponseHead, ConnectionType, ResponseHead}, @@ -235,9 +236,9 @@ impl ResponseBuilder { /// /// This `ResponseBuilder` will be left in a useless state. #[inline] - pub fn body>(&mut self, body: B) -> Response { + pub fn body>(&mut self, body: B) -> Response { self.message_body(body.into()) - .unwrap_or_else(Response::from_error) + .unwrap_or_else(Response::from) } /// Generate response with a body. @@ -245,7 +246,7 @@ impl ResponseBuilder { /// This `ResponseBuilder` will be left in a useless state. pub fn message_body(&mut self, body: B) -> Result, Error> { if let Some(err) = self.err.take() { - return Err(err.into()); + return Err(Error::new_http().with_cause(err)); } let head = self.head.take().expect("cannot reuse response builder"); @@ -256,20 +257,20 @@ impl ResponseBuilder { /// /// This `ResponseBuilder` will be left in a useless state. #[inline] - pub fn streaming(&mut self, stream: S) -> Response + pub fn streaming(&mut self, stream: S) -> Response where - S: Stream> + Unpin + 'static, - E: Into + 'static, + S: Stream> + 'static, + E: Into> + 'static, { - self.body(Body::from_message(BodyStream::new(stream))) + self.body(AnyBody::from_message(BodyStream::new(stream))) } /// Generate response with an empty body. /// /// This `ResponseBuilder` will be left in a useless state. #[inline] - pub fn finish(&mut self) -> Response { - self.body(Body::Empty) + pub fn finish(&mut self) -> Response { + self.body(AnyBody::Empty) } /// Create an owned `ResponseBuilder`, leaving the original in a useless state. @@ -327,7 +328,7 @@ impl<'a> From<&'a ResponseHead> for ResponseBuilder { } impl Future for ResponseBuilder { - type Output = Result, Error>; + type Output = Result, Error>; fn poll(mut self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll { Poll::Ready(Ok(self.finish())) diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index 1c81e7568..afe47bf2d 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -1,4 +1,5 @@ use std::{ + error::Error as StdError, fmt, future::Future, marker::PhantomData, @@ -8,6 +9,7 @@ use std::{ task::{Context, Poll}, }; +use ::h2::server::{handshake as h2_handshake, Handshake as H2Handshake}; use actix_codec::{AsyncRead, AsyncWrite, Framed}; use actix_rt::net::TcpStream; use actix_service::{ @@ -15,16 +17,15 @@ use actix_service::{ }; use bytes::Bytes; use futures_core::{future::LocalBoxFuture, ready}; -use h2::server::{handshake, Handshake}; use pin_project::pin_project; -use crate::body::MessageBody; -use crate::builder::HttpServiceBuilder; -use crate::config::{KeepAlive, ServiceConfig}; -use crate::error::{DispatchError, Error}; -use crate::request::Request; -use crate::response::Response; -use crate::{h1, h2::Dispatcher, ConnectCallback, OnConnectData, Protocol}; +use crate::{ + body::{AnyBody, MessageBody}, + builder::HttpServiceBuilder, + config::{KeepAlive, ServiceConfig}, + error::DispatchError, + h1, h2, ConnectCallback, OnConnectData, Protocol, Request, Response, +}; /// A `ServiceFactory` for HTTP/1.1 or HTTP/2 protocol. pub struct HttpService { @@ -39,7 +40,7 @@ pub struct HttpService { impl HttpService where S: ServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, @@ -54,12 +55,12 @@ where impl HttpService where S: ServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { /// Create new `HttpService` instance. pub fn new>(service: F) -> Self { @@ -94,7 +95,7 @@ where impl HttpService where S: ServiceFactory, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, @@ -108,7 +109,7 @@ where pub fn expect(self, expect: X1) -> HttpService where X1: ServiceFactory, - X1::Error: Into, + X1::Error: Into>, X1::InitError: fmt::Debug, { HttpService { @@ -152,17 +153,17 @@ impl HttpService where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory< @@ -171,7 +172,7 @@ where Response = (), >, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { /// Create simple tcp stream service @@ -204,17 +205,17 @@ mod openssl { where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory< @@ -223,7 +224,7 @@ mod openssl { Response = (), >, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { /// Create openssl based service @@ -272,17 +273,17 @@ mod rustls { where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory< @@ -291,7 +292,7 @@ mod rustls { Response = (), >, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { /// Create rustls based service @@ -338,22 +339,22 @@ where S: ServiceFactory, S::Future: 'static, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, X: ServiceFactory, X::Future: 'static, - X::Error: Into, + X::Error: Into>, X::InitError: fmt::Debug, U: ServiceFactory<(Request, Framed), Config = (), Response = ()>, U::Future: 'static, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, U::InitError: fmt::Debug, { type Response = (); @@ -416,11 +417,11 @@ where impl HttpServiceHandler where S: Service, - S::Error: Into, + S::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed)>, - U::Error: Into, + U::Error: Into>, { pub(super) fn new( cfg: ServiceConfig, @@ -437,7 +438,10 @@ where } } - pub(super) fn _poll_ready(&self, cx: &mut Context<'_>) -> Poll> { + pub(super) fn _poll_ready( + &self, + cx: &mut Context<'_>, + ) -> Poll>> { ready!(self.flow.expect.poll_ready(cx).map_err(Into::into))?; ready!(self.flow.service.poll_ready(cx).map_err(Into::into))?; @@ -473,18 +477,18 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Future: 'static, S::Response: Into> + 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, - U::Error: fmt::Display + Into, + U::Error: fmt::Display + Into>, { type Response = (); type Error = DispatchError; @@ -507,7 +511,7 @@ where match proto { Protocol::Http2 => HttpServiceHandlerResponse { state: State::H2Handshake(Some(( - handshake(io), + h2_handshake(io), self.cfg.clone(), self.flow.clone(), on_connect_data, @@ -537,22 +541,22 @@ where S: Service, S::Future: 'static, - S::Error: Into, + S::Error: Into>, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, { H1(#[pin] h1::Dispatcher), - H2(#[pin] Dispatcher), + H2(#[pin] h2::Dispatcher), H2Handshake( Option<( - Handshake, + H2Handshake, ServiceConfig, Rc>, OnConnectData, @@ -567,15 +571,15 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Future: 'static, S::Response: Into> + 'static, B: MessageBody, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -589,15 +593,15 @@ where T: AsyncRead + AsyncWrite + Unpin, S: Service, - S::Error: Into + 'static, + S::Error: Into> + 'static, S::Future: 'static, S::Response: Into> + 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, X: Service, - X::Error: Into, + X::Error: Into>, U: Service<(Request, Framed), Response = ()>, U::Error: fmt::Display, @@ -613,13 +617,15 @@ where Ok(conn) => { let (_, cfg, srv, on_connect_data, peer_addr) = data.take().unwrap(); - self.as_mut().project().state.set(State::H2(Dispatcher::new( - srv, - conn, - on_connect_data, - cfg, - peer_addr, - ))); + self.as_mut().project().state.set(State::H2( + h2::Dispatcher::new( + srv, + conn, + on_connect_data, + cfg, + peer_addr, + ), + )); self.poll(cx) } Err(err) => { diff --git a/actix-http/src/ws/dispatcher.rs b/actix-http/src/ws/dispatcher.rs index 576851139..f49cbe5d4 100644 --- a/actix-http/src/ws/dispatcher.rs +++ b/actix-http/src/ws/dispatcher.rs @@ -72,7 +72,7 @@ mod inner { use actix_codec::{AsyncRead, AsyncWrite, Decoder, Encoder, Framed}; - use crate::ResponseError; + use crate::{body::AnyBody, Response}; /// Framed transport errors pub enum DispatcherError @@ -136,13 +136,16 @@ mod inner { } } - impl ResponseError for DispatcherError + impl From> for Response where E: fmt::Debug + fmt::Display, U: Encoder + Decoder, >::Error: fmt::Debug, ::Error: fmt::Debug, { + fn from(err: DispatcherError) -> Self { + Response::internal_server_error().set_body(AnyBody::from(err.to_string())) + } } /// Message type wrapper for signalling end of message stream. diff --git a/actix-http/src/ws/mod.rs b/actix-http/src/ws/mod.rs index 22df2b4ff..7df924cf5 100644 --- a/actix-http/src/ws/mod.rs +++ b/actix-http/src/ws/mod.rs @@ -9,8 +9,8 @@ use derive_more::{Display, Error, From}; use http::{header, Method, StatusCode}; use crate::{ - body::Body, error::ResponseError, header::HeaderValue, message::RequestHead, - response::Response, ResponseBuilder, + body::AnyBody, header::HeaderValue, message::RequestHead, response::Response, + ResponseBuilder, }; mod codec; @@ -25,7 +25,7 @@ pub use self::frame::Parser; pub use self::proto::{hash_key, CloseCode, CloseReason, OpCode}; /// WebSocket protocol errors. -#[derive(Debug, Display, From, Error)] +#[derive(Debug, Display, Error, From)] pub enum ProtocolError { /// Received an unmasked frame from client. #[display(fmt = "Received an unmasked frame from client.")] @@ -68,10 +68,8 @@ pub enum ProtocolError { Io(io::Error), } -impl ResponseError for ProtocolError {} - /// WebSocket handshake errors -#[derive(PartialEq, Debug, Display)] +#[derive(Debug, PartialEq, Display, Error)] pub enum HandshakeError { /// Only get method is allowed. #[display(fmt = "Method not allowed.")] @@ -98,44 +96,55 @@ pub enum HandshakeError { BadWebsocketKey, } -impl ResponseError for HandshakeError { - fn error_response(&self) -> Response { - match self { +impl From<&HandshakeError> for Response { + fn from(err: &HandshakeError) -> Self { + match err { HandshakeError::GetMethodRequired => { - Response::build(StatusCode::METHOD_NOT_ALLOWED) - .insert_header((header::ALLOW, "GET")) - .finish() + let mut res = Response::new(StatusCode::METHOD_NOT_ALLOWED); + res.headers_mut() + .insert(header::ALLOW, HeaderValue::from_static("GET")); + res } HandshakeError::NoWebsocketUpgrade => { - Response::build(StatusCode::BAD_REQUEST) - .reason("No WebSocket Upgrade header found") - .finish() + let mut res = Response::bad_request(); + res.head_mut().reason = Some("No WebSocket Upgrade header found"); + res } HandshakeError::NoConnectionUpgrade => { - Response::build(StatusCode::BAD_REQUEST) - .reason("No Connection upgrade") - .finish() + let mut res = Response::bad_request(); + res.head_mut().reason = Some("No Connection upgrade"); + res } - HandshakeError::NoVersionHeader => Response::build(StatusCode::BAD_REQUEST) - .reason("WebSocket version header is required") - .finish(), + HandshakeError::NoVersionHeader => { + let mut res = Response::bad_request(); + res.head_mut().reason = Some("WebSocket version header is required"); + res + } HandshakeError::UnsupportedVersion => { - Response::build(StatusCode::BAD_REQUEST) - .reason("Unsupported WebSocket version") - .finish() + let mut res = Response::bad_request(); + res.head_mut().reason = Some("Unsupported WebSocket version"); + res } - HandshakeError::BadWebsocketKey => Response::build(StatusCode::BAD_REQUEST) - .reason("Handshake error") - .finish(), + HandshakeError::BadWebsocketKey => { + let mut res = Response::bad_request(); + res.head_mut().reason = Some("Handshake error"); + res + } } } } +impl From for Response { + fn from(err: HandshakeError) -> Self { + (&err).into() + } +} + /// Verify WebSocket handshake request and create handshake response. pub fn handshake(req: &RequestHead) -> Result { verify_handshake(req)?; @@ -213,7 +222,7 @@ pub fn handshake_response(req: &RequestHead) -> ResponseBuilder { #[cfg(test)] mod tests { use super::*; - use crate::test::TestRequest; + use crate::{body::AnyBody, test::TestRequest}; use http::{header, Method}; #[test] @@ -327,18 +336,18 @@ mod tests { } #[test] - fn test_wserror_http_response() { - let resp = HandshakeError::GetMethodRequired.error_response(); + fn test_ws_error_http_response() { + let resp: Response = HandshakeError::GetMethodRequired.into(); assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); - let resp = HandshakeError::NoWebsocketUpgrade.error_response(); + let resp: Response = HandshakeError::NoWebsocketUpgrade.into(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp = HandshakeError::NoConnectionUpgrade.error_response(); + let resp: Response = HandshakeError::NoConnectionUpgrade.into(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp = HandshakeError::NoVersionHeader.error_response(); + let resp: Response = HandshakeError::NoVersionHeader.into(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp = HandshakeError::UnsupportedVersion.error_response(); + let resp: Response = HandshakeError::UnsupportedVersion.into(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let resp = HandshakeError::BadWebsocketKey.error_response(); + let resp: Response = HandshakeError::BadWebsocketKey.into(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); } } diff --git a/actix-http/tests/test_client.rs b/actix-http/tests/test_client.rs index 4bd7dbe14..414266d81 100644 --- a/actix-http/tests/test_client.rs +++ b/actix-http/tests/test_client.rs @@ -1,5 +1,7 @@ +use std::convert::Infallible; + use actix_http::{ - http, http::StatusCode, HttpMessage, HttpService, Request, Response, ResponseError, + body::AnyBody, http, http::StatusCode, HttpMessage, HttpService, Request, Response, }; use actix_http_test::test_server; use actix_service::ServiceFactoryExt; @@ -34,7 +36,7 @@ const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ async fn test_h1_v2() { let srv = test_server(move || { HttpService::build() - .finish(|_| future::ok::<_, ()>(Response::ok().set_body(STR))) + .finish(|_| future::ok::<_, Infallible>(Response::ok().set_body(STR))) .tcp() }) .await; @@ -62,7 +64,7 @@ async fn test_h1_v2() { async fn test_connection_close() { let srv = test_server(move || { HttpService::build() - .finish(|_| future::ok::<_, ()>(Response::ok().set_body(STR))) + .finish(|_| future::ok::<_, Infallible>(Response::ok().set_body(STR))) .tcp() .map(|_| ()) }) @@ -76,11 +78,11 @@ async fn test_connection_close() { async fn test_with_query_parameter() { let srv = test_server(move || { HttpService::build() - .finish(|req: Request| { + .finish(|req: Request| async move { if req.uri().query().unwrap().contains("qp=") { - future::ok::<_, ()>(Response::ok()) + Ok::<_, Infallible>(Response::ok()) } else { - future::ok::<_, ()>(Response::bad_request()) + Ok(Response::bad_request()) } }) .tcp() @@ -97,9 +99,9 @@ async fn test_with_query_parameter() { #[display(fmt = "expect failed")] struct ExpectFailed; -impl ResponseError for ExpectFailed { - fn status_code(&self) -> StatusCode { - StatusCode::EXPECTATION_FAILED +impl From for Response { + fn from(_: ExpectFailed) -> Self { + Response::new(StatusCode::EXPECTATION_FAILED) } } @@ -123,7 +125,7 @@ async fn test_h1_expect() { let str = std::str::from_utf8(&buf).unwrap(); assert_eq!(str, "expect body"); - Ok::<_, ()>(Response::ok()) + Ok::<_, Infallible>(Response::ok()) }) .tcp() }) diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index d3a3bea3b..a58d0cc70 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -2,16 +2,16 @@ extern crate tls_openssl as openssl; -use std::io; +use std::{convert::Infallible, io}; use actix_http::{ - body::{Body, SizedStream}, + body::{AnyBody, Body, SizedStream}, error::PayloadError, http::{ header::{self, HeaderName, HeaderValue}, Method, StatusCode, Version, }, - Error, HttpMessage, HttpService, Request, Response, ResponseError, + Error, HttpMessage, HttpService, Request, Response, }; use actix_http_test::test_server; use actix_service::{fn_service, ServiceFactoryExt}; @@ -136,7 +136,7 @@ async fn test_h2_content_length() { StatusCode::OK, StatusCode::NOT_FOUND, ]; - ok::<_, ()>(Response::new(statuses[idx])) + ok::<_, Infallible>(Response::new(statuses[idx])) }) .openssl(tls_config()) .map_err(|_| ()) @@ -206,7 +206,7 @@ async fn test_h2_headers() { TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST ", )); } - ok::<_, ()>(builder.body(data.clone())) + ok::<_, Infallible>(builder.body(data.clone())) }) .openssl(tls_config()) .map_err(|_| ()) @@ -246,7 +246,7 @@ const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ async fn test_h2_body2() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .openssl(tls_config()) .map_err(|_| ()) }) @@ -264,7 +264,7 @@ async fn test_h2_body2() { async fn test_h2_head_empty() { let mut srv = test_server(move || { HttpService::build() - .finish(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .finish(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .openssl(tls_config()) .map_err(|_| ()) }) @@ -288,7 +288,7 @@ async fn test_h2_head_empty() { async fn test_h2_head_binary() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .openssl(tls_config()) .map_err(|_| ()) }) @@ -311,7 +311,7 @@ async fn test_h2_head_binary() { async fn test_h2_head_binary2() { let srv = test_server(move || { HttpService::build() - .h2(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .openssl(tls_config()) .map_err(|_| ()) }) @@ -330,9 +330,12 @@ async fn test_h2_head_binary2() { async fn test_h2_body_length() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| { - let body = once(ok(Bytes::from_static(STR.as_ref()))); - ok::<_, ()>( + .h2(|_| async { + let body = once(async { + Ok::<_, Infallible>(Bytes::from_static(STR.as_ref())) + }); + + Ok::<_, Infallible>( Response::ok().set_body(SizedStream::new(STR.len() as u64, body)), ) }) @@ -355,7 +358,7 @@ async fn test_h2_body_chunked_explicit() { HttpService::build() .h2(|_| { let body = once(ok::<_, Error>(Bytes::from_static(STR.as_ref()))); - ok::<_, ()>( + ok::<_, Infallible>( Response::build(StatusCode::OK) .insert_header((header::TRANSFER_ENCODING, "chunked")) .streaming(body), @@ -383,7 +386,7 @@ async fn test_h2_response_http_error_handling() { HttpService::build() .h2(fn_service(|_| { let broken_header = Bytes::from_static(b"\0\0\0"); - ok::<_, ()>( + ok::<_, Infallible>( Response::build(StatusCode::OK) .insert_header((header::CONTENT_TYPE, broken_header)) .body(STR), @@ -399,16 +402,19 @@ async fn test_h2_response_http_error_handling() { // read response let bytes = srv.load_body(response).await.unwrap(); - assert_eq!(bytes, Bytes::from_static(b"failed to parse header value")); + assert_eq!( + bytes, + Bytes::from_static(b"error processing HTTP: failed to parse header value") + ); } #[derive(Debug, Display, Error)] #[display(fmt = "error")] struct BadRequest; -impl ResponseError for BadRequest { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST +impl From for Response { + fn from(err: BadRequest) -> Self { + Response::build(StatusCode::BAD_REQUEST).body(err.to_string()) } } @@ -439,7 +445,7 @@ async fn test_h2_on_connect() { }) .h2(|req: Request| { assert!(req.extensions().contains::()); - ok::<_, ()>(Response::ok()) + ok::<_, Infallible>(Response::ok()) }) .openssl(tls_config()) .map_err(|_| ()) diff --git a/actix-http/tests/test_rustls.rs b/actix-http/tests/test_rustls.rs index eec417541..cb7c77ad6 100644 --- a/actix-http/tests/test_rustls.rs +++ b/actix-http/tests/test_rustls.rs @@ -2,14 +2,21 @@ extern crate tls_rustls as rustls; +use std::{ + convert::Infallible, + io::{self, BufReader, Write}, + net::{SocketAddr, TcpStream as StdTcpStream}, + sync::Arc, +}; + use actix_http::{ - body::{Body, SizedStream}, + body::{AnyBody, Body, SizedStream}, error::PayloadError, http::{ header::{self, HeaderName, HeaderValue}, Method, StatusCode, Version, }, - Error, HttpService, Request, Response, ResponseError, + Error, HttpService, Request, Response, }; use actix_http_test::test_server; use actix_service::{fn_factory_with_config, fn_service}; @@ -24,12 +31,6 @@ use rustls::{ }; use webpki::DNSNameRef; -use std::{ - io::{self, BufReader, Write}, - net::{SocketAddr, TcpStream as StdTcpStream}, - sync::Arc, -}; - async fn load_body(mut stream: S) -> Result where S: Stream> + Unpin, @@ -173,7 +174,7 @@ async fn test_h2_content_length() { StatusCode::OK, StatusCode::NOT_FOUND, ]; - ok::<_, ()>(Response::new(statuses[indx])) + ok::<_, Infallible>(Response::new(statuses[indx])) }) .rustls(tls_config()) }) @@ -242,7 +243,7 @@ async fn test_h2_headers() { TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST ", )); } - ok::<_, ()>(config.body(data.clone())) + ok::<_, Infallible>(config.body(data.clone())) }) .rustls(tls_config()) }).await; @@ -281,7 +282,7 @@ const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ async fn test_h2_body2() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .rustls(tls_config()) }) .await; @@ -298,7 +299,7 @@ async fn test_h2_body2() { async fn test_h2_head_empty() { let mut srv = test_server(move || { HttpService::build() - .finish(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .finish(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .rustls(tls_config()) }) .await; @@ -324,7 +325,7 @@ async fn test_h2_head_empty() { async fn test_h2_head_binary() { let mut srv = test_server(move || { HttpService::build() - .h2(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .rustls(tls_config()) }) .await; @@ -349,7 +350,7 @@ async fn test_h2_head_binary() { async fn test_h2_head_binary2() { let srv = test_server(move || { HttpService::build() - .h2(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .rustls(tls_config()) }) .await; @@ -371,8 +372,8 @@ async fn test_h2_body_length() { let mut srv = test_server(move || { HttpService::build() .h2(|_| { - let body = once(ok(Bytes::from_static(STR.as_ref()))); - ok::<_, ()>( + let body = once(ok::<_, Infallible>(Bytes::from_static(STR.as_ref()))); + ok::<_, Infallible>( Response::ok().set_body(SizedStream::new(STR.len() as u64, body)), ) }) @@ -394,7 +395,7 @@ async fn test_h2_body_chunked_explicit() { HttpService::build() .h2(|_| { let body = once(ok::<_, Error>(Bytes::from_static(STR.as_ref()))); - ok::<_, ()>( + ok::<_, Infallible>( Response::build(StatusCode::OK) .insert_header((header::TRANSFER_ENCODING, "chunked")) .streaming(body), @@ -420,9 +421,9 @@ async fn test_h2_response_http_error_handling() { let mut srv = test_server(move || { HttpService::build() .h2(fn_factory_with_config(|_: ()| { - ok::<_, ()>(fn_service(|_| { + ok::<_, Infallible>(fn_service(|_| { let broken_header = Bytes::from_static(b"\0\0\0"); - ok::<_, ()>( + ok::<_, Infallible>( Response::build(StatusCode::OK) .insert_header((http::header::CONTENT_TYPE, broken_header)) .body(STR), @@ -438,16 +439,19 @@ async fn test_h2_response_http_error_handling() { // read response let bytes = srv.load_body(response).await.unwrap(); - assert_eq!(bytes, Bytes::from_static(b"failed to parse header value")); + assert_eq!( + bytes, + Bytes::from_static(b"error processing HTTP: failed to parse header value") + ); } #[derive(Debug, Display, Error)] #[display(fmt = "error")] struct BadRequest; -impl ResponseError for BadRequest { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST +impl From for Response { + fn from(_: BadRequest) -> Self { + Response::bad_request().set_body(AnyBody::from("error")) } } diff --git a/actix-http/tests/test_server.rs b/actix-http/tests/test_server.rs index abfda249c..1e6d0b637 100644 --- a/actix-http/tests/test_server.rs +++ b/actix-http/tests/test_server.rs @@ -1,21 +1,25 @@ -use std::io::{Read, Write}; -use std::time::Duration; -use std::{net, thread}; +use std::{ + convert::Infallible, + io::{Read, Write}, + net, thread, + time::Duration, +}; use actix_http::{ - body::{Body, SizedStream}, - http::{self, header, StatusCode}, - Error, HttpService, KeepAlive, Request, Response, + body::{AnyBody, Body, SizedStream}, + header, http, Error, HttpMessage, HttpService, KeepAlive, Request, Response, + StatusCode, }; -use actix_http::{HttpMessage, ResponseError}; use actix_http_test::test_server; use actix_rt::time::sleep; use actix_service::fn_service; use actix_utils::future::{err, ok, ready}; use bytes::Bytes; use derive_more::{Display, Error}; -use futures_util::stream::{once, StreamExt as _}; -use futures_util::FutureExt as _; +use futures_util::{ + stream::{once, StreamExt as _}, + FutureExt as _, +}; use regex::Regex; #[actix_rt::test] @@ -27,7 +31,7 @@ async fn test_h1() { .client_disconnect(1000) .h1(|req: Request| { assert!(req.peer_addr().is_some()); - ok::<_, ()>(Response::ok()) + ok::<_, Infallible>(Response::ok()) }) .tcp() }) @@ -47,7 +51,7 @@ async fn test_h1_2() { .finish(|req: Request| { assert!(req.peer_addr().is_some()); assert_eq!(req.version(), http::Version::HTTP_11); - ok::<_, ()>(Response::ok()) + ok::<_, Infallible>(Response::ok()) }) .tcp() }) @@ -61,9 +65,9 @@ async fn test_h1_2() { #[display(fmt = "expect failed")] struct ExpectFailed; -impl ResponseError for ExpectFailed { - fn status_code(&self) -> StatusCode { - StatusCode::PRECONDITION_FAILED +impl From for Response { + fn from(_: ExpectFailed) -> Self { + Response::new(StatusCode::EXPECTATION_FAILED) } } @@ -78,7 +82,7 @@ async fn test_expect_continue() { err(ExpectFailed) } })) - .finish(|_| ok::<_, ()>(Response::ok())) + .finish(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -87,7 +91,7 @@ async fn test_expect_continue() { let _ = stream.write_all(b"GET /test HTTP/1.1\r\nexpect: 100-continue\r\n\r\n"); let mut data = String::new(); let _ = stream.read_to_string(&mut data); - assert!(data.starts_with("HTTP/1.1 412 Precondition Failed\r\ncontent-length")); + assert!(data.starts_with("HTTP/1.1 417 Expectation Failed\r\ncontent-length")); let mut stream = net::TcpStream::connect(srv.addr()).unwrap(); let _ = stream.write_all(b"GET /test?yes= HTTP/1.1\r\nexpect: 100-continue\r\n\r\n"); @@ -109,7 +113,7 @@ async fn test_expect_continue_h1() { } }) })) - .h1(fn_service(|_| ok::<_, ()>(Response::ok()))) + .h1(fn_service(|_| ok::<_, Infallible>(Response::ok()))) .tcp() }) .await; @@ -118,7 +122,7 @@ async fn test_expect_continue_h1() { let _ = stream.write_all(b"GET /test HTTP/1.1\r\nexpect: 100-continue\r\n\r\n"); let mut data = String::new(); let _ = stream.read_to_string(&mut data); - assert!(data.starts_with("HTTP/1.1 412 Precondition Failed\r\ncontent-length")); + assert!(data.starts_with("HTTP/1.1 417 Expectation Failed\r\ncontent-length")); let mut stream = net::TcpStream::connect(srv.addr()).unwrap(); let _ = stream.write_all(b"GET /test?yes= HTTP/1.1\r\nexpect: 100-continue\r\n\r\n"); @@ -190,7 +194,7 @@ async fn test_slow_request() { let srv = test_server(|| { HttpService::build() .client_timeout(100) - .finish(|_| ok::<_, ()>(Response::ok())) + .finish(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -206,7 +210,7 @@ async fn test_slow_request() { async fn test_http1_malformed_request() { let srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok())) + .h1(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -222,7 +226,7 @@ async fn test_http1_malformed_request() { async fn test_http1_keepalive() { let srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok())) + .h1(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -244,7 +248,7 @@ async fn test_http1_keepalive_timeout() { let srv = test_server(|| { HttpService::build() .keep_alive(1) - .h1(|_| ok::<_, ()>(Response::ok())) + .h1(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -265,7 +269,7 @@ async fn test_http1_keepalive_timeout() { async fn test_http1_keepalive_close() { let srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok())) + .h1(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -286,7 +290,7 @@ async fn test_http1_keepalive_close() { async fn test_http10_keepalive_default_close() { let srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok())) + .h1(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -306,7 +310,7 @@ async fn test_http10_keepalive_default_close() { async fn test_http10_keepalive() { let srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok())) + .h1(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -334,7 +338,7 @@ async fn test_http1_keepalive_disabled() { let srv = test_server(|| { HttpService::build() .keep_alive(KeepAlive::Disabled) - .h1(|_| ok::<_, ()>(Response::ok())) + .h1(|_| ok::<_, Infallible>(Response::ok())) .tcp() }) .await; @@ -369,7 +373,7 @@ async fn test_content_length() { StatusCode::OK, StatusCode::NOT_FOUND, ]; - ok::<_, ()>(Response::new(statuses[indx])) + ok::<_, Infallible>(Response::new(statuses[indx])) }) .tcp() }) @@ -424,7 +428,7 @@ async fn test_h1_headers() { TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST ", )); } - ok::<_, ()>(builder.body(data.clone())) + ok::<_, Infallible>(builder.body(data.clone())) }).tcp() }).await; @@ -462,7 +466,7 @@ const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ async fn test_h1_body() { let mut srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h1(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .tcp() }) .await; @@ -479,7 +483,7 @@ async fn test_h1_body() { async fn test_h1_head_empty() { let mut srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h1(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .tcp() }) .await; @@ -504,7 +508,7 @@ async fn test_h1_head_empty() { async fn test_h1_head_binary() { let mut srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h1(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .tcp() }) .await; @@ -529,7 +533,7 @@ async fn test_h1_head_binary() { async fn test_h1_head_binary2() { let srv = test_server(|| { HttpService::build() - .h1(|_| ok::<_, ()>(Response::ok().set_body(STR))) + .h1(|_| ok::<_, Infallible>(Response::ok().set_body(STR))) .tcp() }) .await; @@ -551,8 +555,8 @@ async fn test_h1_body_length() { let mut srv = test_server(|| { HttpService::build() .h1(|_| { - let body = once(ok(Bytes::from_static(STR.as_ref()))); - ok::<_, ()>( + let body = once(ok::<_, Infallible>(Bytes::from_static(STR.as_ref()))); + ok::<_, Infallible>( Response::ok().set_body(SizedStream::new(STR.len() as u64, body)), ) }) @@ -574,7 +578,7 @@ async fn test_h1_body_chunked_explicit() { HttpService::build() .h1(|_| { let body = once(ok::<_, Error>(Bytes::from_static(STR.as_ref()))); - ok::<_, ()>( + ok::<_, Infallible>( Response::build(StatusCode::OK) .insert_header((header::TRANSFER_ENCODING, "chunked")) .streaming(body), @@ -609,7 +613,7 @@ async fn test_h1_body_chunked_implicit() { HttpService::build() .h1(|_| { let body = once(ok::<_, Error>(Bytes::from_static(STR.as_ref()))); - ok::<_, ()>(Response::build(StatusCode::OK).streaming(body)) + ok::<_, Infallible>(Response::build(StatusCode::OK).streaming(body)) }) .tcp() }) @@ -638,7 +642,7 @@ async fn test_h1_response_http_error_handling() { HttpService::build() .h1(fn_service(|_| { let broken_header = Bytes::from_static(b"\0\0\0"); - ok::<_, ()>( + ok::<_, Infallible>( Response::build(StatusCode::OK) .insert_header((http::header::CONTENT_TYPE, broken_header)) .body(STR), @@ -653,16 +657,19 @@ async fn test_h1_response_http_error_handling() { // read response let bytes = srv.load_body(response).await.unwrap(); - assert_eq!(bytes, Bytes::from_static(b"failed to parse header value")); + assert_eq!( + bytes, + Bytes::from_static(b"error processing HTTP: failed to parse header value") + ); } #[derive(Debug, Display, Error)] #[display(fmt = "error")] struct BadRequest; -impl ResponseError for BadRequest { - fn status_code(&self) -> StatusCode { - StatusCode::BAD_REQUEST +impl From for Response { + fn from(_: BadRequest) -> Self { + Response::bad_request().set_body(AnyBody::from("error")) } } @@ -692,7 +699,7 @@ async fn test_h1_on_connect() { }) .h1(|req: Request| { assert!(req.extensions().contains::()); - ok::<_, ()>(Response::ok()) + ok::<_, Infallible>(Response::ok()) }) .tcp() }) diff --git a/actix-http/tests/test_ws.rs b/actix-http/tests/test_ws.rs index b17d4211f..6d0de2316 100644 --- a/actix-http/tests/test_ws.rs +++ b/actix-http/tests/test_ws.rs @@ -1,11 +1,12 @@ use std::{ cell::Cell, + convert::Infallible, task::{Context, Poll}, }; use actix_codec::{AsyncRead, AsyncWrite, Framed}; use actix_http::{ - body::BodySize, + body::{AnyBody, BodySize}, h1, ws::{self, CloseCode, Frame, Item, Message}, Error, HttpService, Request, Response, @@ -13,6 +14,7 @@ use actix_http::{ use actix_http_test::test_server; use actix_service::{fn_factory, Service}; use bytes::Bytes; +use derive_more::{Display, Error, From}; use futures_core::future::LocalBoxFuture; use futures_util::{SinkExt as _, StreamExt as _}; @@ -33,12 +35,39 @@ impl WsService { } } +#[derive(Debug, Display, Error, From)] +enum WsServiceError { + #[display(fmt = "http error")] + Http(actix_http::Error), + + #[display(fmt = "ws handshake error")] + Ws(actix_http::ws::HandshakeError), + + #[display(fmt = "io error")] + Io(std::io::Error), + + #[display(fmt = "dispatcher error")] + Dispatcher, +} + +impl From for Response { + fn from(err: WsServiceError) -> Self { + match err { + WsServiceError::Http(err) => err.into(), + WsServiceError::Ws(err) => err.into(), + WsServiceError::Io(_err) => unreachable!(), + WsServiceError::Dispatcher => Response::internal_server_error() + .set_body(AnyBody::from(format!("{}", err))), + } + } +} + impl Service<(Request, Framed)> for WsService where T: AsyncRead + AsyncWrite + Unpin + 'static, { type Response = (); - type Error = Error; + type Error = WsServiceError; type Future = LocalBoxFuture<'static, Result>; fn poll_ready(&self, _: &mut Context<'_>) -> Poll> { @@ -56,7 +85,9 @@ where let framed = framed.replace_codec(ws::Codec::new()); - ws::Dispatcher::with(framed, service).await?; + ws::Dispatcher::with(framed, service) + .await + .map_err(|_| WsServiceError::Dispatcher)?; Ok(()) }) @@ -72,7 +103,7 @@ async fn service(msg: Frame) -> Result { Frame::Binary(bin) => Message::Binary(bin), Frame::Continuation(item) => Message::Continuation(item), Frame::Close(reason) => Message::Close(reason), - _ => return Err(Error::from(ws::ProtocolError::BadOpCode)), + _ => return Err(ws::ProtocolError::BadOpCode.into()), }; Ok(msg) @@ -82,8 +113,10 @@ async fn service(msg: Frame) -> Result { async fn test_simple() { let mut srv = test_server(|| { HttpService::build() - .upgrade(fn_factory(|| async { Ok::<_, ()>(WsService::new()) })) - .finish(|_| async { Ok::<_, ()>(Response::not_found()) }) + .upgrade(fn_factory(|| async { + Ok::<_, Infallible>(WsService::new()) + })) + .finish(|_| async { Ok::<_, Infallible>(Response::not_found()) }) .tcp() }) .await; diff --git a/actix-test/src/lib.rs b/actix-test/src/lib.rs index 5d85c2687..c863af44a 100644 --- a/actix-test/src/lib.rs +++ b/actix-test/src/lib.rs @@ -31,7 +31,7 @@ extern crate tls_openssl as openssl; #[cfg(feature = "rustls")] extern crate tls_rustls as rustls; -use std::{fmt, net, sync::mpsc, thread, time}; +use std::{error::Error as StdError, fmt, net, sync::mpsc, thread, time}; use actix_codec::{AsyncRead, AsyncWrite, Framed}; pub use actix_http::test::TestBuffer; @@ -39,7 +39,7 @@ use actix_http::{ http::{HeaderMap, Method}, ws, HttpService, Request, Response, }; -use actix_service::{map_config, IntoServiceFactory, ServiceFactory}; +use actix_service::{map_config, IntoServiceFactory, ServiceFactory, ServiceFactoryExt as _}; use actix_web::{ dev::{AppConfig, MessageBody, Server, Service}, rt, web, Error, @@ -86,7 +86,7 @@ where S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { start_with(TestServerConfig::default(), factory) } @@ -126,7 +126,7 @@ where S::Response: Into> + 'static, >::Future: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { let (tx, rx) = mpsc::channel(); @@ -153,25 +153,40 @@ where HttpVer::Http1 => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .h1(map_config(factory(), move |_| app_cfg.clone())) + .h1(map_config(fac, move |_| app_cfg.clone())) .tcp() }), HttpVer::Http2 => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .h2(map_config(factory(), move |_| app_cfg.clone())) + .h2(map_config(fac, move |_| app_cfg.clone())) .tcp() }), HttpVer::Both => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .finish(map_config(factory(), move |_| app_cfg.clone())) + .finish(map_config(fac, move |_| app_cfg.clone())) .tcp() }), }, @@ -180,25 +195,40 @@ where HttpVer::Http1 => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .h1(map_config(factory(), move |_| app_cfg.clone())) + .h1(map_config(fac, move |_| app_cfg.clone())) .openssl(acceptor.clone()) }), HttpVer::Http2 => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .h2(map_config(factory(), move |_| app_cfg.clone())) + .h2(map_config(fac, move |_| app_cfg.clone())) .openssl(acceptor.clone()) }), HttpVer::Both => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .finish(map_config(factory(), move |_| app_cfg.clone())) + .finish(map_config(fac, move |_| app_cfg.clone())) .openssl(acceptor.clone()) }), }, @@ -207,25 +237,40 @@ where HttpVer::Http1 => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .h1(map_config(factory(), move |_| app_cfg.clone())) + .h1(map_config(fac, move |_| app_cfg.clone())) .rustls(config.clone()) }), HttpVer::Http2 => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .h2(map_config(factory(), move |_| app_cfg.clone())) + .h2(map_config(fac, move |_| app_cfg.clone())) .rustls(config.clone()) }), HttpVer::Both => builder.listen("test", tcp, move || { let app_cfg = AppConfig::__priv_test_new(false, local_addr.to_string(), local_addr); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + HttpService::build() .client_timeout(timeout) - .finish(map_config(factory(), move |_| app_cfg.clone())) + .finish(map_config(fac, move |_| app_cfg.clone())) .rustls(config.clone()) }), }, diff --git a/actix-web-actors/src/ws.rs b/actix-web-actors/src/ws.rs index 8c575206d..f0a53d4e0 100644 --- a/actix-web-actors/src/ws.rs +++ b/actix-web-actors/src/ws.rs @@ -22,10 +22,11 @@ use actix_http::{ http::HeaderValue, ws::{hash_key, Codec}, }; -use actix_web::error::{Error, PayloadError}; -use actix_web::http::{header, Method, StatusCode}; -use actix_web::HttpResponseBuilder; -use actix_web::{HttpRequest, HttpResponse}; +use actix_web::{ + error::{Error, PayloadError}, + http::{header, Method, StatusCode}, + HttpRequest, HttpResponse, HttpResponseBuilder, +}; use bytes::{Bytes, BytesMut}; use bytestring::ByteString; use futures_core::Stream; diff --git a/awc/examples/client.rs b/awc/examples/client.rs index b9574590d..234ee3ae4 100644 --- a/awc/examples/client.rs +++ b/awc/examples/client.rs @@ -1,7 +1,7 @@ -use actix_http::Error; +use std::error::Error as StdError; #[actix_web::main] -async fn main() -> Result<(), Error> { +async fn main() -> Result<(), Box> { std::env::set_var("RUST_LOG", "actix_http=trace"); env_logger::init(); diff --git a/awc/src/error.rs b/awc/src/error.rs index b715f6213..c83c5ebbf 100644 --- a/awc/src/error.rs +++ b/awc/src/error.rs @@ -6,7 +6,6 @@ pub use actix_http::http::Error as HttpError; pub use actix_http::ws::HandshakeError as WsHandshakeError; pub use actix_http::ws::ProtocolError as WsProtocolError; -use actix_http::ResponseError; use serde_json::error::Error as JsonError; use actix_http::http::{header::HeaderValue, StatusCode}; @@ -77,6 +76,3 @@ pub enum JsonPayloadError { } impl std::error::Error for JsonPayloadError {} - -/// Return `InternalServerError` for `JsonPayloadError` -impl ResponseError for JsonPayloadError {} diff --git a/awc/src/frozen.rs b/awc/src/frozen.rs index 5fe8edb19..cb8c0f1bf 100644 --- a/awc/src/frozen.rs +++ b/awc/src/frozen.rs @@ -1,21 +1,21 @@ -use std::convert::TryFrom; -use std::net; -use std::rc::Rc; -use std::time::Duration; +use std::{convert::TryFrom, error::Error as StdError, net, rc::Rc, time::Duration}; use bytes::Bytes; use futures_core::Stream; use serde::Serialize; -use actix_http::body::Body; -use actix_http::http::header::IntoHeaderValue; -use actix_http::http::{Error as HttpError, HeaderMap, HeaderName, Method, Uri}; -use actix_http::{Error, RequestHead}; +use actix_http::{ + body::Body, + http::{header::IntoHeaderValue, Error as HttpError, HeaderMap, HeaderName, Method, Uri}, + RequestHead, +}; -use crate::sender::{RequestSender, SendClientRequest}; -use crate::ClientConfig; +use crate::{ + sender::{RequestSender, SendClientRequest}, + ClientConfig, +}; -/// `FrozenClientRequest` struct represents clonable client request. +/// `FrozenClientRequest` struct represents cloneable client request. /// It could be used to send same request multiple times. #[derive(Clone)] pub struct FrozenClientRequest { @@ -82,7 +82,7 @@ impl FrozenClientRequest { pub fn send_stream(&self, stream: S) -> SendClientRequest where S: Stream> + Unpin + 'static, - E: Into + 'static, + E: Into> + 'static, { RequestSender::Rc(self.head.clone(), None).send_stream( self.addr, @@ -207,7 +207,7 @@ impl FrozenSendBuilder { pub fn send_stream(self, stream: S) -> SendClientRequest where S: Stream> + Unpin + 'static, - E: Into + 'static, + E: Into> + 'static, { if let Some(e) = self.err { return e.into(); diff --git a/awc/src/lib.rs b/awc/src/lib.rs index 562d6ee7f..122f3845c 100644 --- a/awc/src/lib.rs +++ b/awc/src/lib.rs @@ -128,8 +128,7 @@ pub use self::sender::SendClientRequest; /// An asynchronous HTTP and WebSocket client. /// -/// ## Examples -/// +/// # Examples /// ``` /// use awc::Client; /// diff --git a/awc/src/request.rs b/awc/src/request.rs index 483524102..c95cee839 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -1,25 +1,26 @@ -use std::convert::TryFrom; -use std::rc::Rc; -use std::time::Duration; -use std::{fmt, net}; +use std::{convert::TryFrom, error::Error as StdError, fmt, net, rc::Rc, time::Duration}; use bytes::Bytes; use futures_core::Stream; use serde::Serialize; -use actix_http::body::Body; -use actix_http::http::header::{self, IntoHeaderPair}; -use actix_http::http::{ - uri, ConnectionType, Error as HttpError, HeaderMap, HeaderValue, Method, Uri, Version, +use actix_http::{ + body::Body, + http::{ + header::{self, IntoHeaderPair}, + uri, ConnectionType, Error as HttpError, HeaderMap, HeaderValue, Method, Uri, Version, + }, + RequestHead, }; -use actix_http::{Error, RequestHead}; #[cfg(feature = "cookies")] use crate::cookie::{Cookie, CookieJar}; -use crate::error::{FreezeRequestError, InvalidUrl}; -use crate::frozen::FrozenClientRequest; -use crate::sender::{PrepForSendingError, RequestSender, SendClientRequest}; -use crate::ClientConfig; +use crate::{ + error::{FreezeRequestError, InvalidUrl}, + frozen::FrozenClientRequest, + sender::{PrepForSendingError, RequestSender, SendClientRequest}, + ClientConfig, +}; #[cfg(feature = "compress")] const HTTPS_ENCODING: &str = "br, gzip, deflate"; @@ -408,7 +409,7 @@ impl ClientRequest { pub fn send_stream(self, stream: S) -> SendClientRequest where S: Stream> + Unpin + 'static, - E: Into + 'static, + E: Into> + 'static, { let slf = match self.prep_for_sending() { Ok(slf) => slf, diff --git a/awc/src/sender.rs b/awc/src/sender.rs index 0e63be221..7ac9c8ce9 100644 --- a/awc/src/sender.rs +++ b/awc/src/sender.rs @@ -1,6 +1,7 @@ use std::{ + error::Error as StdError, future::Future, - io, net, + net, pin::Pin, rc::Rc, task::{Context, Poll}, @@ -24,22 +25,30 @@ use serde::Serialize; #[cfg(feature = "compress")] use actix_http::{encoding::Decoder, http::header::ContentEncoding, Payload, PayloadStream}; -use crate::connect::{ConnectRequest, ConnectResponse}; -use crate::error::{FreezeRequestError, InvalidUrl, SendRequestError}; -use crate::response::ClientResponse; -use crate::ClientConfig; +use crate::{ + error::{FreezeRequestError, InvalidUrl, SendRequestError}, + ClientConfig, ClientResponse, ConnectRequest, ConnectResponse, +}; #[derive(Debug, From)] pub(crate) enum PrepForSendingError { Url(InvalidUrl), Http(HttpError), + Json(serde_json::Error), + Form(serde_urlencoded::ser::Error), } impl From for FreezeRequestError { fn from(err: PrepForSendingError) -> FreezeRequestError { match err { - PrepForSendingError::Url(e) => FreezeRequestError::Url(e), - PrepForSendingError::Http(e) => FreezeRequestError::Http(e), + PrepForSendingError::Url(err) => FreezeRequestError::Url(err), + PrepForSendingError::Http(err) => FreezeRequestError::Http(err), + PrepForSendingError::Json(err) => { + FreezeRequestError::Custom(Box::new(err), Box::new("json serialization error")) + } + PrepForSendingError::Form(err) => { + FreezeRequestError::Custom(Box::new(err), Box::new("form serialization error")) + } } } } @@ -49,6 +58,12 @@ impl From for SendRequestError { match err { PrepForSendingError::Url(e) => SendRequestError::Url(e), PrepForSendingError::Http(e) => SendRequestError::Http(e), + PrepForSendingError::Json(err) => { + SendRequestError::Custom(Box::new(err), Box::new("json serialization error")) + } + PrepForSendingError::Form(err) => { + SendRequestError::Custom(Box::new(err), Box::new("form serialization error")) + } } } } @@ -209,8 +224,7 @@ impl RequestSender { ) -> SendClientRequest { let body = match serde_json::to_string(value) { Ok(body) => body, - // TODO: own error type - Err(e) => return Error::from(io::Error::new(io::ErrorKind::Other, e)).into(), + Err(err) => return PrepForSendingError::Json(err).into(), }; if let Err(e) = self.set_header_if_none(header::CONTENT_TYPE, "application/json") { @@ -236,8 +250,7 @@ impl RequestSender { ) -> SendClientRequest { let body = match serde_urlencoded::to_string(value) { Ok(body) => body, - // TODO: own error type - Err(e) => return Error::from(io::Error::new(io::ErrorKind::Other, e)).into(), + Err(err) => return PrepForSendingError::Form(err).into(), }; // set content-type @@ -266,7 +279,7 @@ impl RequestSender { ) -> SendClientRequest where S: Stream> + Unpin + 'static, - E: Into + 'static, + E: Into> + 'static, { self.send_body( addr, diff --git a/src/data.rs b/src/data.rs index d63c15580..f09a88891 100644 --- a/src/data.rs +++ b/src/data.rs @@ -1,12 +1,13 @@ use std::{any::type_name, ops::Deref, sync::Arc}; -use actix_http::{error::Error, Extensions}; +use actix_http::Extensions; use actix_utils::future::{err, ok, Ready}; use futures_core::future::LocalBoxFuture; use serde::Serialize; use crate::{ dev::Payload, error::ErrorInternalServerError, extract::FromRequest, request::HttpRequest, + Error, }; /// Data factory. diff --git a/src/error/error.rs b/src/error/error.rs new file mode 100644 index 000000000..add290867 --- /dev/null +++ b/src/error/error.rs @@ -0,0 +1,76 @@ +use std::{error::Error as StdError, fmt}; + +use actix_http::{body::AnyBody, Response}; + +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. 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 used to create an HTTP response from it this means that +/// if you have access to an actix `Error` you can always get a +/// `ResponseError` reference from it. +pub struct Error { + cause: Box, +} + +impl Error { + /// Returns the reference to the underlying `ResponseError`. + pub fn as_response_error(&self) -> &dyn ResponseError { + self.cause.as_ref() + } + + /// Similar to `as_response_error` but downcasts. + pub fn as_error(&self) -> Option<&T> { + ::downcast_ref(self.cause.as_ref()) + } + + /// Shortcut for creating an `HttpResponse`. + pub fn error_response(&self) -> HttpResponse { + self.cause.error_response() + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.cause, f) + } +} + +impl fmt::Debug for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", &self.cause) + } +} + +impl StdError for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + // TODO: populate if replacement for Box is found + None + } +} + +impl From for Error { + fn from(val: std::convert::Infallible) -> Self { + match val {} + } +} + +/// `Error` for any error that implements `ResponseError` +impl From for Error { + fn from(err: T) -> Error { + Error { + cause: Box::new(err), + } + } +} + +impl From for Response { + fn from(err: Error) -> Response { + err.error_response().into() + } +} diff --git a/src/error/internal.rs b/src/error/internal.rs index 23b7dc31e..1d9ca904e 100644 --- a/src/error/internal.rs +++ b/src/error/internal.rs @@ -1,9 +1,9 @@ use std::{cell::RefCell, fmt, io::Write as _}; -use actix_http::{body::Body, header, Response, StatusCode}; +use actix_http::{body::Body, header, StatusCode}; use bytes::{BufMut as _, BytesMut}; -use crate::{Error, HttpResponse, ResponseError}; +use crate::{Error, HttpRequest, HttpResponse, Responder, ResponseError}; /// Wraps errors to alter the generated response status code. /// @@ -77,10 +77,10 @@ where } } - fn error_response(&self) -> Response { + fn error_response(&self) -> HttpResponse { match self.status { InternalErrorType::Status(status) => { - let mut res = Response::new(status); + let mut res = HttpResponse::new(status); let mut buf = BytesMut::new().writer(); let _ = write!(buf, "{}", self); @@ -88,20 +88,29 @@ where header::CONTENT_TYPE, header::HeaderValue::from_static("text/plain; charset=utf-8"), ); - res.set_body(Body::from(buf.into_inner())).into() + res.set_body(Body::from(buf.into_inner())) } InternalErrorType::Response(ref resp) => { if let Some(resp) = resp.borrow_mut().take() { - resp.into() + resp } else { - Response::new(StatusCode::INTERNAL_SERVER_ERROR) + HttpResponse::new(StatusCode::INTERNAL_SERVER_ERROR) } } } } } +impl Responder for InternalError +where + T: fmt::Debug + fmt::Display + 'static, +{ + fn respond_to(self, _: &HttpRequest) -> HttpResponse { + HttpResponse::from_error(self) + } +} + macro_rules! error_helper { ($name:ident, $status:ident) => { paste::paste! { @@ -171,134 +180,134 @@ error_helper!( #[cfg(test)] mod tests { - use actix_http::{error::ParseError, Response}; + use actix_http::error::ParseError; use super::*; #[test] fn test_internal_error() { let err = InternalError::from_response(ParseError::Method, HttpResponse::Ok().finish()); - let resp: Response = err.error_response(); + let resp: HttpResponse = err.error_response(); assert_eq!(resp.status(), StatusCode::OK); } #[test] fn test_error_helpers() { - let res: Response = ErrorBadRequest("err").into(); + let res: HttpResponse = ErrorBadRequest("err").into(); assert_eq!(res.status(), StatusCode::BAD_REQUEST); - let res: Response = ErrorUnauthorized("err").into(); + let res: HttpResponse = ErrorUnauthorized("err").into(); assert_eq!(res.status(), StatusCode::UNAUTHORIZED); - let res: Response = ErrorPaymentRequired("err").into(); + let res: HttpResponse = ErrorPaymentRequired("err").into(); assert_eq!(res.status(), StatusCode::PAYMENT_REQUIRED); - let res: Response = ErrorForbidden("err").into(); + let res: HttpResponse = ErrorForbidden("err").into(); assert_eq!(res.status(), StatusCode::FORBIDDEN); - let res: Response = ErrorNotFound("err").into(); + let res: HttpResponse = ErrorNotFound("err").into(); assert_eq!(res.status(), StatusCode::NOT_FOUND); - let res: Response = ErrorMethodNotAllowed("err").into(); + let res: HttpResponse = ErrorMethodNotAllowed("err").into(); assert_eq!(res.status(), StatusCode::METHOD_NOT_ALLOWED); - let res: Response = ErrorNotAcceptable("err").into(); + let res: HttpResponse = ErrorNotAcceptable("err").into(); assert_eq!(res.status(), StatusCode::NOT_ACCEPTABLE); - let res: Response = ErrorProxyAuthenticationRequired("err").into(); + let res: HttpResponse = ErrorProxyAuthenticationRequired("err").into(); assert_eq!(res.status(), StatusCode::PROXY_AUTHENTICATION_REQUIRED); - let res: Response = ErrorRequestTimeout("err").into(); + let res: HttpResponse = ErrorRequestTimeout("err").into(); assert_eq!(res.status(), StatusCode::REQUEST_TIMEOUT); - let res: Response = ErrorConflict("err").into(); + let res: HttpResponse = ErrorConflict("err").into(); assert_eq!(res.status(), StatusCode::CONFLICT); - let res: Response = ErrorGone("err").into(); + let res: HttpResponse = ErrorGone("err").into(); assert_eq!(res.status(), StatusCode::GONE); - let res: Response = ErrorLengthRequired("err").into(); + let res: HttpResponse = ErrorLengthRequired("err").into(); assert_eq!(res.status(), StatusCode::LENGTH_REQUIRED); - let res: Response = ErrorPreconditionFailed("err").into(); + let res: HttpResponse = ErrorPreconditionFailed("err").into(); assert_eq!(res.status(), StatusCode::PRECONDITION_FAILED); - let res: Response = ErrorPayloadTooLarge("err").into(); + let res: HttpResponse = ErrorPayloadTooLarge("err").into(); assert_eq!(res.status(), StatusCode::PAYLOAD_TOO_LARGE); - let res: Response = ErrorUriTooLong("err").into(); + let res: HttpResponse = ErrorUriTooLong("err").into(); assert_eq!(res.status(), StatusCode::URI_TOO_LONG); - let res: Response = ErrorUnsupportedMediaType("err").into(); + let res: HttpResponse = ErrorUnsupportedMediaType("err").into(); assert_eq!(res.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); - let res: Response = ErrorRangeNotSatisfiable("err").into(); + let res: HttpResponse = ErrorRangeNotSatisfiable("err").into(); assert_eq!(res.status(), StatusCode::RANGE_NOT_SATISFIABLE); - let res: Response = ErrorExpectationFailed("err").into(); + let res: HttpResponse = ErrorExpectationFailed("err").into(); assert_eq!(res.status(), StatusCode::EXPECTATION_FAILED); - let res: Response = ErrorImATeapot("err").into(); + let res: HttpResponse = ErrorImATeapot("err").into(); assert_eq!(res.status(), StatusCode::IM_A_TEAPOT); - let res: Response = ErrorMisdirectedRequest("err").into(); + let res: HttpResponse = ErrorMisdirectedRequest("err").into(); assert_eq!(res.status(), StatusCode::MISDIRECTED_REQUEST); - let res: Response = ErrorUnprocessableEntity("err").into(); + let res: HttpResponse = ErrorUnprocessableEntity("err").into(); assert_eq!(res.status(), StatusCode::UNPROCESSABLE_ENTITY); - let res: Response = ErrorLocked("err").into(); + let res: HttpResponse = ErrorLocked("err").into(); assert_eq!(res.status(), StatusCode::LOCKED); - let res: Response = ErrorFailedDependency("err").into(); + let res: HttpResponse = ErrorFailedDependency("err").into(); assert_eq!(res.status(), StatusCode::FAILED_DEPENDENCY); - let res: Response = ErrorUpgradeRequired("err").into(); + let res: HttpResponse = ErrorUpgradeRequired("err").into(); assert_eq!(res.status(), StatusCode::UPGRADE_REQUIRED); - let res: Response = ErrorPreconditionRequired("err").into(); + let res: HttpResponse = ErrorPreconditionRequired("err").into(); assert_eq!(res.status(), StatusCode::PRECONDITION_REQUIRED); - let res: Response = ErrorTooManyRequests("err").into(); + let res: HttpResponse = ErrorTooManyRequests("err").into(); assert_eq!(res.status(), StatusCode::TOO_MANY_REQUESTS); - let res: Response = ErrorRequestHeaderFieldsTooLarge("err").into(); + let res: HttpResponse = ErrorRequestHeaderFieldsTooLarge("err").into(); assert_eq!(res.status(), StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE); - let res: Response = ErrorUnavailableForLegalReasons("err").into(); + let res: HttpResponse = ErrorUnavailableForLegalReasons("err").into(); assert_eq!(res.status(), StatusCode::UNAVAILABLE_FOR_LEGAL_REASONS); - let res: Response = ErrorInternalServerError("err").into(); + let res: HttpResponse = ErrorInternalServerError("err").into(); assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); - let res: Response = ErrorNotImplemented("err").into(); + let res: HttpResponse = ErrorNotImplemented("err").into(); assert_eq!(res.status(), StatusCode::NOT_IMPLEMENTED); - let res: Response = ErrorBadGateway("err").into(); + let res: HttpResponse = ErrorBadGateway("err").into(); assert_eq!(res.status(), StatusCode::BAD_GATEWAY); - let res: Response = ErrorServiceUnavailable("err").into(); + let res: HttpResponse = ErrorServiceUnavailable("err").into(); assert_eq!(res.status(), StatusCode::SERVICE_UNAVAILABLE); - let res: Response = ErrorGatewayTimeout("err").into(); + let res: HttpResponse = ErrorGatewayTimeout("err").into(); assert_eq!(res.status(), StatusCode::GATEWAY_TIMEOUT); - let res: Response = ErrorHttpVersionNotSupported("err").into(); + let res: HttpResponse = ErrorHttpVersionNotSupported("err").into(); assert_eq!(res.status(), StatusCode::HTTP_VERSION_NOT_SUPPORTED); - let res: Response = ErrorVariantAlsoNegotiates("err").into(); + let res: HttpResponse = ErrorVariantAlsoNegotiates("err").into(); assert_eq!(res.status(), StatusCode::VARIANT_ALSO_NEGOTIATES); - let res: Response = ErrorInsufficientStorage("err").into(); + let res: HttpResponse = ErrorInsufficientStorage("err").into(); assert_eq!(res.status(), StatusCode::INSUFFICIENT_STORAGE); - let res: Response = ErrorLoopDetected("err").into(); + let res: HttpResponse = ErrorLoopDetected("err").into(); assert_eq!(res.status(), StatusCode::LOOP_DETECTED); - let res: Response = ErrorNotExtended("err").into(); + let res: HttpResponse = ErrorNotExtended("err").into(); assert_eq!(res.status(), StatusCode::NOT_EXTENDED); - let res: Response = ErrorNetworkAuthenticationRequired("err").into(); + let res: HttpResponse = ErrorNetworkAuthenticationRequired("err").into(); assert_eq!(res.status(), StatusCode::NETWORK_AUTHENTICATION_REQUIRED); } } diff --git a/src/error/macros.rs b/src/error/macros.rs new file mode 100644 index 000000000..aeab74308 --- /dev/null +++ b/src/error/macros.rs @@ -0,0 +1,109 @@ +#[macro_export] +#[doc(hidden)] +macro_rules! __downcast_get_type_id { + () => { + /// A helper method to get the type ID of the type + /// this trait is implemented on. + /// This method is unsafe to *implement*, since `downcast_ref` relies + /// on the returned `TypeId` to perform a cast. + /// + /// Unfortunately, Rust has no notion of a trait method that is + /// unsafe to implement (marking it as `unsafe` makes it unsafe + /// to *call*). As a workaround, we require this method + /// to return a private type along with the `TypeId`. This + /// private type (`PrivateHelper`) has a private constructor, + /// making it impossible for safe code to construct outside of + /// this module. This ensures that safe code cannot violate + /// type-safety by implementing this method. + /// + /// We also take `PrivateHelper` as a parameter, to ensure that + /// safe code cannot obtain a `PrivateHelper` instance by + /// delegating to an existing implementation of `__private_get_type_id__` + #[doc(hidden)] + #[allow(dead_code)] + fn __private_get_type_id__(&self, _: PrivateHelper) -> (std::any::TypeId, PrivateHelper) + where + Self: 'static, + { + (std::any::TypeId::of::(), PrivateHelper(())) + } + }; +} + +//Generate implementation for dyn $name +#[doc(hidden)] +#[macro_export] +macro_rules! __downcast_dyn { + ($name:ident) => { + /// A struct with a private constructor, for use with + /// `__private_get_type_id__`. Its single field is private, + /// ensuring that it can only be constructed from this module + #[doc(hidden)] + #[allow(dead_code)] + pub struct PrivateHelper(()); + + impl dyn $name + 'static { + /// Downcasts generic body to a specific type. + #[allow(dead_code)] + pub fn downcast_ref(&self) -> Option<&T> { + if self.__private_get_type_id__(PrivateHelper(())).0 + == std::any::TypeId::of::() + { + // SAFETY: external crates cannot override the default + // implementation of `__private_get_type_id__`, since + // it requires returning a private type. We can therefore + // rely on the returned `TypeId`, which ensures that this + // case is correct. + unsafe { Some(&*(self as *const dyn $name as *const T)) } + } else { + None + } + } + + /// Downcasts a generic body to a mutable specific type. + #[allow(dead_code)] + pub fn downcast_mut(&mut self) -> Option<&mut T> { + if self.__private_get_type_id__(PrivateHelper(())).0 + == std::any::TypeId::of::() + { + // SAFETY: external crates cannot override the default + // implementation of `__private_get_type_id__`, since + // it requires returning a private type. We can therefore + // rely on the returned `TypeId`, which ensures that this + // case is correct. + unsafe { Some(&mut *(self as *const dyn $name as *const T as *mut T)) } + } else { + None + } + } + } + }; +} + +#[cfg(test)] +mod tests { + #![allow(clippy::upper_case_acronyms)] + + trait MB { + __downcast_get_type_id!(); + } + + __downcast_dyn!(MB); + + impl MB for String {} + impl MB for () {} + + #[actix_rt::test] + async fn test_any_casting() { + let mut body = String::from("hello cast"); + let resp_body: &mut dyn MB = &mut body; + let body = resp_body.downcast_ref::().unwrap(); + assert_eq!(body, "hello cast"); + let body = &mut resp_body.downcast_mut::().unwrap(); + body.push('!'); + let body = resp_body.downcast_ref::().unwrap(); + assert_eq!(body, "hello cast!"); + let not_body = resp_body.downcast_ref::<()>(); + assert!(not_body.is_none()); + } +} diff --git a/src/error/mod.rs b/src/error/mod.rs index 146146c71..637d6ff16 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -9,14 +9,20 @@ use url::ParseError as UrlParseError; use crate::http::StatusCode; +#[allow(clippy::module_inception)] +mod error; mod internal; +mod macros; +mod response_error; +pub use self::error::Error; pub use self::internal::*; +pub use self::response_error::ResponseError; /// A convenience [`Result`](std::result::Result) for Actix Web operations. /// /// This type alias is generally used to avoid writing out `actix_http::Error` directly. -pub type Result = std::result::Result; +pub type Result = std::result::Result; /// Errors which can occur when attempting to generate resource uri. #[derive(Debug, PartialEq, Display, Error, From)] diff --git a/src/error/response_error.rs b/src/error/response_error.rs new file mode 100644 index 000000000..c58fff8be --- /dev/null +++ b/src/error/response_error.rs @@ -0,0 +1,144 @@ +//! `ResponseError` trait and foreign impls. + +use std::{ + error::Error as StdError, + fmt, + io::{self, Write as _}, +}; + +use actix_http::{body::AnyBody, header, Response, StatusCode}; +use bytes::BytesMut; + +use crate::{__downcast_dyn, __downcast_get_type_id}; +use crate::{helpers, HttpResponse}; + +/// Errors that can generate responses. +// TODO: add std::error::Error bound when replacement for Box is found +pub trait ResponseError: fmt::Debug + fmt::Display { + /// Returns appropriate status code for error. + /// + /// A 500 Internal Server Error is used by default. If [error_response](Self::error_response) is + /// also implemented and does not call `self.status_code()`, then this will not be used. + fn status_code(&self) -> StatusCode { + StatusCode::INTERNAL_SERVER_ERROR + } + + /// Creates full response for error. + /// + /// By default, the generated response uses a 500 Internal Server Error status code, a + /// `Content-Type` of `text/plain`, and the body is set to `Self`'s `Display` impl. + fn error_response(&self) -> HttpResponse { + let mut res = HttpResponse::new(self.status_code()); + + let mut buf = BytesMut::new(); + let _ = write!(helpers::MutWriter(&mut buf), "{}", self); + + res.headers_mut().insert( + header::CONTENT_TYPE, + header::HeaderValue::from_static("text/plain; charset=utf-8"), + ); + + res.set_body(AnyBody::from(buf)) + } + + __downcast_get_type_id!(); +} + +__downcast_dyn!(ResponseError); + +impl ResponseError for Box {} + +#[cfg(feature = "openssl")] +impl ResponseError for actix_tls::accept::openssl::SslError {} + +impl ResponseError for serde::de::value::Error { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + +impl ResponseError for std::str::Utf8Error { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + +impl ResponseError for std::io::Error { + fn status_code(&self) -> StatusCode { + // TODO: decide if these errors should consider not found or permission errors + match self.kind() { + io::ErrorKind::NotFound => StatusCode::NOT_FOUND, + io::ErrorKind::PermissionDenied => StatusCode::FORBIDDEN, + _ => StatusCode::INTERNAL_SERVER_ERROR, + } + } +} + +impl ResponseError for actix_http::error::HttpError {} + +impl ResponseError for actix_http::Error { + fn status_code(&self) -> StatusCode { + // TODO: map error kinds to status code better + StatusCode::INTERNAL_SERVER_ERROR + } + + fn error_response(&self) -> HttpResponse { + HttpResponse::new(self.status_code()).set_body(self.to_string().into()) + } +} + +impl ResponseError for actix_http::header::InvalidHeaderValue { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + +impl ResponseError for actix_http::error::ParseError { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + +impl ResponseError for actix_http::error::BlockingError {} + +impl ResponseError for actix_http::error::PayloadError { + fn status_code(&self) -> StatusCode { + match *self { + actix_http::error::PayloadError::Overflow => StatusCode::PAYLOAD_TOO_LARGE, + _ => StatusCode::BAD_REQUEST, + } + } +} + +impl ResponseError for actix_http::ws::ProtocolError {} + +impl ResponseError for actix_http::error::ContentTypeError { + fn status_code(&self) -> StatusCode { + StatusCode::BAD_REQUEST + } +} + +impl ResponseError for actix_http::ws::HandshakeError { + fn error_response(&self) -> HttpResponse { + Response::from(self).into() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_error_casting() { + use actix_http::error::{ContentTypeError, PayloadError}; + + let err = PayloadError::Overflow; + let resp_err: &dyn ResponseError = &err; + + let err = resp_err.downcast_ref::().unwrap(); + assert_eq!(err.to_string(), "Payload reached size limit."); + + let not_err = resp_err.downcast_ref::(); + assert!(not_err.is_none()); + } +} diff --git a/src/extract.rs b/src/extract.rs index 80f2384a0..45cb330a3 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -47,8 +47,7 @@ pub trait FromRequest: Sized { /// /// If the FromRequest for T fails, return None rather than returning an error response /// -/// ## Example -/// +/// # Examples /// ``` /// use actix_web::{web, dev, App, Error, HttpRequest, FromRequest}; /// use actix_web::error::ErrorBadRequest; @@ -139,8 +138,7 @@ where /// /// If the `FromRequest` for T fails, inject Err into handler rather than returning an error response /// -/// ## Example -/// +/// # Examples /// ``` /// use actix_web::{web, dev, App, Result, Error, HttpRequest, FromRequest}; /// use actix_web::error::ErrorBadRequest; diff --git a/src/handler.rs b/src/handler.rs index 822dcafdd..bc91ce41b 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -3,18 +3,14 @@ use std::marker::PhantomData; use std::pin::Pin; use std::task::{Context, Poll}; -use actix_http::Error; use actix_service::{Service, ServiceFactory}; use actix_utils::future::{ready, Ready}; use futures_core::ready; use pin_project::pin_project; use crate::{ - extract::FromRequest, - request::HttpRequest, - responder::Responder, - response::HttpResponse, service::{ServiceRequest, ServiceResponse}, + Error, FromRequest, HttpRequest, HttpResponse, Responder, }; /// A request handler is an async function that accepts zero or more parameters that can be diff --git a/src/helpers.rs b/src/helpers.rs new file mode 100644 index 000000000..1d2679fce --- /dev/null +++ b/src/helpers.rs @@ -0,0 +1,25 @@ +use std::io; + +use bytes::BufMut; + +/// An `io::Write`r that only requires mutable reference and assumes that there is space available +/// in the buffer for every write operation or that it can be extended implicitly (like +/// `bytes::BytesMut`, for example). +/// +/// This is slightly faster (~10%) than `bytes::buf::Writer` in such cases because it does not +/// perform a remaining length check before writing. +pub(crate) struct MutWriter<'a, B>(pub(crate) &'a mut B); + +impl<'a, B> io::Write for MutWriter<'a, B> +where + B: BufMut, +{ + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.put_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} diff --git a/src/lib.rs b/src/lib.rs index 4e8093a2a..b488b962b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,6 @@ //! Actix Web is a powerful, pragmatic, and extremely fast web framework for Rust. //! -//! ## Example -//! +//! # Examples //! ```no_run //! use actix_web::{get, web, App, HttpServer, Responder}; //! @@ -20,8 +19,7 @@ //! } //! ``` //! -//! ## Documentation & Community Resources -//! +//! # Documentation & Community Resources //! In addition to this API documentation, several other resources are available: //! //! * [Website & User Guide](https://actix.rs/) @@ -44,8 +42,7 @@ //! structs represent HTTP requests and responses and expose methods for creating, inspecting, //! and otherwise utilizing them. //! -//! ## Features -//! +//! # Features //! * Supports *HTTP/1.x* and *HTTP/2* //! * Streaming and pipelining //! * Keep-alive and slow requests handling @@ -59,8 +56,7 @@ //! * Includes an async [HTTP client](https://docs.rs/awc/) //! * Runs on stable Rust 1.46+ //! -//! ## Crate Features -//! +//! # Crate Features //! * `compress` - content encoding compression support (enabled by default) //! * `cookies` - cookies support (enabled by default) //! * `openssl` - HTTPS support via `openssl` crate, supports `HTTP/2` @@ -80,6 +76,7 @@ pub mod error; mod extract; pub mod guard; mod handler; +mod helpers; pub mod http; mod info; pub mod middleware; @@ -98,7 +95,7 @@ pub(crate) mod types; pub mod web; pub use actix_http::Response as BaseHttpResponse; -pub use actix_http::{body, Error, HttpMessage, ResponseError}; +pub use actix_http::{body, HttpMessage}; #[doc(inline)] pub use actix_rt as rt; pub use actix_web_codegen::*; @@ -106,7 +103,7 @@ pub use actix_web_codegen::*; pub use cookie; pub use crate::app::App; -pub use crate::error::Result; +pub use crate::error::{Error, ResponseError, Result}; pub use crate::extract::FromRequest; pub use crate::request::HttpRequest; pub use crate::resource::Resource; @@ -140,7 +137,9 @@ pub mod dev { pub use crate::types::json::JsonBody; pub use crate::types::readlines::Readlines; - pub use actix_http::body::{Body, BodySize, MessageBody, ResponseBody, SizedStream}; + pub use actix_http::body::{ + AnyBody, Body, BodySize, MessageBody, ResponseBody, SizedStream, + }; #[cfg(feature = "compress")] pub use actix_http::encoding::Decoder as Decompress; pub use actix_http::ResponseBuilder as BaseHttpResponseBuilder; diff --git a/src/middleware/compat.rs b/src/middleware/compat.rs index 4f2f2a504..95f5f4b52 100644 --- a/src/middleware/compat.rs +++ b/src/middleware/compat.rs @@ -50,7 +50,7 @@ where T: Transform, T::Future: 'static, T::Response: MapServiceResponseBody, - Error: From, + T::Error: Into, { type Response = ServiceResponse; type Error = Error; @@ -75,7 +75,7 @@ impl Service for CompatMiddleware where S: Service, S::Response: MapServiceResponseBody, - Error: From, + S::Error: Into, { type Response = ServiceResponse; type Error = Error; @@ -99,12 +99,16 @@ impl Future for CompatMiddlewareFuture where Fut: Future>, T: MapServiceResponseBody, - Error: From, + E: Into, { type Output = Result; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let res = ready!(self.project().fut.poll(cx))?; + let res = match ready!(self.project().fut.poll(cx)) { + Ok(res) => res, + Err(err) => return Poll::Ready(Err(err.into())), + }; + Poll::Ready(Ok(res.map_body())) } } diff --git a/src/middleware/compress.rs b/src/middleware/compress.rs index f8514c7cc..0eb4d0a83 100644 --- a/src/middleware/compress.rs +++ b/src/middleware/compress.rs @@ -13,7 +13,6 @@ use actix_http::{ body::{MessageBody, ResponseBody}, encoding::Encoder, http::header::{ContentEncoding, ACCEPT_ENCODING}, - Error, }; use actix_service::{Service, Transform}; use actix_utils::future::{ok, Ready}; @@ -23,6 +22,7 @@ use pin_project::pin_project; use crate::{ dev::BodyEncoding, service::{ServiceRequest, ServiceResponse}, + Error, }; /// Middleware for compressing response payloads. diff --git a/src/middleware/err_handlers.rs b/src/middleware/err_handlers.rs index 88834f8ce..75cc819bc 100644 --- a/src/middleware/err_handlers.rs +++ b/src/middleware/err_handlers.rs @@ -13,8 +13,8 @@ use futures_core::{future::LocalBoxFuture, ready}; use crate::{ dev::{ServiceRequest, ServiceResponse}, - error::{Error, Result}, http::StatusCode, + Error, Result, }; /// Return type for [`ErrorHandlers`] custom handlers. diff --git a/src/request.rs b/src/request.rs index e3da991de..42c722c46 100644 --- a/src/request.rs +++ b/src/request.rs @@ -7,7 +7,7 @@ use std::{ use actix_http::{ http::{HeaderMap, Method, Uri, Version}, - Error, Extensions, HttpMessage, Message, Payload, RequestHead, + Extensions, HttpMessage, Message, Payload, RequestHead, }; use actix_router::{Path, Url}; use actix_utils::future::{ok, Ready}; @@ -17,7 +17,7 @@ use smallvec::SmallVec; use crate::{ app_service::AppInitServiceState, config::AppConfig, error::UrlGenerationError, - extract::FromRequest, info::ConnectionInfo, rmap::ResourceMap, + info::ConnectionInfo, rmap::ResourceMap, Error, FromRequest, }; #[cfg(feature = "cookies")] @@ -356,8 +356,7 @@ impl Drop for HttpRequest { /// It is possible to get `HttpRequest` as an extractor handler parameter /// -/// ## Example -/// +/// # Examples /// ``` /// use actix_web::{web, App, HttpRequest}; /// use serde_derive::Deserialize; diff --git a/src/request_data.rs b/src/request_data.rs index 559d6ecbf..581943015 100644 --- a/src/request_data.rs +++ b/src/request_data.rs @@ -1,9 +1,8 @@ use std::{any::type_name, ops::Deref}; -use actix_http::error::Error; use actix_utils::future::{err, ok, Ready}; -use crate::{dev::Payload, error::ErrorInternalServerError, FromRequest, HttpRequest}; +use crate::{dev::Payload, error::ErrorInternalServerError, Error, FromRequest, HttpRequest}; /// Request-local data extractor. /// diff --git a/src/resource.rs b/src/resource.rs index 049e56291..8c2b83b60 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -3,7 +3,7 @@ use std::fmt; use std::future::Future; use std::rc::Rc; -use actix_http::{Error, Extensions}; +use actix_http::Extensions; use actix_router::IntoPattern; use actix_service::boxed::{self, BoxService, BoxServiceFactory}; use actix_service::{ @@ -13,14 +13,16 @@ use actix_service::{ use futures_core::future::LocalBoxFuture; use futures_util::future::join_all; -use crate::dev::{insert_slash, AppService, HttpServiceFactory, ResourceDef}; -use crate::extract::FromRequest; -use crate::guard::Guard; -use crate::handler::Handler; -use crate::responder::Responder; -use crate::route::{Route, RouteService}; -use crate::service::{ServiceRequest, ServiceResponse}; -use crate::{data::Data, HttpResponse}; +use crate::{ + data::Data, + dev::{insert_slash, AppService, HttpServiceFactory, ResourceDef}, + guard::Guard, + handler::Handler, + responder::Responder, + route::{Route, RouteService}, + service::{ServiceRequest, ServiceResponse}, + Error, FromRequest, HttpResponse, +}; type HttpService = BoxService; type HttpNewService = BoxServiceFactory<(), ServiceRequest, ServiceResponse, Error, ()>; diff --git a/src/responder.rs b/src/responder.rs index 8bf8d9ea0..c5852a501 100644 --- a/src/responder.rs +++ b/src/responder.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, fmt}; +use std::borrow::Cow; use actix_http::{ body::Body, @@ -6,7 +6,7 @@ use actix_http::{ }; use bytes::{Bytes, BytesMut}; -use crate::{error::InternalError, Error, HttpRequest, HttpResponse, HttpResponseBuilder}; +use crate::{Error, HttpRequest, HttpResponse, HttpResponseBuilder}; /// Trait implemented by types that can be converted to an HTTP response. /// @@ -226,15 +226,6 @@ impl Responder for CustomResponder { } } -impl Responder for InternalError -where - T: fmt::Debug + fmt::Display + 'static, -{ - fn respond_to(self, _: &HttpRequest) -> HttpResponse { - HttpResponse::from_error(self.into()) - } -} - #[cfg(test)] pub(crate) mod tests { use actix_service::Service; diff --git a/src/response/builder.rs b/src/response/builder.rs index b9a10c56b..6e013cae2 100644 --- a/src/response/builder.rs +++ b/src/response/builder.rs @@ -1,13 +1,14 @@ use std::{ cell::{Ref, RefMut}, convert::TryInto, + error::Error as StdError, future::Future, pin::Pin, task::{Context, Poll}, }; use actix_http::{ - body::{Body, BodyStream}, + body::{AnyBody, BodyStream}, http::{ header::{self, HeaderName, IntoHeaderPair, IntoHeaderValue}, ConnectionType, Error as HttpError, StatusCode, @@ -32,7 +33,7 @@ use crate::{ /// /// This type can be used to construct an instance of `Response` through a builder-like pattern. pub struct HttpResponseBuilder { - res: Option>, + res: Option>, err: Option, #[cfg(feature = "cookies")] cookies: Option, @@ -310,7 +311,7 @@ impl HttpResponseBuilder { /// /// `HttpResponseBuilder` can not be used after this call. #[inline] - pub fn body>(&mut self, body: B) -> HttpResponse { + pub fn body>(&mut self, body: B) -> HttpResponse { match self.message_body(body.into()) { Ok(res) => res, Err(err) => HttpResponse::from_error(err), @@ -354,9 +355,9 @@ impl HttpResponseBuilder { pub fn streaming(&mut self, stream: S) -> HttpResponse where S: Stream> + Unpin + 'static, - E: Into + 'static, + E: Into> + 'static, { - self.body(Body::from_message(BodyStream::new(stream))) + self.body(AnyBody::from_message(BodyStream::new(stream))) } /// Set a json body and generate `Response` @@ -375,9 +376,9 @@ impl HttpResponseBuilder { self.insert_header((header::CONTENT_TYPE, mime::APPLICATION_JSON)); } - self.body(Body::from(body)) + self.body(AnyBody::from(body)) } - Err(err) => HttpResponse::from_error(JsonPayloadError::Serialize(err).into()), + Err(err) => HttpResponse::from_error(JsonPayloadError::Serialize(err)), } } @@ -386,7 +387,7 @@ impl HttpResponseBuilder { /// `HttpResponseBuilder` can not be used after this call. #[inline] pub fn finish(&mut self) -> HttpResponse { - self.body(Body::Empty) + self.body(AnyBody::Empty) } /// This method construct new `HttpResponseBuilder` @@ -415,7 +416,7 @@ impl From for HttpResponse { } } -impl From for Response { +impl From for Response { fn from(mut builder: HttpResponseBuilder) -> Self { builder.finish().into() } diff --git a/src/response/response.rs b/src/response/response.rs index 194e2dff8..9dd804be0 100644 --- a/src/response/response.rs +++ b/src/response/response.rs @@ -8,7 +8,7 @@ use std::{ }; use actix_http::{ - body::{Body, MessageBody}, + body::{AnyBody, Body, MessageBody}, http::{header::HeaderMap, StatusCode}, Extensions, Response, ResponseHead, }; @@ -25,12 +25,12 @@ use { use crate::{error::Error, HttpResponseBuilder}; /// An HTTP Response -pub struct HttpResponse { +pub struct HttpResponse { res: Response, pub(crate) error: Option, } -impl HttpResponse { +impl HttpResponse { /// Create HTTP response builder with specific status. #[inline] pub fn build(status: StatusCode) -> HttpResponseBuilder { @@ -48,13 +48,8 @@ impl HttpResponse { /// Create an error response. #[inline] - pub fn from_error(error: Error) -> Self { - let res = error.as_response_error().error_response(); - - Self { - res, - error: Some(error), - } + pub fn from_error(error: impl Into) -> Self { + error.into().as_response_error().error_response() } } @@ -238,7 +233,6 @@ impl HttpResponse { impl fmt::Debug for HttpResponse where B: MessageBody, - B::Error: Into, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("HttpResponse") diff --git a/src/route.rs b/src/route.rs index 0a297b456..44f7e30b8 100644 --- a/src/route.rs +++ b/src/route.rs @@ -2,19 +2,19 @@ use std::{future::Future, rc::Rc}; -use actix_http::{http::Method, Error}; +use actix_http::http::Method; use actix_service::{ boxed::{self, BoxService, BoxServiceFactory}, Service, ServiceFactory, }; use futures_core::future::LocalBoxFuture; -use crate::extract::FromRequest; -use crate::guard::{self, Guard}; -use crate::handler::{Handler, HandlerService}; -use crate::responder::Responder; -use crate::service::{ServiceRequest, ServiceResponse}; -use crate::HttpResponse; +use crate::{ + guard::{self, Guard}, + handler::{Handler, HandlerService}, + service::{ServiceRequest, ServiceResponse}, + Error, FromRequest, HttpResponse, Responder, +}; /// Resource route definition /// @@ -188,7 +188,7 @@ impl Route { #[cfg(test)] mod tests { - use std::time::Duration; + use std::{convert::Infallible, time::Duration}; use actix_rt::time::sleep; use bytes::Bytes; @@ -215,7 +215,7 @@ mod tests { })) .route(web::post().to(|| async { sleep(Duration::from_millis(100)).await; - Ok::<_, ()>(HttpResponse::Created()) + Ok::<_, Infallible>(HttpResponse::Created()) })) .route(web::delete().to(|| async { sleep(Duration::from_millis(100)).await; diff --git a/src/server.rs b/src/server.rs index 80e300b9a..89328215d 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,23 +1,25 @@ use std::{ any::Any, - cmp, fmt, io, + cmp, + error::Error as StdError, + fmt, io, marker::PhantomData, net, sync::{Arc, Mutex}, }; -use actix_http::{ - body::MessageBody, Error, Extensions, HttpService, KeepAlive, Request, Response, -}; +use actix_http::{body::MessageBody, Extensions, HttpService, KeepAlive, Request, Response}; use actix_server::{Server, ServerBuilder}; -use actix_service::{map_config, IntoServiceFactory, Service, ServiceFactory}; +use actix_service::{ + map_config, IntoServiceFactory, Service, ServiceFactory, ServiceFactoryExt as _, +}; #[cfg(feature = "openssl")] use actix_tls::accept::openssl::{AlpnError, SslAcceptor, SslAcceptorBuilder}; #[cfg(feature = "rustls")] use actix_tls::accept::rustls::ServerConfig as RustlsServerConfig; -use crate::config::AppConfig; +use crate::{config::AppConfig, Error}; struct Socket { scheme: &'static str, @@ -81,7 +83,7 @@ where S::Service: 'static, // S::Service: 'static, B: MessageBody + 'static, - B::Error: Into, + B::Error: Into>, { /// Create new HTTP server with application factory pub fn new(factory: F) -> Self { @@ -301,7 +303,11 @@ where svc }; - svc.finish(map_config(factory(), move |_| { + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + + svc.finish(map_config(fac, move |_| { AppConfig::new(false, host.clone(), addr) })) .tcp() @@ -356,7 +362,11 @@ where svc }; - svc.finish(map_config(factory(), move |_| { + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + + svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) .openssl(acceptor.clone()) @@ -410,7 +420,11 @@ where svc }; - svc.finish(map_config(factory(), move |_| { + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + + svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) .rustls(config.clone()) @@ -533,7 +547,11 @@ where svc }; - svc.finish(map_config(factory(), move |_| config.clone())) + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + + svc.finish(map_config(fac, move |_| config.clone())) }) })?; Ok(self) @@ -568,14 +586,20 @@ where c.host.clone().unwrap_or_else(|| format!("{}", socket_addr)), socket_addr, ); + + let fac = factory() + .into_factory() + .map_err(|err| err.into().error_response()); + fn_service(|io: UnixStream| async { Ok((io, Protocol::Http1, None)) }).and_then( HttpService::build() .keep_alive(c.keep_alive) .client_timeout(c.client_timeout) - .finish(map_config(factory(), move |_| config.clone())), + .finish(map_config(fac, move |_| config.clone())), ) }, )?; + Ok(self) } } diff --git a/src/service.rs b/src/service.rs index b7f244797..2956fe6cb 100644 --- a/src/service.rs +++ b/src/service.rs @@ -2,24 +2,24 @@ use std::cell::{Ref, RefMut}; use std::rc::Rc; use std::{fmt, net}; -use actix_http::body::{Body, MessageBody}; -use actix_http::http::{HeaderMap, Method, StatusCode, Uri, Version}; use actix_http::{ - Error, Extensions, HttpMessage, Payload, PayloadStream, RequestHead, Response, ResponseHead, + body::{AnyBody, MessageBody}, + http::{HeaderMap, Method, StatusCode, Uri, Version}, + Extensions, HttpMessage, Payload, PayloadStream, RequestHead, Response, ResponseHead, }; use actix_router::{IntoPattern, Path, Resource, ResourceDef, Url}; use actix_service::{IntoServiceFactory, ServiceFactory}; #[cfg(feature = "cookies")] use cookie::{Cookie, ParseError as CookieParseError}; -use crate::dev::insert_slash; -use crate::guard::Guard; -use crate::info::ConnectionInfo; -use crate::request::HttpRequest; -use crate::rmap::ResourceMap; use crate::{ config::{AppConfig, AppService}, - HttpResponse, + dev::insert_slash, + guard::Guard, + info::ConnectionInfo, + request::HttpRequest, + rmap::ResourceMap, + Error, HttpResponse, }; pub trait HttpServiceFactory { @@ -330,15 +330,15 @@ impl fmt::Debug for ServiceRequest { } } -pub struct ServiceResponse { +pub struct ServiceResponse { request: HttpRequest, response: HttpResponse, } -impl ServiceResponse { +impl ServiceResponse { /// Create service response from the error pub fn from_err>(err: E, request: HttpRequest) -> Self { - let response = HttpResponse::from_error(err.into()); + let response = HttpResponse::from_error(err); ServiceResponse { request, response } } } diff --git a/src/types/form.rs b/src/types/form.rs index d1deac937..ce85983d1 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -188,7 +188,7 @@ impl Responder for Form { Ok(body) => HttpResponse::Ok() .content_type(mime::APPLICATION_WWW_FORM_URLENCODED) .body(body), - Err(err) => HttpResponse::from_error(UrlencodedError::Serialize(err).into()), + Err(err) => HttpResponse::from_error(UrlencodedError::Serialize(err)), } } } diff --git a/src/types/json.rs b/src/types/json.rs index 24abcecea..44b548355 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -127,7 +127,7 @@ impl Responder for Json { Ok(body) => HttpResponse::Ok() .content_type(mime::APPLICATION_JSON) .body(body), - Err(err) => HttpResponse::from_error(JsonPayloadError::Serialize(err).into()), + Err(err) => HttpResponse::from_error(JsonPayloadError::Serialize(err)), } } } @@ -500,7 +500,7 @@ mod tests { }; let resp = HttpResponse::BadRequest().body(serde_json::to_string(&msg).unwrap()); - InternalError::from_response(err, resp.into()).into() + InternalError::from_response(err, resp).into() })) .to_http_parts(); diff --git a/src/types/path.rs b/src/types/path.rs index 59a107a7e..9dab79414 100644 --- a/src/types/path.rs +++ b/src/types/path.rs @@ -2,14 +2,13 @@ use std::{fmt, ops, sync::Arc}; -use actix_http::error::Error; use actix_router::PathDeserializer; use actix_utils::future::{ready, Ready}; use serde::de; use crate::{ dev::Payload, - error::{ErrorNotFound, PathError}, + error::{Error, ErrorNotFound, PathError}, FromRequest, HttpRequest, }; @@ -296,11 +295,8 @@ mod tests { async fn test_custom_err_handler() { let (req, mut pl) = TestRequest::with_uri("/name/user1/") .app_data(PathConfig::default().error_handler(|err, _| { - error::InternalError::from_response( - err, - HttpResponse::Conflict().finish().into(), - ) - .into() + error::InternalError::from_response(err, HttpResponse::Conflict().finish()) + .into() })) .to_http_parts(); diff --git a/src/types/query.rs b/src/types/query.rs index 978d00b5f..613a438d3 100644 --- a/src/types/query.rs +++ b/src/types/query.rs @@ -267,7 +267,7 @@ mod tests { let req = TestRequest::with_uri("/name/user1/") .app_data(QueryConfig::default().error_handler(|e, _| { let resp = HttpResponse::UnprocessableEntity().finish(); - InternalError::from_response(e, resp.into()).into() + InternalError::from_response(e, resp).into() })) .to_srv_request();