From 4def74b4ee41428a2e1fb9ffb9ff1ce574c0e3fe Mon Sep 17 00:00:00 2001 From: EdJoPaTo Date: Tue, 10 Sep 2024 22:20:01 +0200 Subject: [PATCH 1/2] refactor!: merge Error and HttpError (#204) Also, `From for Error` is removed as it could result in accidental wrong error kinds when the `?` operator is used. BREAKING CHANGE: Error::Http changed and HttpError struct is gone. --- examples/api_trait_implementation.rs | 59 +++++++++------------------- src/api.rs | 11 +----- src/api/async_telegram_api_impl.rs | 18 ++------- src/api/telegram_api_impl.rs | 11 +++--- 4 files changed, 29 insertions(+), 70 deletions(-) diff --git a/examples/api_trait_implementation.rs b/examples/api_trait_implementation.rs index 853f4f8..699fdfe 100644 --- a/examples/api_trait_implementation.rs +++ b/examples/api_trait_implementation.rs @@ -14,14 +14,8 @@ pub struct Api { #[derive(Debug)] pub enum Error { - HttpError(HttpError), - ApiError(ErrorResponse), -} - -#[derive(Debug)] -pub struct HttpError { - pub code: u16, - pub message: String, + Http { code: u16, message: String }, + Api(ErrorResponse), } impl Api { @@ -40,24 +34,14 @@ impl Api { impl From for Error { fn from(error: isahc::http::Error) -> Self { let message = format!("{error:?}"); - let error = HttpError { code: 500, message }; - Self::HttpError(error) - } -} - -impl From for Error { - fn from(error: std::io::Error) -> Self { - let message = format!("{error:?}"); - let error = HttpError { code: 500, message }; - Self::HttpError(error) + Self::Http { code: 500, message } } } impl From for Error { fn from(error: isahc::Error) -> Self { let message = format!("{error:?}"); - let error = HttpError { code: 500, message }; - Self::HttpError(error) + Self::Http { code: 500, message } } } @@ -81,21 +65,18 @@ impl TelegramApi for Api { } }; - let text = response.text()?; - - let parsed_result: Result = serde_json::from_str(&text); - - parsed_result.map_err(|_| { - let parsed_error: Result = - serde_json::from_str(&text); - - match parsed_error { - Ok(result) => Error::ApiError(result), - Err(error) => { - let message = format!("{error:?}"); - let error = HttpError { code: 500, message }; - Error::HttpError(error) - } + let text = response.text().map_err(|error| Error::Http { + code: 500, + message: format!("{error:?}"), + })?; + + serde_json::from_str(&text).map_err(|_| { + match serde_json::from_str::(&text) { + Ok(result) => Error::Api(result), + Err(error) => Error::Http { + code: 500, + message: format!("{error:?}"), + }, } }) } @@ -108,12 +89,8 @@ impl TelegramApi for Api { _params: T1, _files: Vec<(&str, PathBuf)>, ) -> Result { - let error = HttpError { - code: 500, - message: "isahc doesn't support form data requests".to_string(), - }; - - Err(Error::HttpError(error)) + let message = "isahc doesn't support form data requests".to_string(); + Err(Error::Http { code: 500, message }) } } diff --git a/src/api.rs b/src/api.rs index 90f2c05..4503a34 100644 --- a/src/api.rs +++ b/src/api.rs @@ -16,8 +16,8 @@ pub static BASE_API_URL: &str = "https://api.telegram.org/bot"; #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, thiserror::Error)] #[serde(untagged)] pub enum Error { - #[error("{0}")] - Http(HttpError), + #[error("Http Error {code}: {message}")] + Http { code: u16, message: String }, #[error("Api Error {0:?}")] Api(ErrorResponse), #[error("Decode Error {0}")] @@ -25,10 +25,3 @@ pub enum Error { #[error("Encode Error {0}")] Encode(String), } - -#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, thiserror::Error)] -#[error("Http Error {code}: {message}")] -pub struct HttpError { - pub code: u16, - pub message: String, -} diff --git a/src/api/async_telegram_api_impl.rs b/src/api/async_telegram_api_impl.rs index 501df7f..d45a6d9 100644 --- a/src/api/async_telegram_api_impl.rs +++ b/src/api/async_telegram_api_impl.rs @@ -1,5 +1,4 @@ use super::Error; -use super::HttpError; use crate::api_traits::AsyncTelegramApi; use crate::api_traits::ErrorResponse; use async_trait::async_trait; @@ -79,17 +78,7 @@ impl From for Error { let code = error .status() .map_or(500, |status_code| status_code.as_u16()); - - let error = HttpError { code, message }; - Self::Http(error) - } -} - -impl From for Error { - fn from(error: std::io::Error) -> Self { - let message = error.to_string(); - - Self::Encode(message) + Self::Http { code, message } } } @@ -162,8 +151,9 @@ impl AsyncTelegramApi for AsyncApi { } for (parameter_name, file_path, file_name) in files_with_paths { - let file = File::open(file_path).await?; - + let file = File::open(file_path) + .await + .map_err(|err| Error::Encode(err.to_string()))?; let part = multipart::Part::stream(file).file_name(file_name); form = form.part(parameter_name, part); } diff --git a/src/api/telegram_api_impl.rs b/src/api/telegram_api_impl.rs index a1c2151..0935871 100644 --- a/src/api/telegram_api_impl.rs +++ b/src/api/telegram_api_impl.rs @@ -1,5 +1,4 @@ use super::Error; -use super::HttpError; use crate::api_traits::TelegramApi; use multipart::client::lazy::Multipart; use serde_json::Value; @@ -61,17 +60,17 @@ impl From for Error { ureq::Error::Status(code, response) => match response.into_string() { Ok(message) => match serde_json::from_str(&message) { Ok(json_result) => Self::Api(json_result), - Err(_) => Self::Http(HttpError { code, message }), + Err(_) => Self::Http { code, message }, }, - Err(_) => Self::Http(HttpError { + Err(_) => Self::Http { code, message: "Failed to decode response".to_string(), - }), + }, }, - ureq::Error::Transport(transport_error) => Self::Http(HttpError { + ureq::Error::Transport(transport_error) => Self::Http { message: format!("{transport_error:?}"), code: 500, - }), + }, } } } From 060cba161147beeb3574cf42985ce5011c2443b4 Mon Sep 17 00:00:00 2001 From: EdJoPaTo Date: Tue, 10 Sep 2024 22:35:48 +0200 Subject: [PATCH 2/2] refactor: improve generic argument naming (#205) * refactor: improve generic argument naming * refactor: simplify request impl --- examples/api_trait_implementation.rs | 20 +++++-- src/api/async_telegram_api_impl.rs | 89 +++++++++++----------------- src/api/telegram_api_impl.rs | 66 ++++++++------------- src/api_traits/async_telegram_api.rs | 52 ++++++++-------- src/api_traits/telegram_api.rs | 49 ++++++++------- 5 files changed, 126 insertions(+), 150 deletions(-) diff --git a/examples/api_trait_implementation.rs b/examples/api_trait_implementation.rs index 699fdfe..68dacb6 100644 --- a/examples/api_trait_implementation.rs +++ b/examples/api_trait_implementation.rs @@ -48,11 +48,15 @@ impl From for Error { impl TelegramApi for Api { type Error = Error; - fn request( + fn request( &self, method: &str, - params: Option, - ) -> Result { + params: Option, + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + Output: serde::de::DeserializeOwned, + { let url = format!("{}/{method}", self.api_url); let request_builder = Request::post(url).header("Content-Type", "application/json"); @@ -83,12 +87,16 @@ impl TelegramApi for Api { // isahc doesn't support multipart uploads // https://github.com/sagebind/isahc/issues/14 - fn request_with_form_data( + fn request_with_form_data( &self, _method: &str, - _params: T1, + _params: Params, _files: Vec<(&str, PathBuf)>, - ) -> Result { + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + Output: serde::de::DeserializeOwned, + { let message = "isahc doesn't support form data requests".to_string(); Err(Error::Http { code: 500, message }) } diff --git a/src/api/async_telegram_api_impl.rs b/src/api/async_telegram_api_impl.rs index d45a6d9..9d83075 100644 --- a/src/api/async_telegram_api_impl.rs +++ b/src/api/async_telegram_api_impl.rs @@ -1,6 +1,5 @@ use super::Error; use crate::api_traits::AsyncTelegramApi; -use crate::api_traits::ErrorResponse; use async_trait::async_trait; use reqwest::multipart; use serde_json::Value; @@ -27,48 +26,36 @@ impl AsyncApi { } /// Create a new `AsyncApi`. You can use [`AsyncApi::builder`] for more options. - pub fn new_url>(api_url: T) -> Self { + pub fn new_url>(api_url: S) -> Self { Self::builder().api_url(api_url).build() } - pub fn encode_params( - params: &T, - ) -> Result { + pub fn encode_params(params: &Params) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + { serde_json::to_string(params).map_err(|e| Error::Encode(format!("{e:?} : {params:?}"))) } - pub async fn decode_response( - response: reqwest::Response, - ) -> Result { + pub async fn decode_response(response: reqwest::Response) -> Result + where + Output: serde::de::DeserializeOwned, + { let status_code = response.status().as_u16(); match response.text().await { Ok(message) => { if status_code == 200 { - let success_response: T = Self::parse_json(&message)?; - return Ok(success_response); + Ok(Self::parse_json(&message)?) + } else { + Err(Error::Api(Self::parse_json(&message)?)) } - - let error_response: ErrorResponse = Self::parse_json(&message)?; - Err(Error::Api(error_response)) - } - Err(e) => { - let err = Error::Decode(format!("Failed to decode response: {e:?}")); - Err(err) } + Err(e) => Err(Error::Decode(format!("Failed to decode response: {e:?}"))), } } - fn parse_json(body: &str) -> Result { - let json_result: Result = serde_json::from_str(body); - - match json_result { - Ok(result) => Ok(result), - - Err(e) => { - let err = Error::Decode(format!("{e:?} : {body:?}")); - Err(err) - } - } + fn parse_json(body: &str) -> Result { + serde_json::from_str(body).map_err(|e| Error::Decode(format!("{e:?} : {body:?}"))) } } @@ -86,44 +73,38 @@ impl From for Error { impl AsyncTelegramApi for AsyncApi { type Error = Error; - async fn request< - T1: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, - T2: serde::de::DeserializeOwned, - >( + async fn request( &self, method: &str, - params: Option, - ) -> Result { + params: Option, + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, + Output: serde::de::DeserializeOwned, + { let url = format!("{}/{method}", self.api_url); - let mut prepared_request = self .client .post(url) .header("Content-Type", "application/json"); - - prepared_request = if let Some(data) = params { - let json_string = Self::encode_params(&data)?; - - prepared_request.body(json_string) - } else { - prepared_request + if let Some(params) = params { + let json_string = Self::encode_params(¶ms)?; + prepared_request = prepared_request.body(json_string); }; - let response = prepared_request.send().await?; - let parsed_response: T2 = Self::decode_response(response).await?; - - Ok(parsed_response) + Self::decode_response(response).await } - async fn request_with_form_data< - T1: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, - T2: serde::de::DeserializeOwned, - >( + async fn request_with_form_data( &self, method: &str, - params: T1, + params: Params, files: Vec<(&str, PathBuf)>, - ) -> Result { + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, + Output: serde::de::DeserializeOwned, + { let json_string = Self::encode_params(¶ms)?; let json_struct: Value = serde_json::from_str(&json_string).unwrap(); let file_keys: Vec<&str> = files.iter().map(|(key, _)| *key).collect(); @@ -161,9 +142,7 @@ impl AsyncTelegramApi for AsyncApi { let url = format!("{}/{method}", self.api_url); let response = self.client.post(url).multipart(form).send().await?; - let parsed_response: T2 = Self::decode_response(response).await?; - - Ok(parsed_response) + Self::decode_response(response).await } } diff --git a/src/api/telegram_api_impl.rs b/src/api/telegram_api_impl.rs index 0935871..ca63d4d 100644 --- a/src/api/telegram_api_impl.rs +++ b/src/api/telegram_api_impl.rs @@ -23,33 +23,25 @@ impl Api { } /// Create a new `Api`. You can use [`Api::builder`] for more options. - pub fn new_url>(api_url: T) -> Self { + pub fn new_url>(api_url: S) -> Self { Self::builder().api_url(api_url).build() } - pub fn encode_params( - params: &T, - ) -> Result { + pub fn encode_params(params: &Params) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + { serde_json::to_string(params).map_err(|e| Error::Encode(format!("{e:?} : {params:?}"))) } - pub fn decode_response(response: Response) -> Result { + pub fn decode_response(response: Response) -> Result + where + Output: serde::de::DeserializeOwned, + { match response.into_string() { - Ok(message) => { - let json_result: Result = serde_json::from_str(&message); - - match json_result { - Ok(result) => Ok(result), - Err(e) => { - let err = Error::Decode(format!("{e:?} : {message:?}")); - Err(err) - } - } - } - Err(e) => { - let err = Error::Decode(format!("Failed to decode response: {e:?}")); - Err(err) - } + Ok(message) => serde_json::from_str(&message) + .map_err(|error| Error::Decode(format!("{error:?} : {message:?}"))), + Err(e) => Err(Error::Decode(format!("Failed to decode response: {e:?}"))), } } } @@ -78,17 +70,16 @@ impl From for Error { impl TelegramApi for Api { type Error = Error; - fn request( - &self, - method: &str, - params: Option, - ) -> Result { + fn request(&self, method: &str, params: Option) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + Output: serde::de::DeserializeOwned, + { let url = format!("{}/{method}", self.api_url); let prepared_request = self .request_agent .post(&url) .set("Content-Type", "application/json"); - let response = match params { None => prepared_request.call()?, Some(data) => { @@ -96,21 +87,19 @@ impl TelegramApi for Api { prepared_request.send_string(&json)? } }; - - let parsed_response: T2 = Self::decode_response(response)?; - - Ok(parsed_response) + Self::decode_response(response) } - fn request_with_form_data< - T1: serde::ser::Serialize + std::fmt::Debug, - T2: serde::de::DeserializeOwned, - >( + fn request_with_form_data( &self, method: &str, - params: T1, + params: Params, files: Vec<(&str, PathBuf)>, - ) -> Result { + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + Output: serde::de::DeserializeOwned, + { let json_string = Self::encode_params(¶ms)?; let json_struct: Value = serde_json::from_str(&json_string).unwrap(); let file_keys: Vec<&str> = files.iter().map(|(key, _)| *key).collect(); @@ -152,10 +141,7 @@ impl TelegramApi for Api { &format!("multipart/form-data; boundary={}", form_data.boundary()), ) .send(form_data)?; - - let parsed_response: T2 = Self::decode_response(response)?; - - Ok(parsed_response) + Self::decode_response(response) } } diff --git a/src/api_traits/async_telegram_api.rs b/src/api_traits/async_telegram_api.rs index f72e1db..3bc745e 100644 --- a/src/api_traits/async_telegram_api.rs +++ b/src/api_traits/async_telegram_api.rs @@ -361,7 +361,7 @@ pub trait AsyncTelegramApi { let mut new_params = params.clone(); new_params.media = new_medias; - let files_with_str_names: Vec<(&str, PathBuf)> = files + let files_with_str_names = files .iter() .map(|(key, path)| (key.as_str(), path.clone())) .collect(); @@ -1069,7 +1069,7 @@ pub trait AsyncTelegramApi { let mut new_params = params.clone(); new_params.media = new_media; - let files_with_str_names: Vec<(&str, PathBuf)> = files + let files_with_str_names = files .iter() .map(|(key, path)| (key.as_str(), path.clone())) .collect(); @@ -1170,7 +1170,7 @@ pub trait AsyncTelegramApi { let mut new_params = params.clone(); new_params.stickers = new_stickers; - let files_with_str_names: Vec<(&str, PathBuf)> = files + let files_with_str_names = files .iter() .map(|(key, path)| (key.as_str(), path.clone())) .collect(); @@ -1387,33 +1387,33 @@ pub trait AsyncTelegramApi { .await } - async fn request_without_body( - &self, - method: &str, - ) -> Result { + async fn request_without_body(&self, method: &str) -> Result + where + Output: serde::de::DeserializeOwned, + { let params: Option<()> = None; - self.request(method, params).await } - async fn request< - T1: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, - T2: serde::de::DeserializeOwned, - >( + async fn request( &self, method: &str, - params: Option, - ) -> Result; + params: Option, + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, + Output: serde::de::DeserializeOwned; - async fn request_with_possible_form_data< - T1: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, - T2: serde::de::DeserializeOwned, - >( + async fn request_with_possible_form_data( &self, method_name: &str, - params: T1, + params: Params, files: Vec<(&str, PathBuf)>, - ) -> Result { + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, + Output: serde::de::DeserializeOwned, + { if files.is_empty() { self.request(method_name, Some(params)).await } else { @@ -1422,13 +1422,13 @@ pub trait AsyncTelegramApi { } } - async fn request_with_form_data< - T1: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, - T2: serde::de::DeserializeOwned, - >( + async fn request_with_form_data( &self, method: &str, - params: T1, + params: Params, files: Vec<(&str, PathBuf)>, - ) -> Result; + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug + std::marker::Send, + Output: serde::de::DeserializeOwned; } diff --git a/src/api_traits/telegram_api.rs b/src/api_traits/telegram_api.rs index 5da9d19..38dc7cc 100644 --- a/src/api_traits/telegram_api.rs +++ b/src/api_traits/telegram_api.rs @@ -348,7 +348,7 @@ pub trait TelegramApi { let mut new_params = params.clone(); new_params.media = new_medias; - let files_with_str_names: Vec<(&str, PathBuf)> = files + let files_with_str_names = files .iter() .map(|(key, path)| (key.as_str(), path.clone())) .collect(); @@ -1019,7 +1019,7 @@ pub trait TelegramApi { let mut new_params = params.clone(); new_params.media = new_media; - let files_with_str_names: Vec<(&str, PathBuf)> = files + let files_with_str_names = files .iter() .map(|(key, path)| (key.as_str(), path.clone())) .collect(); @@ -1114,7 +1114,7 @@ pub trait TelegramApi { let mut new_params = params.clone(); new_params.stickers = new_stickers; - let files_with_str_names: Vec<(&str, PathBuf)> = files + let files_with_str_names = files .iter() .map(|(key, path)| (key.as_str(), path.clone())) .collect(); @@ -1322,24 +1322,24 @@ pub trait TelegramApi { self.request("unpinAllGeneralForumTopicMessages", Some(params)) } - fn request_without_body( - &self, - method: &str, - ) -> Result { + fn request_without_body(&self, method: &str) -> Result + where + Output: serde::de::DeserializeOwned, + { let params: Option<()> = None; - self.request(method, params) } - fn request_with_possible_form_data< - T1: serde::ser::Serialize + std::fmt::Debug, - T2: serde::de::DeserializeOwned, - >( + fn request_with_possible_form_data( &self, method_name: &str, - params: &T1, + params: &Params, files: Vec<(&str, PathBuf)>, - ) -> Result { + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + Output: serde::de::DeserializeOwned, + { if files.is_empty() { self.request(method_name, Some(params)) } else { @@ -1347,19 +1347,22 @@ pub trait TelegramApi { } } - fn request_with_form_data< - T1: serde::ser::Serialize + std::fmt::Debug, - T2: serde::de::DeserializeOwned, - >( + fn request_with_form_data( &self, method: &str, - params: T1, + params: Params, files: Vec<(&str, PathBuf)>, - ) -> Result; + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + Output: serde::de::DeserializeOwned; - fn request( + fn request( &self, method: &str, - params: Option, - ) -> Result; + params: Option, + ) -> Result + where + Params: serde::ser::Serialize + std::fmt::Debug, + Output: serde::de::DeserializeOwned; }