Include Response bytes when payload didn't deserialize as expected (#655)

docs/progenitor-client.md

@@ -63,21 +63,24 @@ It can be used as the type `T` in most instances and extracted as a `T` using
 
 ## `Error<E>`
 
-There are five sub-categories of error covered by the error type variants:
+There are seven sub-categories of error covered by the error type variants:
 
-- A request that did not conform to API requirements. This can occur when
+- A request that did not conform to API requirements.
-  required builder or body parameters were not specified, and the error message
+  This can occur when required builder or body parameters were not specified,
-  will denote the specific failure.
+  and the error message will denote the specific failure.
 
 - A communication error
 
-- An expected error response, defined by the OpenAPI document with a 4xx or 5xx
+- An expected error response when upgrading connection.
-  status code
 
-- An expected status code (whose payload didn't deserialize as expected (this
+- An expected error response, defined by the OpenAPI document
-  could be viewed as a sub-type of communication error), but it is separately
+  with a 4xx or 5xx status code
-  identified as there's more information; note that this covers both success and
+
-  error status codes
+- An expected status code that encountered an error reading the body
+  or the payload deserialization failed
+  (this could be viewed as a sub-type of communication error),
+  but it is separately identified as there's more information;
+  note that this covers both success and error status codes
 
 - An unexpected status code in the response
 
@@ -87,8 +90,10 @@ These errors are covered by the variants of the `Error<E>` type:
 pub enum Error<E = ()> {
     InvalidRequest(String),
     CommunicationError(reqwest::Error),
+    InvalidUpgrade(reqwest::Error),
     ErrorResponse(ResponseValue<E>),
-    InvalidResponsePayload(reqwest::Error),
+    ResponseBodyError(reqwest::Error),
+    InvalidResponsePayload(bytes::Bytes, reqwest::Error),
     UnexpectedResponse(reqwest::Response),
 }
 ```

progenitor-client/src/progenitor_client.rs

@@ -68,10 +68,9 @@ impl<T: DeserializeOwned> ResponseValue<T> {
     ) -> Result<Self, Error<E>> {
         let status = response.status();
         let headers = response.headers().clone();
-        let inner = response
+        let full = response.bytes().await.map_err(Error::ResponseBodyError)?;
-            .json()
+        let inner = serde_json::from_slice(&full)
-            .await
+            .map_err(|e| Error::InvalidResponsePayload(full, e))?;
-            .map_err(Error::InvalidResponsePayload)?;
 
         Ok(Self {
             inner,
@@ -90,10 +89,8 @@ impl ResponseValue<reqwest::Upgraded> {
         let status = response.status();
         let headers = response.headers().clone();
         if status == reqwest::StatusCode::SWITCHING_PROTOCOLS {
-            let inner = response
+            let inner =
-                .upgrade()
+                response.upgrade().await.map_err(Error::InvalidUpgrade)?;
-                .await
-                .map_err(Error::InvalidResponsePayload)?;
 
             Ok(Self {
                 inner,
@@ -237,12 +234,17 @@ pub enum Error<E = ()> {
     /// A server error either due to the data, or with the connection.
     CommunicationError(reqwest::Error),
 
+    /// An expected response when upgrading connection.
+    InvalidUpgrade(reqwest::Error),
+
     /// A documented, expected error response.
     ErrorResponse(ResponseValue<E>),
 
+    /// Encountered an error reading the body for an expected response.
+    ResponseBodyError(reqwest::Error),
+
     /// An expected response code whose deserialization failed.
-    // TODO we have stuff from the response; should we include it?
+    InvalidResponsePayload(Bytes, serde_json::Error),
-    InvalidResponsePayload(reqwest::Error),
 
     /// A response not listed in the API description. This may represent a
     /// success or failure response; check `status().is_success()`.
@@ -256,7 +258,9 @@ impl<E> Error<E> {
             Error::InvalidRequest(_) => None,
             Error::CommunicationError(e) => e.status(),
             Error::ErrorResponse(rv) => Some(rv.status()),
-            Error::InvalidResponsePayload(e) => e.status(),
+            Error::InvalidUpgrade(e) => e.status(),
+            Error::ResponseBodyError(e) => e.status(),
+            Error::InvalidResponsePayload(_, _) => None,
             Error::UnexpectedResponse(r) => Some(r.status()),
         }
     }
@@ -278,8 +282,10 @@ impl<E> Error<E> {
                 status,
                 headers,
             }),
-            Error::InvalidResponsePayload(e) => {
+            Error::InvalidUpgrade(e) => Error::InvalidUpgrade(e),
-                Error::InvalidResponsePayload(e)
+            Error::ResponseBodyError(e) => Error::ResponseBodyError(e),
+            Error::InvalidResponsePayload(b, e) => {
+                Error::InvalidResponsePayload(b, e)
             }
             Error::UnexpectedResponse(r) => Error::UnexpectedResponse(r),
         }
@@ -314,8 +320,14 @@ where
                 write!(f, "Error Response: ")?;
                 rve.fmt_info(f)
             }
-            Error::InvalidResponsePayload(e) => {
+            Error::InvalidUpgrade(e) => {
-                write!(f, "Invalid Response Payload: {}", e)
+                write!(f, "Invalid Response Upgrade: {}", e)
+            }
+            Error::ResponseBodyError(e) => {
+                write!(f, "Invalid Response Body Bytes: {}", e)
+            }
+            Error::InvalidResponsePayload(b, e) => {
+                write!(f, "Invalid Response Payload ({:?}): {}", b, e)
             }
             Error::UnexpectedResponse(r) => {
                 write!(f, "Unexpected Response: {:?}", r)
@@ -366,7 +378,9 @@ where
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
             Error::CommunicationError(e) => Some(e),
-            Error::InvalidResponsePayload(e) => Some(e),
+            Error::InvalidUpgrade(e) => Some(e),
+            Error::ResponseBodyError(e) => Some(e),
+            Error::InvalidResponsePayload(_b, e) => Some(e),
             _ => None,
         }
     }