From 8b6b91d6baf32d46798751ab9b2a5a41de52a762 Mon Sep 17 00:00:00 2001 From: Everett Pompeii Date: Fri, 29 Dec 2023 12:27:08 -0500 Subject: [PATCH] Include Response bytes when payload didn't deserialize as expected (#655) --- docs/progenitor-client.md | 27 +++++++------ progenitor-client/src/progenitor_client.rs | 46 ++++++++++++++-------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/docs/progenitor-client.md b/docs/progenitor-client.md index d0e90cc..b1ae469 100644 --- a/docs/progenitor-client.md +++ b/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` -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 - required builder or body parameters were not specified, and the error message - will denote the specific failure. +- A request that did not conform to API requirements. + This can occur when required builder or body parameters were not specified, + 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 - status code +- An expected error response when upgrading connection. -- An expected status code (whose payload didn't deserialize as expected (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 expected error response, defined by the OpenAPI document + with a 4xx or 5xx status code + +- 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` type: pub enum Error { InvalidRequest(String), CommunicationError(reqwest::Error), + InvalidUpgrade(reqwest::Error), ErrorResponse(ResponseValue), - InvalidResponsePayload(reqwest::Error), + ResponseBodyError(reqwest::Error), + InvalidResponsePayload(bytes::Bytes, reqwest::Error), UnexpectedResponse(reqwest::Response), } ``` diff --git a/progenitor-client/src/progenitor_client.rs b/progenitor-client/src/progenitor_client.rs index a412f7b..742f8a6 100644 --- a/progenitor-client/src/progenitor_client.rs +++ b/progenitor-client/src/progenitor_client.rs @@ -68,10 +68,9 @@ impl ResponseValue { ) -> Result> { let status = response.status(); let headers = response.headers().clone(); - let inner = response - .json() - .await - .map_err(Error::InvalidResponsePayload)?; + let full = response.bytes().await.map_err(Error::ResponseBodyError)?; + let inner = serde_json::from_slice(&full) + .map_err(|e| Error::InvalidResponsePayload(full, e))?; Ok(Self { inner, @@ -90,10 +89,8 @@ impl ResponseValue { let status = response.status(); let headers = response.headers().clone(); if status == reqwest::StatusCode::SWITCHING_PROTOCOLS { - let inner = response - .upgrade() - .await - .map_err(Error::InvalidResponsePayload)?; + let inner = + response.upgrade().await.map_err(Error::InvalidUpgrade)?; Ok(Self { inner, @@ -237,12 +234,17 @@ pub enum Error { /// 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), + /// 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(reqwest::Error), + InvalidResponsePayload(Bytes, serde_json::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 Error { 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 Error { status, headers, }), - Error::InvalidResponsePayload(e) => { - Error::InvalidResponsePayload(e) + Error::InvalidUpgrade(e) => Error::InvalidUpgrade(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) => { - write!(f, "Invalid Response Payload: {}", e) + Error::InvalidUpgrade(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, } }