Enable tracing of HTTP error bodies (#2214)

We will not trace HTTP error bodies by default, but this causes issues with internal diagnostics.

Resolves #2213. Once #682 is resolved, we could enable this de facto like in the Azure SDK for .NET.

Author: heaths

sdk/core/azure_core/Cargo.toml

@@ -50,7 +50,7 @@ hmac_rust = ["dep:sha2", "dep:hmac"]
 reqwest = ["typespec_client_core/reqwest"]
 reqwest_gzip = ["typespec_client_core/reqwest_gzip"]
 reqwest_rustls = ["typespec_client_core/reqwest_rustls"]
-test = []
+test = ["typespec_client_core/test"]
 tokio_fs = ["typespec_client_core/tokio_fs"]
 tokio_sleep = ["typespec_client_core/tokio_sleep"]
 xml = ["typespec_client_core/xml"]

sdk/typespec/typespec_client_core/Cargo.toml

@@ -53,6 +53,8 @@ json = ["typespec/json"]
 reqwest = ["reqwest/native-tls"]
 reqwest_gzip = ["reqwest/gzip"]
 reqwest_rustls = ["reqwest/rustls-tls-native-roots"]
+# Enables extra tracing including error bodies that may contain PII.
+test = []
 tokio_fs = ["tokio/fs", "tokio/sync", "tokio/io-util"]
 tokio_sleep = ["tokio/time"]
 xml = ["dep:quick-xml"]

sdk/typespec/typespec_client_core/src/error/http_error.rs

@@ -10,7 +10,7 @@ use crate::{
 };
 use bytes::Bytes;
 use serde::Deserialize;
-use std::{collections::HashMap, fmt};
+use std::{collections::HashMap, fmt, str};
 
 /// An HTTP error response.
 pub struct HttpError {
@@ -91,38 +91,90 @@ impl HttpError {
     }
 }
 
+struct Unquote<'a>(&'a str);
+impl fmt::Debug for Unquote<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(self.0)
+    }
+}
+
 impl fmt::Debug for HttpError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         // Elide potential PII since it's too easily to accidentally leak through Debug or Display.
-        f.debug_struct("HttpError")
+        let mut dbg = f.debug_struct("HttpError");
+
+        #[cfg_attr(not(feature = "test"), allow(unused_mut))]
+        let mut dbg = dbg
             .field("status", &self.status)
-            .field("details", &self.details)
-            .finish_non_exhaustive()
+            .field("details", &self.details);
+
+        #[cfg(feature = "test")]
+        {
+            dbg = dbg.field(
+                "body",
+                &Unquote(
+                    String::from_utf8(self.body.to_vec())
+                        .as_deref()
+                        .unwrap_or("(bytes)"),
+                ),
+            );
+        }
+
+        dbg.finish_non_exhaustive()
     }
 }
 
 impl fmt::Display for HttpError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        struct Unquote<'a>(&'a str);
-        impl fmt::Debug for Unquote<'_> {
+        struct Status(StatusCode);
+        impl fmt::Debug for Status {
             fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-                f.write_str(self.0)
+                f.write_fmt(format_args!(
+                    "{} ({})",
+                    std::convert::Into::<u16>::into(self.0),
+                    self.0.canonical_reason()
+                ))
             }
         }
 
         // Elide potential PII since it's too easily to accidentally leak through Debug or Display.
-        f.debug_struct("HttpError")
-            .field("Status", &Unquote(&self.status.to_string()))
+        let mut dbg = f.debug_struct("HttpError");
+
+        #[cfg_attr(not(feature = "test"), allow(unused_mut))]
+        let mut dbg = dbg
+            .field("Status", &Status(self.status))
             .field(
                 "Error Code",
                 &Unquote(
                     self.details
                         .code
                         .as_deref()
-                        .unwrap_or("(unknown error code)"),
+                        .unwrap_or("(error code unavailable)"),
                 ),
             )
-            .finish_non_exhaustive()
+            .field(
+                "Message",
+                &Unquote(
+                    self.details
+                        .message
+                        .as_deref()
+                        .unwrap_or("(message unavailable)"),
+                ),
+            );
+
+        #[cfg(feature = "test")]
+        {
+            dbg = dbg.field(
+                "Body",
+                &Unquote(
+                    String::from_utf8(self.body.to_vec())
+                        .as_deref()
+                        .unwrap_or("(bytes)"),
+                ),
+            );
+        }
+
+        dbg.finish_non_exhaustive()
     }
 }
 
@@ -228,11 +280,16 @@ mod tests {
                 ("x-ms-request-id".to_string(), "abcd1234".to_string()),
             ]),
         };
-        let actual = format!("{err:?}");
+        #[cfg(not(feature = "test"))]
         assert_eq!(
-            actual,
+            format!("{err:?}"),
             r#"HttpError { status: NotFound, details: ErrorDetails { code: Some("Not Found"), message: Some("Resource not found") }, .. }"#
         );
+        #[cfg(feature = "test")]
+        assert_eq!(
+            format!("{err:?}"),
+            r#"HttpError { status: NotFound, details: ErrorDetails { code: Some("Not Found"), message: Some("Resource not found") }, body: resource not found, .. }"#
+        );
     }
 
     #[test]
@@ -249,10 +306,16 @@ mod tests {
                 ("x-ms-request-id".to_string(), "abcd1234".to_string()),
             ]),
         };
-        let actual = format!("{err}");
+        #[cfg(not(feature = "test"))]
+        assert_eq!(
+            format!("{err:}"),
+            r#"HttpError { Status: 404 (Not Found), Error Code: (error code unavailable), Message: (message unavailable), .. }"#
+        );
+
+        #[cfg(feature = "test")]
         assert_eq!(
-            actual,
-            r#"HttpError { Status: 404, Error Code: (unknown error code), .. }"#
+            format!("{err:}"),
+            r#"HttpError { Status: 404 (Not Found), Error Code: (error code unavailable), Message: (message unavailable), Body: resource not found, .. }"#
         );
     }