refactor: report custom reason phrase in error message (#2697)

Closes #2565

Author: chanbengz

src/async_impl/response.rs

@@ -385,8 +385,9 @@ impl Response {
     /// ```
     pub fn error_for_status(self) -> crate::Result<Self> {
         let status = self.status();
+        let reason = self.extensions().get::<hyper::ext::ReasonPhrase>().cloned();
         if status.is_client_error() || status.is_server_error() {
-            Err(crate::error::status_code(*self.url, status))
+            Err(crate::error::status_code(*self.url, status, reason))
         } else {
             Ok(self)
         }
@@ -415,8 +416,9 @@ impl Response {
     /// ```
     pub fn error_for_status_ref(&self) -> crate::Result<&Self> {
         let status = self.status();
+        let reason = self.extensions().get::<hyper::ext::ReasonPhrase>().cloned();
         if status.is_client_error() || status.is_server_error() {
-            Err(crate::error::status_code(*self.url.clone(), status))
+            Err(crate::error::status_code(*self.url.clone(), status, reason))
         } else {
             Ok(self)
         }

src/connect.rs

@@ -1289,6 +1289,7 @@ mod socks {
 }
 
 mod verbose {
+    use crate::util::Escape;
     use hyper::rt::{Read, ReadBufCursor, Write};
     use hyper_util::client::legacy::connect::{Connected, Connection};
     use std::cmp::min;
@@ -1339,7 +1340,7 @@ mod verbose {
             let mut vbuf = hyper::rt::ReadBuf::uninit(unsafe { buf.as_mut() });
             match Pin::new(&mut self.inner).poll_read(cx, vbuf.unfilled()) {
                 Poll::Ready(Ok(())) => {
-                    log::trace!("{:08x} read: {:?}", self.id, Escape(vbuf.filled()));
+                    log::trace!("{:08x} read: {:?}", self.id, Escape::new(vbuf.filled()));
                     let len = vbuf.filled().len();
                     // SAFETY: The two cursors were for the same buffer. What was
                     // filled in one is safe in the other.
@@ -1362,7 +1363,7 @@ mod verbose {
         ) -> Poll<Result<usize, std::io::Error>> {
             match Pin::new(&mut self.inner).poll_write(cx, buf) {
                 Poll::Ready(Ok(n)) => {
-                    log::trace!("{:08x} write: {:?}", self.id, Escape(&buf[..n]));
+                    log::trace!("{:08x} write: {:?}", self.id, Escape::new(&buf[..n]));
                     Poll::Ready(Ok(n))
                 }
                 Poll::Ready(Err(e)) => Poll::Ready(Err(e)),
@@ -1415,35 +1416,6 @@ mod verbose {
         }
     }
 
-    struct Escape<'a>(&'a [u8]);
-
-    impl fmt::Debug for Escape<'_> {
-        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-            write!(f, "b\"")?;
-            for &c in self.0 {
-                // https://doc.rust-lang.org/reference.html#byte-escapes
-                if c == b'\n' {
-                    write!(f, "\\n")?;
-                } else if c == b'\r' {
-                    write!(f, "\\r")?;
-                } else if c == b'\t' {
-                    write!(f, "\\t")?;
-                } else if c == b'\\' || c == b'"' {
-                    write!(f, "\\{}", c as char)?;
-                } else if c == b'\0' {
-                    write!(f, "\\0")?;
-                // ASCII printable
-                } else if c >= 0x20 && c < 0x7f {
-                    write!(f, "{}", c as char)?;
-                } else {
-                    write!(f, "\\x{c:02x}")?;
-                }
-            }
-            write!(f, "\"")?;
-            Ok(())
-        }
-    }
-
     struct Vectored<'a, 'b> {
         bufs: &'a [IoSlice<'b>],
         nwritten: usize,
@@ -1457,7 +1429,7 @@ mod verbose {
                     break;
                 }
                 let n = min(left, buf.len());
-                Escape(&buf[..n]).fmt(f)?;
+                Escape::new(&buf[..n]).fmt(f)?;
                 left -= n;
             }
             Ok(())

src/error.rs

@@ -3,6 +3,7 @@ use std::error::Error as StdError;
 use std::fmt;
 use std::io;
 
+use crate::util::Escape;
 use crate::{StatusCode, Url};
 
 /// A `Result` alias where the `Err` case is `reqwest::Error`.
@@ -93,8 +94,9 @@ impl Error {
     }
 
     /// Returns true if the error is from `Response::error_for_status`.
+    #[cfg(not(target_arch = "wasm32"))]
     pub fn is_status(&self) -> bool {
-        matches!(self.inner.kind, Kind::Status(_))
+        matches!(self.inner.kind, Kind::Status(_, _))
     }
 
     /// Returns true if the error is related to a timeout.
@@ -152,7 +154,10 @@ impl Error {
     /// Returns the status code, if the error was generated from a response.
     pub fn status(&self) -> Option<StatusCode> {
         match self.inner.kind {
+            #[cfg(target_arch = "wasm32")]
             Kind::Status(code) => Some(code),
+            #[cfg(not(target_arch = "wasm32"))]
+            Kind::Status(code, _) => Some(code),
             _ => None,
         }
     }
@@ -204,6 +209,7 @@ impl fmt::Display for Error {
             Kind::Decode => f.write_str("error decoding response body")?,
             Kind::Redirect => f.write_str("error following redirect")?,
             Kind::Upgrade => f.write_str("error upgrading connection")?,
+            #[cfg(target_arch = "wasm32")]
             Kind::Status(ref code) => {
                 let prefix = if code.is_client_error() {
                     "HTTP status client error"
@@ -213,6 +219,25 @@ impl fmt::Display for Error {
                 };
                 write!(f, "{prefix} ({code})")?;
             }
+            #[cfg(not(target_arch = "wasm32"))]
+            Kind::Status(ref code, ref reason) => {
+                let prefix = if code.is_client_error() {
+                    "HTTP status client error"
+                } else {
+                    debug_assert!(code.is_server_error());
+                    "HTTP status server error"
+                };
+                if let Some(reason) = reason {
+                    write!(
+                        f,
+                        "{prefix} ({} {})",
+                        code.as_str(),
+                        Escape::new(reason.as_bytes())
+                    )?;
+                } else {
+                    write!(f, "{prefix} ({code})")?;
+                }
+            }
         };
 
         if let Some(url) = &self.inner.url {
@@ -248,6 +273,9 @@ pub(crate) enum Kind {
     Builder,
     Request,
     Redirect,
+    #[cfg(not(target_arch = "wasm32"))]
+    Status(StatusCode, Option<hyper::ext::ReasonPhrase>),
+    #[cfg(target_arch = "wasm32")]
     Status(StatusCode),
     Body,
     Decode,
@@ -276,8 +304,20 @@ pub(crate) fn redirect<E: Into<BoxError>>(e: E, url: Url) -> Error {
     Error::new(Kind::Redirect, Some(e)).with_url(url)
 }
 
-pub(crate) fn status_code(url: Url, status: StatusCode) -> Error {
-    Error::new(Kind::Status(status), None::<Error>).with_url(url)
+pub(crate) fn status_code(
+    url: Url,
+    status: StatusCode,
+    #[cfg(not(target_arch = "wasm32"))] reason: Option<hyper::ext::ReasonPhrase>,
+) -> Error {
+    Error::new(
+        Kind::Status(
+            status,
+            #[cfg(not(target_arch = "wasm32"))]
+            reason,
+        ),
+        None::<Error>,
+    )
+    .with_url(url)
 }
 
 pub(crate) fn url_bad_scheme(url: Url) -> Error {

src/util.rs

@@ -1,4 +1,5 @@
 use crate::header::{Entry, HeaderMap, HeaderValue, OccupiedEntry};
+use std::fmt;
 
 pub fn basic_auth<U, P>(username: U, password: Option<P>) -> HeaderValue
 where
@@ -98,3 +99,44 @@ pub(crate) fn add_cookie_header(
         headers.insert(crate::header::COOKIE, header);
     }
 }
+
+pub(crate) struct Escape<'a>(&'a [u8]);
+
+#[cfg(not(target_arch = "wasm32"))]
+impl<'a> Escape<'a> {
+    pub(crate) fn new(bytes: &'a [u8]) -> Self {
+        Escape(bytes)
+    }
+}
+
+impl fmt::Debug for Escape<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "b\"{}\"", self)?;
+        Ok(())
+    }
+}
+
+impl fmt::Display for Escape<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for &c in self.0 {
+            // https://doc.rust-lang.org/reference.html#byte-escapes
+            if c == b'\n' {
+                write!(f, "\\n")?;
+            } else if c == b'\r' {
+                write!(f, "\\r")?;
+            } else if c == b'\t' {
+                write!(f, "\\t")?;
+            } else if c == b'\\' || c == b'"' {
+                write!(f, "\\{}", c as char)?;
+            } else if c == b'\0' {
+                write!(f, "\\0")?;
+            // ASCII printable
+            } else if c >= 0x20 && c < 0x7f {
+                write!(f, "{}", c as char)?;
+            } else {
+                write!(f, "\\x{c:02x}")?;
+            }
+        }
+        Ok(())
+    }
+}

tests/client.rs

@@ -2,13 +2,14 @@
 #![cfg(not(feature = "rustls-tls-manual-roots-no-provider"))]
 mod support;
 
-use support::server;
+use support::server::{self, low_level_with_response};
 
 use http::header::{CONTENT_LENGTH, CONTENT_TYPE, TRANSFER_ENCODING};
 #[cfg(feature = "json")]
 use std::collections::HashMap;
 
 use reqwest::Client;
+use tokio::io::AsyncWriteExt;
 
 #[tokio::test]
 async fn auto_headers() {
@@ -564,3 +565,31 @@ async fn close_connection_after_idle_timeout() {
         .iter()
         .any(|e| matches!(e, server::Event::ConnectionClosed)));
 }
+
+#[tokio::test]
+async fn http1_reason_phrase() {
+    let server = server::low_level_with_response(|_raw_request, client_socket| {
+        Box::new(async move {
+            client_socket
+                .write_all(b"HTTP/1.1 418 I'm not a teapot\r\nContent-Length: 0\r\n\r\n")
+                .await
+                .expect("response write_all failed");
+        })
+    });
+
+    let client = Client::new();
+
+    let res = client
+        .get(&format!("http://{}", server.addr()))
+        .send()
+        .await
+        .expect("Failed to get");
+
+    assert_eq!(
+        res.error_for_status().unwrap_err().to_string(),
+        format!(
+            "HTTP status client error (418 I'm not a teapot) for url (http://{}/)",
+            server.addr()
+        )
+    );
+}