diff --git a/implementations/rust/ockam/ockam_api/src/influxdb/lease_issuer/worker.rs b/implementations/rust/ockam/ockam_api/src/influxdb/lease_issuer/worker.rs index c52fd91be63..6f635b107ea 100644 --- a/implementations/rust/ockam/ockam_api/src/influxdb/lease_issuer/worker.rs +++ b/implementations/rust/ockam/ockam_api/src/influxdb/lease_issuer/worker.rs @@ -255,8 +255,7 @@ impl InfluxDBTokenLessorWorkerApi for InfluxDBTokenLessorWorker { let is_authorized_to_revoke = self .get_token(requester, token_id) .await? - .into_parts() - .1 + .get_body() .is_some(); if !is_authorized_to_revoke { return Err(Response::unauthorized_no_request( diff --git a/implementations/rust/ockam/ockam_core/src/api.rs b/implementations/rust/ockam/ockam_core/src/api.rs index 8d8b9031844..8818d621deb 100644 --- a/implementations/rust/ockam/ockam_core/src/api.rs +++ b/implementations/rust/ockam/ockam_core/src/api.rs @@ -654,8 +654,7 @@ impl Message for Request {} #[derive(Debug, Clone, PartialEq, Eq)] pub struct Response { header: ResponseHeader, - body: Option, - error: Option, + body: Option>, } impl Response { @@ -679,8 +678,8 @@ impl Response { &self.header } - pub fn into_parts(self) -> (ResponseHeader, Option, Option) { - (self.header, self.body, self.error) + pub fn into_parts(self) -> (ResponseHeader, Option>) { + (self.header, self.body) } /// Convenient wrapper to append the requests header to the response @@ -696,6 +695,14 @@ impl Response { pub fn has_body(&self) -> bool { self.header.has_body() } + + pub fn get_body(self) -> Option { + self.body.and_then(|b| b.ok()) + } + + pub fn get_error(self) -> Option { + self.body.and_then(|b| b.err()) + } } impl Response { @@ -706,9 +713,20 @@ impl Response { let mut e = Encoder::new(buf); e.encode(&self.header)?; if let Some(b) = self.body { - e.writer_mut() - .write_all(&::encode(b).map_err(encode::Error::message)?) - .map_err(|_| encode::Error::message("encoding error"))?; + match b { + Ok(t) => { + e.writer_mut() + .write_all(&::encode(t).map_err(encode::Error::message)?) + .map_err(|_| encode::Error::message("encoding error"))?; + } + Err(error) => { + e.writer_mut() + .write_all( + &::encode(error).map_err(encode::Error::message)?, + ) + .map_err(|_| encode::Error::message("encoding error"))?; + } + } } Ok(()) } @@ -720,19 +738,20 @@ impl Response { } pub fn encode_body(self) -> Result>> { - let (header, body, error_message) = self.into_parts(); + let (header, body) = self.into_parts(); let r = if let Some(b) = body { - Response { - header, - body: Some(::encode(b)?), - error: error_message, + match b { + Ok(t) => Response { + header, + body: Some(Ok(::encode(t)?)), + }, + Err(e) => Response { + header, + body: Some(Err(e)), + }, } } else { - Response { - header, - body: None, - error: error_message, - } + Response { header, body: None } }; Ok(r) } @@ -767,15 +786,10 @@ impl Decodable for Response { })?; Ok(Response { header, - body: Some(::decode(body)?), - error: None, + body: Some(Ok(::decode(body)?)), }) } else { - Ok(Response { - header, - body: None, - error: None, - }) + Ok(Response { header, body: None }) } } else { let error = if matches!(dec.datatype(), Ok(Type::String)) { @@ -787,8 +801,7 @@ impl Decodable for Response { Ok(Response { header, - body: None, - error: Some(error), + body: Some(Err(error)), }) } } @@ -798,26 +811,25 @@ impl Message for Response {} impl Response> { pub fn to_reply(self) -> Result> { - let (header, body, error) = self.into_parts(); - if header.is_ok() { - if let Some(body) = body { - match T::decode(body.as_slice()) { + let (header, body) = self.into_parts(); + if let Some(body) = body { + match body { + Ok(t) => match T::decode(t.as_slice()) { Err(e) => Err(crate::Error::new( Origin::Api, Kind::Serialization, format!("{e:?}"), )), Ok(r) => Ok(Reply::Successful(r)), - } - } else { - Err(crate::Error::new( - Origin::Api, - Kind::Serialization, - "expected a message body, got nothing".to_string(), - )) + }, + Err(error) => Ok(Reply::Failed(error, header.status())), } - } else if let Some(error) = error { - Ok(Reply::Failed(error, header.status())) + } else if header.is_ok() { + Err(crate::Error::new( + Origin::Api, + Kind::Serialization, + "expected a message body, got nothing".to_string(), + )) } else { Err(crate::Error::new( Origin::Api, @@ -844,8 +856,7 @@ impl Response<()> { pub fn body(self, b: T) -> Response { let mut b = Response { header: self.header, - body: Some(b), - error: None, + body: Some(Ok(b)), }; b.header.has_body = true; b @@ -858,7 +869,6 @@ impl Response { Response { header: ResponseHeader::new(re, status, false), body: None, - error: None, } } @@ -958,15 +968,24 @@ impl Response { impl Response> { pub fn parse_error(self) -> Result> { let status = self.header().status; - let body = self.body.unwrap_or_default(); - let error = if let Ok(msg) = ::decode(&body) { - Ok(Error::new_without_path().with_message(msg)) - } else { - ::decode(&body) - }; - match error { - Ok(e) => Ok(Reply::Failed(e, status)), - Err(e) => Err(crate::Error::new(Origin::Api, Kind::Serialization, e)), + match self.body { + None => Err(crate::Error::new( + Origin::Api, + Kind::Serialization, + "missing body", + )), + Some(Ok(b)) => { + let error = if let Ok(msg) = ::decode(&b) { + Ok(Error::new_without_path().with_message(msg)) + } else { + ::decode(&b) + }; + match error { + Ok(e) => Ok(Reply::Failed(e, status)), + Err(e) => Err(crate::Error::new(Origin::Api, Kind::Serialization, e)), + } + } + Some(Err(error)) => Ok(Reply::Failed(error, status)), } } @@ -977,26 +996,38 @@ impl Response> { let has_body = self.has_body(); match status { - Some(status) if has_body => { - let body = self.body.unwrap_or_default(); - let error = if let Ok(msg) = ::decode(&body) { - Ok(format!("Message: {msg}")) - } else { - ::decode(&body).map(|msg| format!("Message: {msg}")) - }; - match error { - Ok(err) => { - format!( + Some(status) if has_body => match self.body { + None => { + format!( + "No error message could be found in the response. Status code: {status}" + ) + } + Some(Ok(b)) => { + let error = if let Ok(msg) = ::decode(&b) { + Ok(format!("Message: {msg}")) + } else { + ::decode(&b).map(|msg| format!("Message: {msg}")) + }; + match error { + Ok(err) => { + format!( + "An error occurred while processing the request. Status code: {status}. {err}" + ) + } + Err(err) => { + let msg = + format!("No error message could be read from the response: {err}"); + error!(msg); + msg + } + } + } + Some(Err(err)) => { + format!( "An error occurred while processing the request. Status code: {status}. {err}" ) - } - Err(err) => { - let msg = format!("No error message could be found in response: {err}"); - error!(msg); - msg - } } - } + }, Some(status) => { format!("An error occurred while processing the request. Status code: {status}") } @@ -1088,7 +1119,7 @@ mod tests { let request = Request::post("path").body(person.clone()); let decoded: Request> = Request::decode(&Request::encode(request.clone()).unwrap()).unwrap(); - let decoded_person = ::decode(&decoded.body.unwrap_or_default()).ok(); + let decoded_person = ::decode(&decoded.body.unwrap()).ok(); assert_eq!(decoded_person, Some(person)); } @@ -1123,7 +1154,7 @@ mod tests { let request = Response::ok().body(person.clone()); let decoded: Response> = Response::decode(&Response::encode(request.clone()).unwrap()).unwrap(); - let decoded_person = ::decode(&decoded.body.unwrap_or_default()).ok(); + let decoded_person = ::decode(&decoded.body.unwrap().unwrap()).ok(); assert_eq!(decoded_person, Some(person)); } diff --git a/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs b/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs index 8ff20eda5fa..9085bd8012b 100644 --- a/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs +++ b/implementations/rust/ockam/ockam_identity/src/secure_channels/secure_client.rs @@ -189,8 +189,12 @@ impl SecureClient { Ok(Successful(())) } else { let status = response.header().status(); + // The "missing error" case should not happen because we would have failed deserialization + // in the case of a ko response with no error body. Ok(Reply::Failed( - Error::from_failed_request(&request_header, &response.parse_err_msg()), + response + .get_error() + .unwrap_or(Error::from_failed_request(&request_header, "missing error")), status, )) }