From 76ae9a0302fe6b5585670b058d03ba73661b5269 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 5 Jun 2021 18:15:08 +0100 Subject: [PATCH] review tweaks --- CHANGES.md | 6 +-- actix-http/CHANGES.md | 8 ++-- actix-http/src/h1/encoder.rs | 9 ++-- actix-http/src/response.rs | 1 - awc/src/lib.rs | 3 +- src/error/error.rs | 75 ++++++++++++++++++++++++++++++++ src/error/internal.rs | 11 ++++- src/error/mod.rs | 5 ++- src/error/response_error.rs | 84 ------------------------------------ src/extract.rs | 6 +-- src/lib.rs | 12 ++---- src/request.rs | 3 +- src/responder.rs | 13 +----- src/response/response.rs | 12 +----- 14 files changed, 110 insertions(+), 138 deletions(-) create mode 100644 src/error/error.rs diff --git a/CHANGES.md b/CHANGES.md index 38c0ac680..32c6c357e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,8 +10,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>`. [#2224] -* All error trait bounds in message body and stream impls changed from `Into` to `Into>`. [#2224] +* 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] * `middleware::normalize` now will not try to normalize URIs with no valid path [#2246] ### Removed @@ -19,7 +19,7 @@ [#2200]: https://github.com/actix/actix-web/pull/2200 [#2201]: https://github.com/actix/actix-web/pull/2201 -[#2224]: https://github.com/actix/actix-web/pull/2224 +[#2253]: https://github.com/actix/actix-web/pull/2253 [#2246]: https://github.com/actix/actix-web/pull/2246 diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 898c34513..8ccc49fa2 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -13,15 +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>`. [#2224] -* All error trait bounds in message body and stream impls changed from `Into` to `Into>`. [#2224] +* 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`. [#2224] +* Remove `Unpin` bound on `ResponseBuilder::streaming`. [#2253] ### Removed * Stop re-exporting `http` crate's `HeaderMap` types in addition to ours. [#2171] @@ -39,7 +39,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 -[#2224]: https://github.com/actix/actix-web/pull/2224 +[#2253]: https://github.com/actix/actix-web/pull/2253 [#2244]: https://github.com/actix/actix-web/pull/2244 diff --git a/actix-http/src/h1/encoder.rs b/actix-http/src/h1/encoder.rs index ede5ea517..254981123 100644 --- a/actix-http/src/h1/encoder.rs +++ b/actix-http/src/h1/encoder.rs @@ -9,14 +9,11 @@ use bytes::{BufMut, BytesMut}; use crate::{ body::BodySize, config::ServiceConfig, - header::{map::Value, HeaderName}, + header::{map::Value, HeaderMap, HeaderName}, + header::{CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCODING}, helpers, - http::{ - header::{CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCODING}, - HeaderMap, StatusCode, Version, - }, message::{ConnectionType, RequestHeadType}, - response::Response, + Response, StatusCode, Version, }; const AVERAGE_HEADER_SIZE: usize = 30; diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index 61ba9367b..2aa38c153 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -198,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!( 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/src/error/error.rs b/src/error/error.rs new file mode 100644 index 000000000..10ebe1347 --- /dev/null +++ b/src/error/error.rs @@ -0,0 +1,75 @@ +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()) + } + + /// TODO + 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)> { + 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 3708f0fd6..1d9ca904e 100644 --- a/src/error/internal.rs +++ b/src/error/internal.rs @@ -3,7 +3,7 @@ use std::{cell::RefCell, fmt, io::Write as _}; 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. /// @@ -102,6 +102,15 @@ where } } +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! { diff --git a/src/error/mod.rs b/src/error/mod.rs index cc6d09b48..3590198b0 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -9,12 +9,15 @@ 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::{Error, ResponseError}; +pub use self::response_error::ResponseError; /// A convenience [`Result`](std::result::Result) for Actix Web operations. /// diff --git a/src/error/response_error.rs b/src/error/response_error.rs index 6732fd143..c58fff8be 100644 --- a/src/error/response_error.rs +++ b/src/error/response_error.rs @@ -12,90 +12,6 @@ use bytes::BytesMut; use crate::{__downcast_dyn, __downcast_get_type_id}; use crate::{helpers, HttpResponse}; -/// 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()) - } - - /// TODO - 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)> { - 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() - } -} - -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// -///////////////////// - /// Errors that can generate responses. // TODO: add std::error::Error bound when replacement for Box is found pub trait ResponseError: fmt::Debug + fmt::Display { 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/lib.rs b/src/lib.rs index 332cff1dd..f57badaf8 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` diff --git a/src/request.rs b/src/request.rs index 752a88d61..42c722c46 100644 --- a/src/request.rs +++ b/src/request.rs @@ -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/responder.rs b/src/responder.rs index 60bb2b973..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) - } -} - #[cfg(test)] pub(crate) mod tests { use actix_service::Service; diff --git a/src/response/response.rs b/src/response/response.rs index 354bbd56b..9dd804be0 100644 --- a/src/response/response.rs +++ b/src/response/response.rs @@ -22,10 +22,7 @@ use { cookie::Cookie, }; -use crate::{ - error::{Error, ResponseError}, - HttpResponseBuilder, -}; +use crate::{error::Error, HttpResponseBuilder}; /// An HTTP Response pub struct HttpResponse { @@ -54,12 +51,6 @@ impl HttpResponse { pub fn from_error(error: impl Into) -> Self { error.into().as_response_error().error_response() } - - /// Create an error response. - #[inline] - pub fn from_http_error(error: &dyn ResponseError) -> Self { - error.error_response() - } } impl HttpResponse { @@ -242,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")