From c49f40128d85db563f71ac155b35fb85a10cf001 Mon Sep 17 00:00:00 2001 From: Koen Bolhuis Date: Wed, 1 Sep 2021 18:04:56 +0200 Subject: [PATCH] Use the attohttpc crate instead of ureq. This has some implications for Error: - because attohttpc doesn't distinguish between errors when serializing or deserializing request/response data, Error::RequestJson and Error::ResponseJson have been merged into Error::Json. In practice this shouldn't matter much because request data is (usually) well-formed. - attohttpc also doesn't return errors for HTTP responses with statuses in the [400-599] range, so manual conversion to an error is needed (this now happens in ResponseType::from_response). --- Cargo.toml | 2 +- src/error.rs | 45 ++++++++++++++---------- src/raw/client.rs | 86 ++++++++++++++++++++------------------------- src/raw/response.rs | 21 +++++++---- 4 files changed, 79 insertions(+), 75 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 81dbcee..8c7cc93 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,4 +18,4 @@ publish = true thiserror = "1" serde = { version = "1", features = ["derive"] } serde_json = "1" -ureq = { version = "2", features = ["json"] } +attohttpc = { version = "0.17", features = ["json"] } diff --git a/src/error.rs b/src/error.rs index 2bec6c0..fab9ec8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,8 +1,8 @@ -use std::io; - use serde::Deserialize; -/// Represents errors that can occor while interacting with the API. +use attohttpc::Response; + +/// Represents errors that can occur while interacting with the API. #[derive(Debug, thiserror::Error)] pub enum Error { /// The API returned a non-200 status code. @@ -15,17 +15,13 @@ pub enum Error { error: String, }, - /// The input data could not be converted into JSON. - #[error("could not convert request input into JSON")] - RequestJson(#[source] serde_json::Error), - - /// The HTTP response could not be converted into JSON. - #[error("could not convert HTTP response into JSON")] - ResponseJson(#[source] io::Error), + /// The request or response data could not be converted into or from JSON. + #[error("could not convert request or response data into or from JSON")] + Json(#[source] attohttpc::Error), /// There was some other HTTP error while interacting with the API. #[error("HTTP error")] - Http(#[source] Box), + Http(#[source] attohttpc::Error), /// The token that was attempted to be used for authentication is invalid. #[error("invalid authentication token")] @@ -36,6 +32,20 @@ pub enum Error { NotAuthenticated, } +impl Error { + /// If the response is a client or server error (status [400-599]), + /// deserialize it into `Error::Api`. Otherwise, return the original response. + pub(crate) fn try_from_error_response(response: Response) -> Result { + let status = response.status(); + if status.is_client_error() || status.is_server_error() { + let api_error: ApiError = response.json()?; + Err(api_error.into()) + } else { + Ok(response) + } + } +} + #[derive(Debug, Deserialize)] struct ApiError { code: u16, @@ -51,14 +61,11 @@ impl From for Error { } } -impl From for Error { - fn from(error: ureq::Error) -> Self { - match error { - ureq::Error::Status(_code, response) => match response.into_json::() { - Ok(api_error) => api_error.into(), - Err(err) => Error::ResponseJson(err), - }, - ureq::Error::Transport(_) => Error::Http(Box::new(error)), +impl From for Error { + fn from(error: attohttpc::Error) -> Self { + match error.kind() { + attohttpc::ErrorKind::Json(_) => Self::Json(error), + _ => Self::Http(error), } } } diff --git a/src/raw/client.rs b/src/raw/client.rs index 2f21f04..2f81d87 100644 --- a/src/raw/client.rs +++ b/src/raw/client.rs @@ -1,7 +1,5 @@ use serde::Serialize; -use ureq::Agent; - use super::endpoint::Endpoint; use super::request::*; use super::response::*; @@ -18,11 +16,9 @@ const API_ROOT_URL: &str = "https://api.listenbrainz.org/1/"; /// /// Client's methods can return the following errors: /// - [`Error::Api`]: the API returned a non-`2XX` status. -/// - [`Error::RequestJson`]: the request data could not be converted into JSON. -/// - [`Error::ResponseJson`]: the response data could not be converted into JSON. +/// - [`Error::Json`]: the request or response data could not be converted from or into JSON. /// - [`Error::Http`]: there was some other HTTP error while interacting with the API. pub struct Client { - agent: Agent, api_root_url: String, } @@ -30,14 +26,12 @@ impl Client { /// Construct a new client. pub fn new() -> Self { Self { - agent: ureq::agent(), api_root_url: API_ROOT_URL.to_string(), } } pub fn new_with_url(url: &str) -> Self { Self { - agent: ureq::agent(), api_root_url: url.to_string(), } } @@ -47,7 +41,7 @@ impl Client { fn get(&self, endpoint: Endpoint) -> Result { let endpoint = format!("{}{}", self.api_root_url, endpoint); - let response = self.agent.get(&endpoint).call()?; + let response = attohttpc::get(endpoint).send()?; R::from_response(response) } @@ -63,19 +57,19 @@ impl Client { ) -> Result, Error> { let endpoint = format!("{}{}", self.api_root_url, endpoint); - let mut request = self.agent.get(&endpoint); + let mut request = attohttpc::get(endpoint); if let Some(count) = count { - request = request.query("count", &count.to_string()); + request = request.param("count", count); } if let Some(offset) = offset { - request = request.query("offset", &offset.to_string()); + request = request.param("offset", offset); } if let Some(range) = range { - request = request.query("range", range); + request = request.param("range", range); } - let response = request.call()?; + let response = request.send()?; // API returns 204 and an empty document if there are no statistics if response.status() == 204 { @@ -92,14 +86,12 @@ impl Client { D: Serialize, R: ResponseType, { - let data = serde_json::to_value(data).map_err(Error::RequestJson)?; - let endpoint = format!("{}{}", self.api_root_url, endpoint); - let response = self.agent - .post(&endpoint) - .set("Authorization", &format!("Token {}", token)) - .send_json(data)?; + let response = attohttpc::post(endpoint) + .header("Authorization", &format!("Token {}", token)) + .json(&data)? + .send()?; R::from_response(response) } @@ -117,10 +109,9 @@ impl Client { pub fn validate_token(&self, token: &str) -> Result { let endpoint = format!("{}{}", self.api_root_url, Endpoint::ValidateToken); - let response = self.agent - .get(&endpoint) - .query("token", token) - .call()?; + let response = attohttpc::get(endpoint) + .param("token", token) + .send()?; ResponseType::from_response(response) } @@ -163,22 +154,22 @@ impl Client { ) -> Result { let endpoint = format!("{}{}", self.api_root_url, Endpoint::UserListens(user_name)); - let mut request = self.agent.get(&endpoint); + let mut request = attohttpc::get(endpoint); if let Some(min_ts) = min_ts { - request = request.query("min_ts", &min_ts.to_string()); + request = request.param("min_ts", min_ts); } if let Some(max_ts) = max_ts { - request = request.query("max_ts", &max_ts.to_string()); + request = request.param("max_ts", max_ts); } if let Some(count) = count { - request = request.query("count", &count.to_string()); + request = request.param("count", count); } if let Some(time_range) = time_range { - request = request.query("time_range", &time_range.to_string()); + request = request.param("time_range", time_range); } - let response = request.call()?; + let response = request.send()?; ResponseType::from_response(response) } @@ -187,12 +178,11 @@ impl Client { pub fn get_latest_import(&self, user_name: &str) -> Result { let endpoint = format!("{}{}", self.api_root_url, Endpoint::LatestImport); - self.agent - .get(&endpoint) - .query("user_name", user_name) - .call()? - .into_json() - .map_err(Error::ResponseJson) + let response = attohttpc::get(endpoint) + .param("user_name", user_name) + .send()?; + + ResponseType::from_response(response) } /// Endpoint: [`latest-import`](https://listenbrainz.readthedocs.io/en/production/dev/api/#post--1-latest-import) (`POST`) @@ -226,13 +216,13 @@ impl Client { Endpoint::StatsUserListeningActivity(user_name) ); - let mut request = self.agent.get(&endpoint); + let mut request = attohttpc::get(endpoint); if let Some(range) = range { - request = request.query("range", range); + request = request.param("range", range); } - let response = request.call()?; + let response = request.send()?; // API returns 204 and an empty document if there are no statistics if response.status() == 204 { @@ -254,13 +244,13 @@ impl Client { Endpoint::StatsUserDailyActivity(user_name) ); - let mut request = self.agent.get(&endpoint); + let mut request = attohttpc::get(endpoint); if let Some(range) = range { - request = request.query("range", range); + request = request.param("range", range); } - let response = request.call()?; + let response = request.send()?; // API returns 204 and an empty document if there are no statistics if response.status() == 204 { @@ -299,16 +289,16 @@ impl Client { Endpoint::StatsUserArtistMap(user_name) ); - let mut request = self.agent.get(&endpoint); + let mut request = attohttpc::get(endpoint); if let Some(range) = range { - request = request.query("range", range); + request = request.param("range", range); } if let Some(force_recalculate) = force_recalculate { - request = request.query("force_recalculate", &force_recalculate.to_string()); + request = request.param("force_recalculate", force_recalculate); } - let response = request.call()?; + let response = request.send()?; ResponseType::from_response(response) } @@ -342,13 +332,13 @@ impl Client { ) -> Result { let endpoint = format!("{}{}", self.api_root_url, Endpoint::StatusGetDumpInfo); - let mut request = self.agent.get(&endpoint); + let mut request = attohttpc::get(endpoint); if let Some(id) = id { - request = request.query("id", &id.to_string()); + request = request.param("id", id); } - let response = request.call()?; + let response = request.send()?; ResponseType::from_response(response) } diff --git a/src/raw/response.rs b/src/raw/response.rs index 73778f3..90aa07c 100644 --- a/src/raw/response.rs +++ b/src/raw/response.rs @@ -8,7 +8,7 @@ use std::collections::HashMap; use serde::de::DeserializeOwned; use serde::Deserialize; -use ureq::Response; +use attohttpc::Response; use crate::Error; @@ -31,13 +31,19 @@ impl RateLimit { /// Extract rate limiting information from the `X-RateLimit-` headers. /// Only returns `Some` if all fields are present and valid. fn from_headers(response: &Response) -> Option { - let limit = response.header("X-RateLimit-Limit")? + let headers = response.headers(); + + let limit = headers.get("X-RateLimit-Limit")? + .to_str().ok()? .parse().ok()?; - let remaining = response.header("X-RateLimit-Remaining")? + let remaining = headers.get("X-RateLimit-Remaining")? + .to_str().ok()? .parse().ok()?; - let reset_in = response.header("X-RateLimit-Reset-In")? + let reset_in = headers.get("X-RateLimit-Reset-In")? + .to_str().ok()? .parse().ok()?; - let reset = response.header("X-RateLimit-Reset")? + let reset = headers.get("X-RateLimit-Reset")? + .to_str().ok()? .parse().ok()?; Some(Self { @@ -50,7 +56,7 @@ impl RateLimit { } /// Internal trait for response types. -/// Allows converting the response type from a `ureq::Response`, +/// Allows converting the response type from an `attohttpc::Response`, /// by deserializing the body into the response type and then /// adding the `rate_limit` field from headers. pub(crate) trait ResponseType: DeserializeOwned { @@ -77,8 +83,9 @@ macro_rules! response_type { impl ResponseType for $name { fn from_response(response: Response) -> Result { + let response = Error::try_from_error_response(response)?; let rate_limit = RateLimit::from_headers(&response); - let mut result: Self = response.into_json().map_err(Error::ResponseJson)?; + let mut result: Self = response.json()?; result.rate_limit = rate_limit; Ok(result) }