From 89f8d0f656a1fd02598055de740b7aba11777007 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sat, 8 Feb 2020 21:44:02 +0100 Subject: [PATCH] Further split the error types --- CHANGELOG.md | 6 +- ruma-api-macros/src/api.rs | 88 ++++++++--- ruma-api-macros/src/api/request.rs | 19 ++- ruma-api-macros/src/api/response.rs | 2 +- src/error.rs | 219 ++++++++++++++++++++++++++++ src/lib.rs | 116 ++++----------- 6 files changed, 333 insertions(+), 117 deletions(-) create mode 100644 src/error.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 87e1fe9f..6088473e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,9 @@ Breaking changes: -* Instead of one `Error` type, there is now two: `FromHttpError` and `IntoHttpError` - * In constrast to the old `Error` types, these can be pattern matched and generally allow getting - much more context for why things failed +* Instead of one `Error` type, there is now many + * The new types live in their own `error` module + * They provide access to details that were previously hidden * Out Minimum Supported Rust Version is now 1.40.0 # 0.12.1 diff --git a/ruma-api-macros/src/api.rs b/ruma-api-macros/src/api.rs index bbef4690..a7a42c88 100644 --- a/ruma-api-macros/src/api.rs +++ b/ruma-api-macros/src/api.rs @@ -154,12 +154,20 @@ impl ToTokens for Api { quote! { #path_var_ident: { use std::ops::Deref as _; + use ruma_api::error::RequestDeserializationError; let segment = path_segments.get(#i).unwrap().as_bytes(); let decoded = ruma_api::exports::percent_encoding::percent_decode(segment) .decode_utf8_lossy(); - ruma_api::exports::serde_json::from_str(decoded.deref())? + match ruma_api::exports::serde_json::from_str(decoded.deref()) { + Ok(val) => val, + Err(err) => { + return Err( + RequestDeserializationError::new(err, request).into() + ); + } + } } } }, @@ -230,13 +238,31 @@ impl ToTokens for Api { let extract_request_query = if self.request.query_map_field().is_some() { quote! { - let request_query = - ruma_api::exports::serde_urlencoded::from_str(&request.uri().query().unwrap_or(""))?; + let request_query = match ruma_api::exports::serde_urlencoded::from_str( + &request.uri().query().unwrap_or("") + ) { + Ok(query) => query, + Err(err) => { + return Err( + ruma_api::error::RequestDeserializationError::new(err, request).into() + ); + } + }; } } else if self.request.has_query_fields() { quote! { let request_query: RequestQuery = - ruma_api::exports::serde_urlencoded::from_str(&request.uri().query().unwrap_or(""))?; + match ruma_api::exports::serde_urlencoded::from_str( + &request.uri().query().unwrap_or("") + ) { + Ok(query) => query, + Err(err) => { + return Err( + ruma_api::error::RequestDeserializationError::new(err, request) + .into() + ); + } + }; } } else { TokenStream::new() @@ -274,7 +300,15 @@ impl ToTokens for Api { if self.request.has_body_fields() || self.request.newtype_body_field().is_some() { quote! { let request_body: ::Incoming = - ruma_api::exports::serde_json::from_slice(request.body().as_slice())?; + match ruma_api::exports::serde_json::from_slice(request.body().as_slice()) { + Ok(body) => body, + Err(err) => { + return Err( + ruma_api::error::RequestDeserializationError::new(err, request) + .into() + ); + } + }; } } else { TokenStream::new() @@ -325,21 +359,30 @@ impl ToTokens for Api { let extract_response_headers = if self.response.has_header_fields() { quote! { - let mut headers = http_response.headers().clone(); + let mut headers = response.headers().clone(); } } else { TokenStream::new() }; - let typed_response_body_decl = - if self.response.has_body_fields() || self.response.newtype_body_field().is_some() { - quote! { - let response_body: ::Incoming = - ruma_api::exports::serde_json::from_slice(response_body.as_slice())?; - } - } else { - TokenStream::new() - }; + let typed_response_body_decl = if self.response.has_body_fields() + || self.response.newtype_body_field().is_some() + { + quote! { + let response_body: ::Incoming = + match ruma_api::exports::serde_json::from_slice(response.body().as_slice()) { + Ok(body) => body, + Err(err) => { + return Err( + ruma_api::error::ResponseDeserializationError::new(err, response) + .into() + ); + } + }; + } + } else { + TokenStream::new() + }; let response_init_fields = self.response.init_fields(); @@ -365,7 +408,7 @@ impl ToTokens for Api { #request_type impl std::convert::TryFrom>> for #request_try_from_type { - type Error = ruma_api::FromHttpError; + type Error = ruma_api::error::FromHttpRequestError; #[allow(unused_variables)] fn try_from(request: ruma_api::exports::http::Request>) -> Result { @@ -384,7 +427,7 @@ impl ToTokens for Api { } impl std::convert::TryFrom for ruma_api::exports::http::Request> { - type Error = ruma_api::IntoHttpError; + type Error = ruma_api::error::IntoHttpError; #[allow(unused_mut, unused_variables)] fn try_from(request: Request) -> Result { @@ -415,7 +458,7 @@ impl ToTokens for Api { #response_type impl std::convert::TryFrom for ruma_api::exports::http::Response> { - type Error = ruma_api::IntoHttpError; + type Error = ruma_api::error::IntoHttpError; #[allow(unused_variables)] fn try_from(response: Response) -> Result { @@ -429,23 +472,22 @@ impl ToTokens for Api { } impl std::convert::TryFrom>> for #response_try_from_type { - type Error = ruma_api::FromHttpError; + type Error = ruma_api::error::FromHttpResponseError; #[allow(unused_variables)] fn try_from( - http_response: ruma_api::exports::http::Response>, + response: ruma_api::exports::http::Response>, ) -> Result { - if http_response.status().is_success() { + if response.status().as_u16() < 400 { #extract_response_headers - let response_body = http_response.into_body(); #typed_response_body_decl Ok(Self { #response_init_fields }) } else { - Err(ruma_api::FromHttpError::Http(http_response)) + Err(ruma_api::error::ServerError::new(response).into()) } } } diff --git a/ruma-api-macros/src/api/request.rs b/ruma-api-macros/src/api/request.rs index 0f4e4f19..cb5e57f0 100644 --- a/ruma-api-macros/src/api/request.rs +++ b/ruma-api-macros/src/api/request.rs @@ -54,10 +54,21 @@ impl Request { let header_name_string = header_name.to_string(); quote! { - #field_name: headers.get(ruma_api::exports::http::header::#header_name) - .and_then(|v| v.to_str().ok()) - .ok_or(ruma_api::exports::serde_json::Error::missing_field(#header_name_string))? - .to_owned() + #field_name: match headers.get(ruma_api::exports::http::header::#header_name) + .and_then(|v| v.to_str().ok()) { + Some(header) => header.to_owned(), + None => { + return Err( + ruma_api::error::RequestDeserializationError::new( + ruma_api::exports::serde_json::Error::missing_field( + #header_name_string + ), + request, + ) + .into() + ); + } + } } }); diff --git a/ruma-api-macros/src/api/response.rs b/ruma-api-macros/src/api/response.rs index c289b2a9..4b2cf89d 100644 --- a/ruma-api-macros/src/api/response.rs +++ b/ruma-api-macros/src/api/response.rs @@ -62,7 +62,7 @@ impl Response { } ResponseField::NewtypeRawBody(_) => { quote_spanned! {span=> - #field_name: response_body + #field_name: response.into_body() } } } diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 00000000..364f36a0 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,219 @@ +//! This module contains types for all kinds of errors that can occur when +//! converting between http requests / responses and ruma's representation of +//! matrix API requests / responses. + +use std::fmt::{self, Display, Formatter}; + +/// An error when converting one of ruma's endpoint-specific request or response +/// types to the corresponding http type. +#[derive(Debug)] +pub struct IntoHttpError(SerializationError); + +#[doc(hidden)] +impl From for IntoHttpError { + fn from(err: serde_json::Error) -> Self { + Self(SerializationError::Json(err)) + } +} + +#[doc(hidden)] +impl From for IntoHttpError { + fn from(err: serde_urlencoded::ser::Error) -> Self { + Self(SerializationError::Query(err)) + } +} + +impl Display for IntoHttpError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match &self.0 { + SerializationError::Json(err) => write!(f, "JSON serialization failed: {}", err), + SerializationError::Query(err) => { + write!(f, "Query parameter serialization failed: {}", err) + } + } + } +} + +impl std::error::Error for IntoHttpError {} + +/// An error when converting a http request to one of ruma's endpoint-specific +/// request types. +#[derive(Debug)] +#[non_exhaustive] +pub enum FromHttpRequestError { + /// Deserialization failed + Deserialization(RequestDeserializationError), +} + +impl Display for FromHttpRequestError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + Self::Deserialization(err) => write!(f, "deserialization failed: {}", err), + } + } +} + +impl From for FromHttpRequestError { + fn from(err: RequestDeserializationError) -> Self { + Self::Deserialization(err) + } +} + +impl std::error::Error for FromHttpRequestError {} + +/// An error that occurred when trying to deserialize a request. +#[derive(Debug)] +pub struct RequestDeserializationError { + inner: DeserializationError, + http_request: http::Request>, +} + +impl RequestDeserializationError { + /// This method is public so it is accessible from `ruma_api!` generated + /// code. It is not considered part of ruma-api's public API. + #[doc(hidden)] + pub fn new( + inner: impl Into, + http_request: http::Request>, + ) -> Self { + Self { inner: inner.into(), http_request } + } +} + +impl Display for RequestDeserializationError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Display::fmt(&self.inner, f) + } +} + +impl std::error::Error for RequestDeserializationError {} + +/// An error when converting a http response to one of ruma's endpoint-specific +/// response types. +#[derive(Debug)] +#[non_exhaustive] +pub enum FromHttpResponseError { + /// Deserialization failed + Deserialization(ResponseDeserializationError), + /// The server returned a non-success status + Http(ServerError), +} + +impl Display for FromHttpResponseError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + Self::Deserialization(err) => write!(f, "deserialization failed: {}", err), + Self::Http(err) => write!(f, "the server returned an error: {}", err), + } + } +} + +impl From for FromHttpResponseError { + fn from(err: ServerError) -> Self { + Self::Http(err) + } +} + +impl From for FromHttpResponseError { + fn from(err: ResponseDeserializationError) -> Self { + Self::Deserialization(err) + } +} + +/// An error that occurred when trying to deserialize a response. +#[derive(Debug)] +pub struct ResponseDeserializationError { + inner: DeserializationError, + http_response: http::Response>, +} + +impl ResponseDeserializationError { + /// This method is public so it is accessible from `ruma_api!` generated + /// code. It is not considered part of ruma-api's public API. + #[doc(hidden)] + pub fn new( + inner: impl Into, + http_response: http::Response>, + ) -> Self { + Self { inner: inner.into(), http_response } + } +} + +impl Display for ResponseDeserializationError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Display::fmt(&self.inner, f) + } +} + +impl std::error::Error for ResponseDeserializationError {} + +/// An error was reported by the server (HTTP status code 4xx or 5xx) +#[derive(Debug)] +pub struct ServerError { + http_response: http::Response>, +} + +impl ServerError { + /// This method is public so it is accessible from `ruma_api!` generated + /// code. It is not considered part of ruma-api's public API. + #[doc(hidden)] + pub fn new(http_response: http::Response>) -> Self { + Self { http_response } + } + + /// Get the HTTP response without parsing its contents. + pub fn into_raw_reponse(self) -> http::Response> { + self.http_response + } +} + +impl Display for ServerError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self.http_response.status().canonical_reason() { + Some(reason) => { + write!(f, "HTTP status {} {}", self.http_response.status().as_str(), reason) + } + None => write!(f, "HTTP status {}", self.http_response.status().as_str()), + } + } +} + +impl std::error::Error for ServerError {} + +#[derive(Debug)] +enum SerializationError { + Json(serde_json::Error), + Query(serde_urlencoded::ser::Error), +} + +/// This type is public so it is accessible from `ruma_api!` generated code. +/// It is not considered part of ruma-api's public API. +#[doc(hidden)] +#[derive(Debug)] +pub enum DeserializationError { + Json(serde_json::Error), + Query(serde_urlencoded::de::Error), +} + +impl Display for DeserializationError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + DeserializationError::Json(err) => Display::fmt(err, f), + DeserializationError::Query(err) => Display::fmt(err, f), + } + } +} + +#[doc(hidden)] +impl From for DeserializationError { + fn from(err: serde_json::Error) -> Self { + Self::Json(err) + } +} + +#[doc(hidden)] +impl From for DeserializationError { + fn from(err: serde_urlencoded::de::Error) -> Self { + Self::Query(err) + } +} diff --git a/src/lib.rs b/src/lib.rs index e2089c27..48f68eb7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,11 +13,7 @@ #![warn(rust_2018_idioms)] #![deny(missing_copy_implementations, missing_debug_implementations, missing_docs)] -use std::{ - convert::{TryFrom, TryInto}, - error::Error as StdError, - fmt::{Display, Formatter, Result as FmtResult}, -}; +use std::convert::{TryFrom, TryInto}; use http::Method; @@ -208,6 +204,7 @@ pub use ruma_api_macros::ruma_api; #[cfg(feature = "with-ruma-api-macros")] pub use ruma_api_macros::Outgoing; +pub mod error; /// This module is used to support the generated code from ruma-api-macros. /// It is not considered part of ruma-api's public API. #[cfg(feature = "with-ruma-api-macros")] @@ -221,6 +218,8 @@ pub mod exports { pub use url; } +use error::{FromHttpRequestError, FromHttpResponseError, IntoHttpError}; + /// A type that can be sent to another party that understands the matrix protocol. If any of the /// fields of `Self` don't implement serde's `Deserialize`, you can derive this trait to generate a /// corresponding 'Incoming' type that supports deserialization. This is useful for things like @@ -238,8 +237,9 @@ pub trait Outgoing { /// The type implementing this trait contains any data needed to make a request to the endpoint. pub trait Endpoint: Outgoing + TryInto>, Error = IntoHttpError> where - ::Incoming: TryFrom>, Error = FromHttpError>, - ::Incoming: TryFrom>, Error = FromHttpError>, + ::Incoming: TryFrom>, Error = FromHttpRequestError>, + ::Incoming: + TryFrom>, Error = FromHttpResponseError>, { /// Data returned in a successful response from the endpoint. type Response: Outgoing + TryInto>, Error = IntoHttpError>; @@ -248,78 +248,6 @@ where const METADATA: Metadata; } -/// An error when converting an `http` request or response to the corresponding -#[derive(Debug)] -#[non_exhaustive] -pub enum FromHttpError { - /// The server returned a non-success status - Http(http::Response>), - /// JSON deserialization failed - Json(serde_json::Error), - /// Deserialization of query parameters failed - Query(serde_urlencoded::de::Error), -} - -/// An error when converting a request or response to the corresponding http type. -#[derive(Debug)] -#[non_exhaustive] -pub enum IntoHttpError { - /// JSON serialization failed - Json(serde_json::Error), - /// Serialization of query parameters failed - Query(serde_urlencoded::ser::Error), -} - -impl From for FromHttpError { - fn from(err: serde_json::Error) -> Self { - Self::Json(err) - } -} - -impl From for FromHttpError { - fn from(err: serde_urlencoded::de::Error) -> Self { - Self::Query(err) - } -} - -impl Display for FromHttpError { - fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { - match self { - Self::Http(res) => match res.status().canonical_reason() { - Some(reason) => write!(f, "HTTP status {} {}", res.status().as_str(), reason), - None => write!(f, "HTTP status {}", res.status().as_str()), - }, - Self::Json(err) => write!(f, "JSON deserialization failed: {}", err), - Self::Query(err) => write!(f, "Query parameter deserialization failed: {}", err), - } - } -} - -impl StdError for FromHttpError {} - -impl From for IntoHttpError { - fn from(err: serde_json::Error) -> Self { - Self::Json(err) - } -} - -impl From for IntoHttpError { - fn from(err: serde_urlencoded::ser::Error) -> Self { - Self::Query(err) - } -} - -impl Display for IntoHttpError { - fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { - match self { - Self::Json(err) => write!(f, "JSON serialization failed: {}", err), - Self::Query(err) => write!(f, "Query parameter serialization failed: {}", err), - } - } -} - -impl StdError for IntoHttpError {} - /// Metadata about an API endpoint. #[derive(Clone, Debug)] pub struct Metadata { @@ -353,7 +281,13 @@ mod tests { use ruma_identifiers::{RoomAliasId, RoomId}; use serde::{Deserialize, Serialize}; - use crate::{Endpoint, FromHttpError, IntoHttpError, Metadata, Outgoing}; + use crate::{ + error::{ + FromHttpRequestError, FromHttpResponseError, IntoHttpError, + RequestDeserializationError, ServerError, + }, + Endpoint, Metadata, Outgoing, + }; /// A request to create a new room alias. #[derive(Debug)] @@ -403,18 +337,28 @@ mod tests { } impl TryFrom>> for Request { - type Error = FromHttpError; + type Error = FromHttpRequestError; fn try_from(request: http::Request>) -> Result { let request_body: RequestBody = - ::serde_json::from_slice(request.body().as_slice())?; + match serde_json::from_slice(request.body().as_slice()) { + Ok(body) => body, + Err(err) => { + return Err(RequestDeserializationError::new(err, request).into()); + } + }; let path_segments: Vec<&str> = request.uri().path()[1..].split('/').collect(); Ok(Request { room_id: request_body.room_id, room_alias: { let segment = path_segments.get(5).unwrap().as_bytes(); let decoded = percent_encoding::percent_decode(segment).decode_utf8_lossy(); - serde_json::from_str(decoded.deref())? + match serde_json::from_str(decoded.deref()) { + Ok(id) => id, + Err(err) => { + return Err(RequestDeserializationError::new(err, request).into()) + } + } }, }) } @@ -434,13 +378,13 @@ mod tests { } impl TryFrom>> for Response { - type Error = FromHttpError; + type Error = FromHttpResponseError; fn try_from(http_response: http::Response>) -> Result { - if http_response.status().is_success() { + if http_response.status().as_u16() < 400 { Ok(Response) } else { - Err(FromHttpError::Http(http_response)) + Err(FromHttpResponseError::Http(ServerError::new(http_response))) } } }