Concrete body in SdkResult/SdkError (#247)

* Refactor out generic `B` parameter from SdkResult / SdkError

* Switch to from_static

* Add docs, fix old broken docs

* Remove unused From<Bytes> bound

* Rustfmt run

aws/rust-runtime/aws-http/src/lib.rs

@@ -48,11 +48,11 @@ impl Default for AwsErrorRetryPolicy {
     }
 }
 
-impl<T, E, B> ClassifyResponse<T, SdkError<E, B>> for AwsErrorRetryPolicy
+impl<T, E> ClassifyResponse<T, SdkError<E>> for AwsErrorRetryPolicy
 where
     E: ProvideErrorKind,
 {
-    fn classify(&self, err: Result<&T, &SdkError<E, B>>) -> RetryKind {
+    fn classify(&self, err: Result<&T, &SdkError<E>>) -> RetryKind {
         let (err, response) = match err {
             Ok(_) => return RetryKind::NotRetryable,
             Err(SdkError::ServiceError { err, raw }) => (err, raw),
@@ -88,6 +88,7 @@ where
 #[cfg(test)]
 mod test {
     use crate::AwsErrorRetryPolicy;
+    use smithy_http::middleware::ResponseBody;
     use smithy_http::result::{SdkError, SdkSuccess};
     use smithy_http::retry::ClassifyResponse;
     use smithy_types::retry::{ErrorKind, ProvideErrorKind, RetryKind};
@@ -119,8 +120,14 @@ mod test {
         }
     }
 
-    fn make_err<E, B>(err: E, raw: http::Response<B>) -> Result<SdkSuccess<(), B>, SdkError<E, B>> {
-        Err(SdkError::ServiceError { err, raw })
+    fn make_err<E>(
+        err: E,
+        raw: http::Response<&'static str>,
+    ) -> Result<SdkSuccess<()>, SdkError<E>> {
+        Err(SdkError::ServiceError {
+            err,
+            raw: raw.map(|b| ResponseBody::from_static(b)),
+        })
     }
 
     #[test]

aws/rust-runtime/aws-hyper/src/conn.rs

@@ -21,7 +21,9 @@ impl Standard {
     /// An https connection
     pub fn https() -> Self {
         let https = HttpsConnector::new();
-        Self(Connector::Https(hyper::Client::builder().build::<_, SdkBody>(https)))
+        Self(Connector::Https(
+            hyper::Client::builder().build::<_, SdkBody>(https),
+        ))
     }
 
     /// A connection based on the provided `impl HttpService`

aws/rust-runtime/aws-hyper/src/lib.rs

@@ -14,6 +14,7 @@ use aws_sig_auth::signer::SigV4Signer;
 use smithy_http::body::SdkBody;
 use smithy_http::operation::Operation;
 use smithy_http::response::ParseHttpResponse;
+pub use smithy_http::result::{SdkError, SdkSuccess};
 use smithy_http::retry::ClassifyResponse;
 use smithy_http_tower::dispatch::DispatchLayer;
 use smithy_http_tower::map_request::MapRequestLayer;
@@ -27,9 +28,6 @@ use tower::{Service, ServiceBuilder, ServiceExt};
 type BoxError = Box<dyn Error + Send + Sync>;
 pub type StandardClient = Client<conn::Standard>;
 
-pub type SdkError<E> = smithy_http::result::SdkError<E, hyper::Body>;
-pub type SdkSuccess<T> = smithy_http::result::SdkSuccess<T, hyper::Body>;
-
 /// AWS Service Client
 ///
 /// Hyper-based AWS Service Client. Most customers will want to construct a client with
@@ -138,8 +136,8 @@ where
 
 #[cfg(test)]
 mod tests {
-    use crate::{Client, conn};
     use crate::test_connection::TestConnection;
+    use crate::{conn, Client};
 
     #[test]
     fn construct_default_client() {

aws/rust-runtime/aws-hyper/src/test_connection.rs

@@ -5,13 +5,13 @@
 
 use http::header::{HeaderName, CONTENT_TYPE};
 use http::Request;
+use protocol_test_helpers::{assert_ok, validate_body, MediaType};
 use smithy_http::body::SdkBody;
 use std::future::Ready;
 use std::ops::Deref;
 use std::sync::{Arc, Mutex};
 use std::task::{Context, Poll};
 use tower::BoxError;
-use protocol_test_helpers::{validate_body, MediaType, assert_ok};
 
 type ConnectVec<B> = Vec<(http::Request<SdkBody>, http::Response<B>)>;
 
@@ -34,14 +34,18 @@ impl ValidateRequest {
         }
         let actual_str = std::str::from_utf8(actual.body().bytes().unwrap_or(&[]));
         let expected_str = std::str::from_utf8(expected.body().bytes().unwrap_or(&[]));
-        let media_type = if actual.headers().get(CONTENT_TYPE).map(|v| v.to_str().unwrap().contains("json")).unwrap_or(false) {
+        let media_type = if actual
+            .headers()
+            .get(CONTENT_TYPE)
+            .map(|v| v.to_str().unwrap().contains("json"))
+            .unwrap_or(false)
+        {
             MediaType::Json
         } else {
             MediaType::Other("unknown".to_string())
         };
         match (actual_str, expected_str) {
-            (Ok(actual), Ok(expected)) =>
-                assert_ok(validate_body(actual, expected, media_type)),
+            (Ok(actual), Ok(expected)) => assert_ok(validate_body(actual, expected, media_type)),
             _ => assert_eq!(actual.body().bytes(), expected.body().bytes()),
         };
         assert_eq!(actual.uri(), expected.uri());
@@ -94,7 +98,7 @@ impl<B> TestConnection<B> {
         }
     }
 
-    pub fn requests(&self) -> impl Deref<Target=Vec<ValidateRequest>> + '_ {
+    pub fn requests(&self) -> impl Deref<Target = Vec<ValidateRequest>> + '_ {
         self.requests.lock().unwrap()
     }
 }
@@ -133,9 +137,9 @@ mod tests {
     fn meets_trait_bounds() {
         fn check() -> impl tower::Service<
             http::Request<SdkBody>,
-            Response=http::Response<hyper::Body>,
-            Error=BoxError,
-            Future=impl Send,
+            Response = http::Response<hyper::Body>,
+            Error = BoxError,
+            Future = impl Send,
         > + Clone {
             TestConnection::<String>::new(vec![])
         }

aws/rust-runtime/aws-hyper/tests/e2e_test.rs

@@ -8,7 +8,7 @@ use aws_endpoint::{set_endpoint_resolver, DefaultAwsEndpointResolver};
 use aws_http::user_agent::AwsUserAgent;
 use aws_http::AwsErrorRetryPolicy;
 use aws_hyper::test_connection::TestConnection;
-use aws_hyper::{Client, RetryConfig, SdkError};
+use aws_hyper::{Client, RetryConfig};
 use aws_sig_auth::signer::OperationSigningConfig;
 use aws_types::region::Region;
 use bytes::Bytes;
@@ -18,6 +18,7 @@ use smithy_http::body::SdkBody;
 use smithy_http::operation;
 use smithy_http::operation::Operation;
 use smithy_http::response::ParseHttpResponse;
+use smithy_http::result::SdkError;
 use smithy_types::retry::{ErrorKind, ProvideErrorKind};
 use std::convert::Infallible;
 use std::error::Error;

aws/sdk/integration-tests/kms/tests/integration.rs

@@ -9,8 +9,8 @@ use aws_hyper::test_connection::TestConnection;
 use aws_hyper::Client;
 use http::Uri;
 use kms::operation::GenerateRandom;
-use kms::{Config, Region};
 use smithy_http::body::SdkBody;
+use kms::{Config, Region};
 use std::time::{Duration, UNIX_EPOCH};
 
 // TODO: having the full HTTP requests right in the code is a bit gross, consider something

aws/sdk/integration-tests/kms/tests/sensitive-it.rs

@@ -10,6 +10,7 @@ use kms::Blob;
 use smithy_http::result::{SdkError, SdkSuccess};
 use smithy_http::retry::ClassifyResponse;
 use smithy_types::retry::{ErrorKind, RetryKind};
+use smithy_http::middleware::ResponseBody;
 
 #[test]
 fn validate_sensitive_trait() {
@@ -29,9 +30,9 @@ fn errors_are_retryable() {
     let conf = kms::Config::builder().build();
 
     let op = CreateAlias::builder().build(&conf);
-    let err = Result::<SdkSuccess<CreateAliasOutput, &str>, SdkError<CreateAliasError, &str>>::Err(
+    let err = Result::<SdkSuccess<CreateAliasOutput>, SdkError<CreateAliasError>>::Err(
         SdkError::ServiceError {
-            raw: http::Response::builder().body("resp").unwrap(),
+            raw: http::Response::builder().body(ResponseBody::from_static("resp")).unwrap(),
             err,
         },
     );

rust-runtime/smithy-http-tower/src/lib.rs

@@ -33,7 +33,7 @@ pub enum SendOperationError {
 }
 
 /// Convert a `SendOperationError` into an `SdkError`
-impl<E, B> From<SendOperationError> for SdkError<E, B> {
+impl<E> From<SendOperationError> for SdkError<E> {
     fn from(err: SendOperationError) -> Self {
         match err {
             SendOperationError::RequestDispatchError(e) => {

rust-runtime/smithy-http-tower/src/parse_response.rs

@@ -4,7 +4,6 @@
  */
 
 use crate::SendOperationError;
-use bytes::Bytes;
 use smithy_http::middleware::load_response;
 use smithy_http::operation;
 use smithy_http::operation::Operation;
@@ -73,13 +72,13 @@ impl<S, O, T, E, B, R> tower::Service<operation::Operation<O, R>> for ParseRespo
 where
     S: Service<operation::Request, Response = http::Response<B>, Error = SendOperationError>,
     S::Future: 'static,
-    B: http_body::Body + Unpin + From<Bytes> + 'static,
+    B: http_body::Body + Unpin + 'static,
     B::Error: Into<BoxError>,
     O: ParseHttpResponse<B, Output = Result<T, E>> + 'static,
     E: Error,
 {
-    type Response = smithy_http::result::SdkSuccess<T, B>;
-    type Error = smithy_http::result::SdkError<E, B>;
+    type Response = smithy_http::result::SdkSuccess<T>;
+    type Error = smithy_http::result::SdkError<E>;
     type Future = BoxedResultFuture<Self::Response, Self::Error>;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {

rust-runtime/smithy-http/src/middleware.rs

@@ -16,6 +16,43 @@ use bytes::{Buf, Bytes};
 use http_body::Body;
 use std::error::Error;
 
+/// Body for debugging purposes
+///
+/// When receiving data from the AWS services, it is often helpful to be able to see the response
+/// body that a service generated. When the SDK has fully buffered the body into memory, this
+/// facilitates straightforward debugging of the response.
+///
+/// Take care when calling the debug implementation to avoid printing responses from sensitive operations.
+#[derive(Debug)]
+pub struct ResponseBody(Inner);
+
+impl ResponseBody {
+    /// Load a response body from a static string
+    pub fn from_static(s: &'static str) -> Self {
+        ResponseBody(Inner::Bytes(Bytes::from_static(s.as_bytes())))
+    }
+
+    /// Returns the raw bytes of this response
+    ///
+    /// When the response has been buffered into memory, the bytes are returned
+    /// If the response is streaming or errored during the read process, `None` is returned.
+    pub fn bytes(&self) -> Option<&[u8]> {
+        match &self.0 {
+            Inner::Bytes(bytes) => Some(&bytes),
+            _ => None
+        }
+    }
+}
+
+/// Private ResponseBody internals
+#[derive(Debug)]
+enum Inner {
+    Bytes(bytes::Bytes),
+    Streaming,
+    Err,
+}
+
+
 type BoxError = Box<dyn Error + Send + Sync>;
 
 /// [`MapRequest`] defines a synchronous middleware that transforms an [`operation::Request`].
@@ -72,22 +109,21 @@ pub trait MapRequest {
 pub async fn load_response<B, T, E, O>(
     mut response: http::Response<B>,
     handler: &O,
-) -> Result<SdkSuccess<T, B>, SdkError<E, B>>
+) -> Result<SdkSuccess<T>, SdkError<E>>
 where
     B: http_body::Body + Unpin,
-    B: From<Bytes> + 'static,
     B::Error: Into<BoxError>,
     O: ParseHttpResponse<B, Output = Result<T, E>>,
 {
     if let Some(parsed_response) = handler.parse_unloaded(&mut response) {
-        return sdk_result(parsed_response, response);
+        return sdk_result(parsed_response, response.map(|_|ResponseBody(Inner::Streaming)));
     }
 
     let body = match read_body(response.body_mut()).await {
         Ok(body) => body,
         Err(e) => {
             return Err(SdkError::ResponseError {
-                raw: response,
+                raw: response.map(|_|ResponseBody(Inner::Err)),
                 err: e.into(),
             });
         }
@@ -95,7 +131,7 @@ where
 
     let response = response.map(|_| Bytes::from(body));
     let parsed = handler.parse_loaded(&response);
-    sdk_result(parsed, response.map(B::from))
+    sdk_result(parsed, response.map(|body|ResponseBody(Inner::Bytes(body))))
 }
 
 async fn read_body<B: http_body::Body>(body: B) -> Result<Vec<u8>, B::Error> {
@@ -112,10 +148,10 @@ async fn read_body<B: http_body::Body>(body: B) -> Result<Vec<u8>, B::Error> {
 }
 
 /// Convert a `Result<T, E>` into an `SdkResult` that includes the raw HTTP response
-fn sdk_result<T, E, B>(
+fn sdk_result<T, E>(
     parsed: Result<T, E>,
-    raw: http::Response<B>,
-) -> Result<SdkSuccess<T, B>, SdkError<E, B>> {
+    raw: http::Response<ResponseBody>,
+) -> Result<SdkSuccess<T>, SdkError<E>> {
     match parsed {
         Ok(parsed) => Ok(SdkSuccess { raw, parsed }),
         Err(err) => Err(SdkError::ServiceError { raw, err }),

rust-runtime/smithy-http/src/result.rs

@@ -6,37 +6,19 @@
 use std::error::Error;
 use std::fmt;
 use std::fmt::{Debug, Display, Formatter};
+use crate::middleware::ResponseBody;
 
 type BoxError = Box<dyn Error + Send + Sync>;
-
 /// Successful Sdk Result
-///
-/// Typically, transport implementations will type alias (or entirely wrap / transform) this type
-/// plugging in a concrete body implementation, eg:
-/// ```rust
-/// # mod hyper {
-/// #    pub struct Body;
-/// # }
-/// type SdkSuccess<O> = smithy_http::result::SdkSuccess<O, hyper::Body>;
-/// ```
 #[derive(Debug)]
-pub struct SdkSuccess<O, B> {
-    pub raw: http::Response<B>,
+pub struct SdkSuccess<O> {
+    pub raw: http::Response<ResponseBody>,
     pub parsed: O,
 }
 
 /// Failing Sdk Result
-///
-/// Typically, transport implementations will type alias (or entirely wrap / transform) this type
-/// by specifying a concrete body implementation:
-/// ```rust
-/// # mod hyper {
-/// #    pub struct Body;
-/// # }
-/// type SdkError<E> = smithy_http::result::SdkError<E, hyper::Body>;
-/// ```
 #[derive(Debug)]
-pub enum SdkError<E, B> {
+pub enum SdkError<E> {
     /// The request failed during construction. It was not dispatched over the network.
     ConstructionFailure(BoxError),
 
@@ -47,28 +29,26 @@ pub enum SdkError<E, B> {
     /// A response was received but it was not parseable according the the protocol (for example
     /// the server hung up while the body was being read)
     ResponseError {
-        raw: http::Response<B>,
+        raw: http::Response<ResponseBody>,
         err: BoxError,
     },
 
     /// An error response was received from the service
-    ServiceError { err: E, raw: http::Response<B> },
+    ServiceError { err: E, raw: http::Response<ResponseBody> },
 }
 
-impl<E, B> Display for SdkError<E, B>
+impl<E> Display for SdkError<E>
 where
     E: Error,
-    B: Debug,
 {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         write!(f, "{:?}", self)
     }
 }
 
-impl<E, B> Error for SdkError<E, B>
+impl<E> Error for SdkError<E>
 where
     E: Error + 'static,
-    B: Debug,
 {
     fn source(&self) -> Option<&(dyn Error + 'static)> {
         match self {