Remove `RequestParts::take_extensions` (#699)

* Remove `RequestParts::take_extensions`

* fix out of date docs

* Remove RequestAlreadyExtracted and replace it with BodyAlreadyExtracted

* fix docs

* fix test

* Update axum-core/src/extract/mod.rs

Co-authored-by: Jonas Platte <jplatte@users.noreply.github.com>

* Remove macro only used once

Co-authored-by: Jonas Platte <jplatte@users.noreply.github.com>

axum-core/CHANGELOG.md

@@ -21,8 +21,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
         - `RequestAlreadyExtracted`
         - `RequestPartsAlreadyExtracted`
     - `<HeaderMap as FromRequest<_>>::Error` has been changed to `std::convert::Infallible`.
+- **breaking:** `axum::http::Extensions` is no longer an extractor (ie it
+  doesn't implement `FromRequest`). The `axum::extract::Extension` extractor is
+  _not_ impacted by this and works the same. This change makes it harder to
+  accidentally remove all extensions which would result in confusing errors
+  elsewhere ([#699])
+  This includes these breaking changes:
+    - `RequestParts::take_extensions` has been removed.
+    - `RequestParts::extensions` returns `&Extensions`.
+    - `RequestParts::extensions_mut` returns `&mut Extensions`.
+    - `RequestAlreadyExtracted` has been removed.
+    - `<Request as FromRequest>::Error` is now `BodyAlreadyExtracted`.
+    - `<http::request::Parts as FromRequest>::Error` is now `Infallible`.
+    - `ExtensionsAlreadyExtracted` has been removed.
 
 [#698]: https://github.com/tokio-rs/axum/pull/698
+[#699]: https://github.com/tokio-rs/axum/pull/699
 
 # 0.1.1 (06. December, 2021)
 

axum-core/src/extract/mod.rs

@@ -78,7 +78,7 @@ pub struct RequestParts<B> {
     uri: Uri,
     version: Version,
     headers: HeaderMap,
-    extensions: Option<Extensions>,
+    extensions: Extensions,
     body: Option<B>,
 }
 
@@ -108,52 +108,38 @@ impl<B> RequestParts<B> {
             uri,
             version,
             headers,
-            extensions: Some(extensions),
+            extensions,
             body: Some(body),
         }
     }
 
     /// Convert this `RequestParts` back into a [`Request`].
     ///
-    /// Fails if
+    /// Fails if The request body has been extracted, that is [`take_body`] has
+    /// been called.
     ///
-    /// - The full [`Extensions`] has been extracted, that is
-    /// [`take_extensions`] have been called.
-    /// - The request body has been extracted, that is [`take_body`] have been
-    /// called.
-    ///
-    /// [`take_extensions`]: RequestParts::take_extensions
     /// [`take_body`]: RequestParts::take_body
-    pub fn try_into_request(self) -> Result<Request<B>, RequestAlreadyExtracted> {
+    pub fn try_into_request(self) -> Result<Request<B>, BodyAlreadyExtracted> {
         let Self {
             method,
             uri,
             version,
             headers,
-            mut extensions,
+            extensions,
             mut body,
         } = self;
 
         let mut req = if let Some(body) = body.take() {
             Request::new(body)
         } else {
-            return Err(RequestAlreadyExtracted::BodyAlreadyExtracted(
-                BodyAlreadyExtracted,
-            ));
+            return Err(BodyAlreadyExtracted);
         };
 
         *req.method_mut() = method;
         *req.uri_mut() = uri;
         *req.version_mut() = version;
         *req.headers_mut() = headers;
-
-        if let Some(extensions) = extensions.take() {
-            *req.extensions_mut() = extensions;
-        } else {
-            return Err(RequestAlreadyExtracted::ExtensionsAlreadyExtracted(
-                ExtensionsAlreadyExtracted,
-            ));
-        }
+        *req.extensions_mut() = extensions;
 
         Ok(req)
     }
@@ -199,22 +185,13 @@ impl<B> RequestParts<B> {
     }
 
     /// Gets a reference to the request extensions.
-    ///
-    /// Returns `None` if the extensions has been taken by another extractor.
-    pub fn extensions(&self) -> Option<&Extensions> {
-        self.extensions.as_ref()
+    pub fn extensions(&self) -> &Extensions {
+        &self.extensions
     }
 
     /// Gets a mutable reference to the request extensions.
-    ///
-    /// Returns `None` if the extensions has been taken by another extractor.
-    pub fn extensions_mut(&mut self) -> Option<&mut Extensions> {
-        self.extensions.as_mut()
-    }
-
-    /// Takes the extensions out of the request, leaving a `None` in its place.
-    pub fn take_extensions(&mut self) -> Option<Extensions> {
-        self.extensions.take()
+    pub fn extensions_mut(&mut self) -> &mut Extensions {
+        &mut self.extensions
     }
 
     /// Gets a reference to the request body.

axum-core/src/extract/rejection.rs

@@ -1,21 +1,36 @@
 //! Rejection response types.
 
-define_rejection! {
-    #[status = INTERNAL_SERVER_ERROR]
-    #[body = "Cannot have two request body extractors for a single handler"]
-    /// Rejection type used if you try and extract the request body more than
-    /// once.
-    pub struct BodyAlreadyExtracted;
+use crate::body;
+use http::{Response, StatusCode};
+use http_body::Full;
+use std::fmt;
+
+/// Rejection type used if you try and extract the request body more than
+/// once.
+#[derive(Debug, Default)]
+#[non_exhaustive]
+pub struct BodyAlreadyExtracted;
+
+impl BodyAlreadyExtracted {
+    const BODY: &'static str = "Cannot have two request body extractors for a single handler";
 }
 
-define_rejection! {
-    #[status = INTERNAL_SERVER_ERROR]
-    #[body = "Extensions taken by other extractor"]
-    /// Rejection used if the request extension has been taken by another
-    /// extractor.
-    pub struct ExtensionsAlreadyExtracted;
+impl crate::response::IntoResponse for BodyAlreadyExtracted {
+    fn into_response(self) -> crate::response::Response {
+        let mut res = Response::new(body::boxed(Full::from(Self::BODY)));
+        *res.status_mut() = StatusCode::INTERNAL_SERVER_ERROR;
+        res
+    }
 }
 
+impl fmt::Display for BodyAlreadyExtracted {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", Self::BODY)
+    }
+}
+
+impl std::error::Error for BodyAlreadyExtracted {}
+
 define_rejection! {
     #[status = BAD_REQUEST]
     #[body = "Failed to buffer the request body"]
@@ -32,18 +47,6 @@ define_rejection! {
     pub struct InvalidUtf8(Error);
 }
 
-composite_rejection! {
-    /// Rejection used for [`Request<_>`].
-    ///
-    /// Contains one variant for each way the [`Request<_>`] extractor can fail.
-    ///
-    /// [`Request<_>`]: http::Request
-    pub enum RequestAlreadyExtracted {
-        BodyAlreadyExtracted,
-        ExtensionsAlreadyExtracted,
-    }
-}
-
 composite_rejection! {
     /// Rejection used for [`Bytes`](bytes::Bytes).
     ///
@@ -65,12 +68,3 @@ composite_rejection! {
         InvalidUtf8,
     }
 }
-
-composite_rejection! {
-    /// Rejection used for [`http::request::Parts`].
-    ///
-    /// Contains one variant for each way the [`http::request::Parts`] extractor can fail.
-    pub enum RequestPartsAlreadyExtracted {
-        ExtensionsAlreadyExtracted,
-    }
-}

axum-core/src/extract/request_parts.rs

@@ -10,7 +10,7 @@ impl<B> FromRequest<B> for Request<B>
 where
     B: Send,
 {
-    type Rejection = RequestAlreadyExtracted;
+    type Rejection = BodyAlreadyExtracted;
 
     async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
         let req = std::mem::replace(
@@ -20,7 +20,7 @@ where
                 version: req.version,
                 uri: req.uri.clone(),
                 headers: HeaderMap::new(),
-                extensions: None,
+                extensions: Extensions::default(),
                 body: None,
             },
         );
@@ -82,18 +82,6 @@ where
     }
 }
 
-#[async_trait]
-impl<B> FromRequest<B> for Extensions
-where
-    B: Send,
-{
-    type Rejection = ExtensionsAlreadyExtracted;
-
-    async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
-        req.take_extensions().ok_or(ExtensionsAlreadyExtracted)
-    }
-}
-
 #[async_trait]
 impl<B> FromRequest<B> for Bytes
 where
@@ -142,17 +130,14 @@ impl<B> FromRequest<B> for http::request::Parts
 where
     B: Send,
 {
-    type Rejection = RequestPartsAlreadyExtracted;
+    type Rejection = Infallible;
 
     async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
         let method = unwrap_infallible(Method::from_request(req).await);
         let uri = unwrap_infallible(Uri::from_request(req).await);
         let version = unwrap_infallible(Version::from_request(req).await);
-        let headers = match HeaderMap::from_request(req).await {
-            Ok(headers) => headers,
-            Err(err) => match err {},
-        };
-        let extensions = Extensions::from_request(req).await?;
+        let headers = unwrap_infallible(HeaderMap::from_request(req).await);
+        let extensions = std::mem::take(req.extensions_mut());
 
         let mut temp_request = Request::new(());
         *temp_request.method_mut() = method;

axum-core/src/macros.rs

@@ -1,39 +1,4 @@
 macro_rules! define_rejection {
-    (
-        #[status = $status:ident]
-        #[body = $body:expr]
-        $(#[$m:meta])*
-        pub struct $name:ident;
-    ) => {
-        $(#[$m])*
-        #[derive(Debug)]
-        #[non_exhaustive]
-        pub struct $name;
-
-        #[allow(deprecated)]
-        impl $crate::response::IntoResponse for $name {
-            fn into_response(self) -> $crate::response::Response {
-                let mut res = http::Response::new($crate::body::boxed(http_body::Full::from($body)));
-                *res.status_mut() = http::StatusCode::$status;
-                res
-            }
-        }
-
-        impl std::fmt::Display for $name {
-            fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-                write!(f, "{}", $body)
-            }
-        }
-
-        impl std::error::Error for $name {}
-
-        impl Default for $name {
-            fn default() -> Self {
-                Self
-            }
-        }
-    };
-
     (
         #[status = $status:ident]
         #[body = $body:expr]

axum-extra/CHANGELOG.md

@@ -12,8 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 # 0.1.2 (13. January, 2021)
 
 - **fix:** Depend on tower with `default_features = false` ([#666])
+- **breaking:** `CachedRejection` has been removed ([#699])
+- **breaking:** `<Cached<T> as FromRequest>::Error` is now `T::Rejection`. ([#699])
 
 [#666]: https://github.com/tokio-rs/axum/pull/666
+[#699]: https://github.com/tokio-rs/axum/pull/699
 
 # 0.1.1 (27. December, 2021)
 

axum-extra/src/extract/cached.rs

@@ -1,15 +1,8 @@
 use axum::{
     async_trait,
-    extract::{
-        rejection::{ExtensionRejection, ExtensionsAlreadyExtracted},
-        Extension, FromRequest, RequestParts,
-    },
-    response::{IntoResponse, Response},
-};
-use std::{
-    fmt,
-    ops::{Deref, DerefMut},
+    extract::{Extension, FromRequest, RequestParts},
 };
+use std::ops::{Deref, DerefMut};
 
 /// Cache results of other extractors.
 ///
@@ -100,25 +93,14 @@ where
     B: Send,
     T: FromRequest<B> + Clone + Send + Sync + 'static,
 {
-    type Rejection = CachedRejection<T::Rejection>;
+    type Rejection = T::Rejection;
 
     async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
         match Extension::<CachedEntry<T>>::from_request(req).await {
             Ok(Extension(CachedEntry(value))) => Ok(Self(value)),
-            Err(ExtensionRejection::ExtensionsAlreadyExtracted(err)) => {
-                Err(CachedRejection::ExtensionsAlreadyExtracted(err))
-            }
             Err(_) => {
-                let value = T::from_request(req).await.map_err(CachedRejection::Inner)?;
-
-                req.extensions_mut()
-                    .ok_or_else(|| {
-                        CachedRejection::ExtensionsAlreadyExtracted(
-                            ExtensionsAlreadyExtracted::default(),
-                        )
-                    })?
-                    .insert(CachedEntry(value.clone()));
-
+                let value = T::from_request(req).await?;
+                req.extensions_mut().insert(CachedEntry(value.clone()));
                 Ok(Self(value))
             }
         }
@@ -139,54 +121,6 @@ impl<T> DerefMut for Cached<T> {
     }
 }
 
-/// Rejection used for [`Cached`].
-///
-/// Contains one variant for each way the [`Cached`] extractor can fail.
-#[derive(Debug)]
-#[non_exhaustive]
-pub enum CachedRejection<R> {
-    #[allow(missing_docs)]
-    ExtensionsAlreadyExtracted(ExtensionsAlreadyExtracted),
-    #[allow(missing_docs)]
-    Inner(R),
-}
-
-impl<R> IntoResponse for CachedRejection<R>
-where
-    R: IntoResponse,
-{
-    fn into_response(self) -> Response {
-        match self {
-            Self::ExtensionsAlreadyExtracted(inner) => inner.into_response(),
-            Self::Inner(inner) => inner.into_response(),
-        }
-    }
-}
-
-impl<R> fmt::Display for CachedRejection<R>
-where
-    R: fmt::Display,
-{
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::ExtensionsAlreadyExtracted(inner) => write!(f, "{}", inner),
-            Self::Inner(inner) => write!(f, "{}", inner),
-        }
-    }
-}
-
-impl<R> std::error::Error for CachedRejection<R>
-where
-    R: std::error::Error + 'static,
-{
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match self {
-            Self::ExtensionsAlreadyExtracted(inner) => Some(inner),
-            Self::Inner(inner) => Some(inner),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

axum-extra/src/extract/mod.rs

@@ -3,9 +3,3 @@
 mod cached;
 
 pub use self::cached::Cached;
-
-pub mod rejection {
-    //! Rejection response types.
-
-    pub use super::cached::CachedRejection;
-}

axum/CHANGELOG.md

@@ -31,10 +31,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
         - `ContentLengthLimitRejection`
         - `WebSocketUpgradeRejection`
     - `<HeaderMap as FromRequest<_>>::Error` has been changed to `std::convert::Infallible`.
+- **breaking:** `axum::http::Extensions` is no longer an extractor (ie it
+  doesn't implement `FromRequest`). The `axum::extract::Extension` extractor is
+  _not_ impacted by this and works the same. This change makes it harder to
+  accidentally remove all extensions which would result in confusing errors
+  elsewhere ([#699])
+  This includes these breaking changes:
+    - `RequestParts::take_extensions` has been removed.
+    - `RequestParts::extensions` returns `&Extensions`.
+    - `RequestParts::extensions_mut` returns `&mut Extensions`.
+    - `RequestAlreadyExtracted` has been removed.
+    - `<Request as FromRequest>::Error` is now `BodyAlreadyExtracted`.
+    - `<http::request::Parts as FromRequest>::Error` is now `Infallible`.
+    - `ExtensionsAlreadyExtracted` has been removed.
+    - The `ExtensionsAlreadyExtracted` removed variant has been removed from these rejections:
+        - `ExtensionRejection`
+        - `PathRejection`
+        - `MatchedPathRejection`
+        - `WebSocketUpgradeRejection`
 
 [#644]: https://github.com/tokio-rs/axum/pull/644
 [#665]: https://github.com/tokio-rs/axum/pull/665
 [#698]: https://github.com/tokio-rs/axum/pull/698
+[#699]: https://github.com/tokio-rs/axum/pull/699
 
 # 0.4.4 (13. January, 2021)
 

axum/src/extract/extension.rs

@@ -53,7 +53,6 @@ where
     async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
         let value = req
             .extensions()
-            .ok_or_else(ExtensionsAlreadyExtracted::default)?
             .get::<T>()
             .ok_or_else(|| {
                 MissingExtension::from_err(format!(

axum/src/extract/matched_path.rs

@@ -70,11 +70,8 @@ where
     type Rejection = MatchedPathRejection;
 
     async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
-        let extensions = req.extensions().ok_or_else(|| {
-            MatchedPathRejection::ExtensionsAlreadyExtracted(ExtensionsAlreadyExtracted::default())
-        })?;
-
-        let matched_path = extensions
+        let matched_path = req
+            .extensions()
             .get::<Self>()
             .ok_or(MatchedPathRejection::MatchedPathMissing(MatchedPathMissing))?
             .clone();

axum/src/extract/path/mod.rs

@@ -3,7 +3,6 @@
 
 mod de;
 
-use super::rejection::ExtensionsAlreadyExtracted;
 use crate::{
     body::{boxed, Full},
     extract::{rejection::*, FromRequest, RequestParts},
@@ -164,11 +163,7 @@ where
     type Rejection = PathRejection;
 
     async fn from_request(req: &mut RequestParts<B>) -> Result<Self, Self::Rejection> {
-        let ext = req
-            .extensions_mut()
-            .ok_or_else::<Self::Rejection, _>(|| ExtensionsAlreadyExtracted::default().into())?;
-
-        let params = match ext.get::<Option<UrlParams>>() {
+        let params = match req.extensions_mut().get::<Option<UrlParams>>() {
             Some(Some(UrlParams(Ok(params)))) => Cow::Borrowed(params),
             Some(Some(UrlParams(Err(InvalidUtf8InPathParam { key })))) => {
                 let err = PathDeserializationError {
@@ -519,6 +514,9 @@ mod tests {
 
         let res = client.get("/foo").send().await;
         assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR);
-        assert_eq!(res.text().await, "Extensions taken by other extractor");
+        assert_eq!(
+            res.text().await,
+            "No paths parameters found for matched route. Are you also extracting `Request<_>`?"
+        );
     }
 }

axum/src/extract/rejection.rs

@@ -52,9 +52,10 @@ define_rejection! {
 
 define_rejection! {
     #[status = INTERNAL_SERVER_ERROR]
-    #[body = "No paths parameters found for matched route. This is a bug in axum. Please open an issue"]
-    /// Rejection type used if axum's internal representation of path parameters is missing. This
-    /// should never happen and is a bug in axum if it does.
+    #[body = "No paths parameters found for matched route. Are you also extracting `Request<_>`?"]
+    /// Rejection type used if axum's internal representation of path parameters
+    /// is missing. This is commonly caused by extracting `Request<_>`. `Path`
+    /// must be extracted first.
     pub struct MissingPathParams;
 }
 
@@ -148,7 +149,6 @@ composite_rejection! {
     /// can fail.
     pub enum ExtensionRejection {
         MissingExtension,
-        ExtensionsAlreadyExtracted,
     }
 }
 
@@ -160,7 +160,6 @@ composite_rejection! {
     pub enum PathRejection {
         FailedToDeserializePathParams,
         MissingPathParams,
-        ExtensionsAlreadyExtracted,
     }
 }
 
@@ -176,7 +175,6 @@ define_rejection! {
 composite_rejection! {
     /// Rejection used for [`MatchedPath`](super::MatchedPath).
     pub enum MatchedPathRejection {
-        ExtensionsAlreadyExtracted,
         MatchedPathMissing,
     }
 }

axum/src/extract/ws.rs

@@ -64,7 +64,7 @@
 //! [`StreamExt::split`]: https://docs.rs/futures/0.3.17/futures/stream/trait.StreamExt.html#method.split
 
 use self::rejection::*;
-use super::{rejection::*, FromRequest, RequestParts};
+use super::{FromRequest, RequestParts};
 use crate::{
     body::{self, Bytes},
     response::Response,
@@ -268,11 +268,7 @@ where
                 return Err(WebSocketKeyHeaderMissing.into());
             };
 
-        let on_upgrade = req
-            .extensions_mut()
-            .ok_or_else(ExtensionsAlreadyExtracted::default)?
-            .remove::<OnUpgrade>()
-            .unwrap();
+        let on_upgrade = req.extensions_mut().remove::<OnUpgrade>().unwrap();
 
         let sec_websocket_protocol = req.headers().get(header::SEC_WEBSOCKET_PROTOCOL).cloned();
 
@@ -514,8 +510,6 @@ fn sign(key: &[u8]) -> HeaderValue {
 pub mod rejection {
     //! WebSocket specific rejections.
 
-    use crate::extract::rejection::*;
-
     define_rejection! {
         #[status = METHOD_NOT_ALLOWED]
         #[body = "Request method must be `GET`"]
@@ -562,7 +556,6 @@ pub mod rejection {
             InvalidUpgradeHeader,
             InvalidWebSocketVersionHeader,
             WebSocketKeyHeaderMissing,
-            ExtensionsAlreadyExtracted,
         }
     }
 }