From a025e452c0d348d6324bc14ec4a06e697e0aeebc Mon Sep 17 00:00:00 2001 From: Nick Dowsett Date: Thu, 11 Jan 2024 21:27:05 +0800 Subject: [PATCH] Improved documentation of Error type and made internal functions private --- Cargo.lock | 2 +- ytmapi-rs/src/auth/browser.rs | 5 +- ytmapi-rs/src/error.rs | 165 +++++++++++++++++++--------------- ytmapi-rs/src/parse.rs | 7 +- 4 files changed, 102 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e01320dc..7f409453 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4188,7 +4188,7 @@ dependencies = [ [[package]] name = "ytmapi-rs" -version = "0.0.1" +version = "0.0.2" dependencies = [ "async-stream", "chrono", diff --git a/ytmapi-rs/src/auth/browser.rs b/ytmapi-rs/src/auth/browser.rs index ca3590dd..594e5a70 100644 --- a/ytmapi-rs/src/auth/browser.rs +++ b/ytmapi-rs/src/auth/browser.rs @@ -94,17 +94,18 @@ impl AuthToken for BrowserToken { impl BrowserToken { pub async fn from_str(cookie_str: &str, client: &Client) -> Result { let cookies = cookie_str.trim().to_string(); + let user_agent = USER_AGENT; let response = client .get(YTM_URL) .header(reqwest::header::COOKIE, &cookies) - .header(reqwest::header::USER_AGENT, USER_AGENT) + .header(reqwest::header::USER_AGENT, user_agent) .send() .await? .text() .await?; // parse for user agent issues here. if response.contains("Sorry, YouTube Music is not optimised for your browser. Check for updates or try Google Chrome.") { - return Err(Error::other("Expired User Agent")); + return Err(Error::invalid_user_agent(user_agent)); }; // TODO: Better error. let client_version = response diff --git a/ytmapi-rs/src/error.rs b/ytmapi-rs/src/error.rs index 117a35c6..3684e317 100644 --- a/ytmapi-rs/src/error.rs +++ b/ytmapi-rs/src/error.rs @@ -1,5 +1,4 @@ //! Module to contain code related to errors that could be produced by the API. -//! This module aims to wrap any non-std crates to avoid leaking dependencies. use core::fmt::{Debug, Display}; use std::{io, sync::Arc}; @@ -9,170 +8,196 @@ pub type Result = core::result::Result; /// This type represents all errors this API could produce. pub struct Error { // This is boxed to avoid passing around very large errors - in the case of an Api error we want to provide the source file to the caller. - inner: Box, + inner: Box, } -enum Inner { - // Wrapper for reqwest::Error currently +/// The kind of the error. +pub enum ErrorKind { + /// General web error. + // TODO: improve and avoid leaking reqwest::Error Web(reqwest::Error), - // Wrapper for std::io::Error currently + /// General io error. + // TODO: improve Io(io::Error), // Api was not in the expected format for the library (e.g, expected an array). // TODO: Add query type to error. + /// Field of the JSON file was not in the expected format. Parsing { - // The target path (JSON pointer notation) that we tried to parse. + /// The target path (JSON pointer notation) that we tried to parse. key: String, - // The source json from Innertube that we were trying to parse. + /// The source json from Innertube that we were trying to parse. // NOTE: API could theoretically produce multiple errors referring to the same source json. // Hence reference counted, Arc particularly to ensure Error is thread safe. json: Arc, + /// The format we were trying to parse into. target: ParseTarget, }, - // Expected key did not occur in the JSON file. + /// Expected key did not occur in the JSON file. Navigation { - // The target path (JSON pointer notation) that we tried to parse. + /// The target path (JSON pointer notation) that we tried to parse. key: String, - // The source json from Innertube. + /// The source json from Innertube. // NOTE: API could theoretically produce multiple errors referring to the same source json. // Hence reference counted, Arc particularly to ensure Error is thread safe. json: Arc, }, - // TODO: Add more detail. - // Guessing this means we got an invalid response from Innertube. - // Currently looks to be getting returned when we fail to deserialize the JSON initially. + /// Received a response from InnerTube that was not in the expected (JSON) format. InvalidResponse { response: String, }, - // XXX: Seems to get returned when Innertube Browser Authentication Response doesn't contain the required fields. + /// InnerTube credential header not in expected format. Header, Other(String), // Generic catchall - TODO: Remove all of these. UnableToSerializeGoogleOAuthToken { response: String, err: serde_json::Error, }, + /// InnerTube rejected the User Agent we are using. + InvalidUserAgent(String), + /// Failed to authenticate using Browse Auth credentials (may have expired, or been incorrectly provided). BrowserAuthenticationFailed, + /// OAuthToken has expired. OAuthTokenExpired, - // Received an error code in the JSON message from Innertube. // This is a u64 not a usize as that is what serde_json will deserialize to. // TODO: Could use a library to handle these. + /// Recieved an error code in the Json reply from InnerTube. OtherErrorCodeInResponse(u64), } +/// The type we were attempting to pass from the Json. #[derive(Debug, Clone)] pub enum ParseTarget { Array, String, + Enum, } impl Error { - pub fn oauth_token_expired() -> Self { - Self { - inner: Box::new(Inner::OAuthTokenExpired), + /// Extract the inner kind from the error for pattern matching. + pub fn into_kind(self) -> ErrorKind { + *self.inner + } + // Only used for tests currently. + pub(crate) fn is_oauth_expired(&self) -> bool { + if let ErrorKind::OAuthTokenExpired = *self.inner { + true + } else { + false } } - pub fn is_oauth_expired(&self) -> bool { - if let Inner::OAuthTokenExpired = *self.inner { + // Only used for tests currently. + pub(crate) fn is_browser_authentication_failed(&self) -> bool { + if let ErrorKind::BrowserAuthenticationFailed = *self.inner { true } else { false } } - pub fn browser_authentication_failed() -> Self { + /// If an error is a Navigation or Parsing error, return the source Json and key at the location of the error. + pub fn get_json_and_key(&self) -> Option<(String, &String)> { + match self.inner.as_ref() { + ErrorKind::Navigation { json, key } => Some((json.to_string(), &key)), + ErrorKind::Parsing { json, key, .. } => Some((json.to_string(), &key)), + ErrorKind::Web(_) + | ErrorKind::Io(_) + | ErrorKind::InvalidResponse { .. } + | ErrorKind::Header + | ErrorKind::Other(_) + | ErrorKind::UnableToSerializeGoogleOAuthToken { .. } + | ErrorKind::OtherErrorCodeInResponse(_) + | ErrorKind::OAuthTokenExpired + | ErrorKind::BrowserAuthenticationFailed + | ErrorKind::InvalidUserAgent(_) => None, + } + } + pub(crate) fn invalid_user_agent>(user_agent: S) -> Self { Self { - inner: Box::new(Inner::BrowserAuthenticationFailed), + inner: Box::new(ErrorKind::InvalidUserAgent(user_agent.into())), } } - pub fn is_browser_authentication_failed(&self) -> bool { - if let Inner::BrowserAuthenticationFailed = *self.inner { - true - } else { - false + pub(crate) fn oauth_token_expired() -> Self { + Self { + inner: Box::new(ErrorKind::OAuthTokenExpired), + } + } + pub(crate) fn browser_authentication_failed() -> Self { + Self { + inner: Box::new(ErrorKind::BrowserAuthenticationFailed), } } - pub fn navigation>(key: S, json: Arc) -> Self { + pub(crate) fn navigation>(key: S, json: Arc) -> Self { Self { - inner: Box::new(Inner::Navigation { + inner: Box::new(ErrorKind::Navigation { key: key.into(), json, }), } } - pub fn parsing>(key: S, json: Arc, target: ParseTarget) -> Self { + pub(crate) fn parsing>(key: S, json: Arc, target: ParseTarget) -> Self { Self { - inner: Box::new(Inner::Parsing { + inner: Box::new(ErrorKind::Parsing { key: key.into(), json, target, }), } } - pub fn header() -> Self { + pub(crate) fn header() -> Self { Self { - inner: Box::new(Inner::Header), + inner: Box::new(ErrorKind::Header), } } - pub fn response>(response: S) -> Self { + pub(crate) fn response>(response: S) -> Self { let response = response.into(); Self { - inner: Box::new(Inner::InvalidResponse { response }), + inner: Box::new(ErrorKind::InvalidResponse { response }), } } - pub fn unable_to_serialize_oauth>(response: S, err: serde_json::Error) -> Self { + pub(crate) fn unable_to_serialize_oauth>( + response: S, + err: serde_json::Error, + ) -> Self { let response = response.into(); Self { - inner: Box::new(Inner::UnableToSerializeGoogleOAuthToken { response, err }), + inner: Box::new(ErrorKind::UnableToSerializeGoogleOAuthToken { response, err }), } } - pub fn other>(msg: S) -> Self { + pub(crate) fn other>(msg: S) -> Self { Self { - inner: Box::new(Inner::Other(msg.into())), + inner: Box::new(ErrorKind::Other(msg.into())), } } - pub fn other_code(code: u64) -> Self { + pub(crate) fn other_code(code: u64) -> Self { Self { - inner: Box::new(Inner::OtherErrorCodeInResponse(code)), - } - } - pub fn get_json_and_key(&self) -> Option<(String, &String)> { - match self.inner.as_ref() { - Inner::Navigation { json, key } => Some((json.to_string(), &key)), - Inner::Parsing { json, key, .. } => Some((json.to_string(), &key)), - Inner::Web(_) - | Inner::Io(_) - | Inner::InvalidResponse { .. } - | Inner::Header - | Inner::Other(_) - | Inner::UnableToSerializeGoogleOAuthToken { .. } - | Inner::OtherErrorCodeInResponse(_) => None, - Inner::OAuthTokenExpired => None, - Inner::BrowserAuthenticationFailed => None, + inner: Box::new(ErrorKind::OtherErrorCodeInResponse(code)), } } } impl std::error::Error for Error {} -impl Display for Inner { +impl Display for ErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Inner::Web(e) => write!(f, "Web error {e} received."), - Inner::Io(e) => write!(f, "IO error {e} recieved."), - Inner::Header => write!(f, "Error parsing header."), - Inner::InvalidResponse { response: _ } => { + ErrorKind::Web(e) => write!(f, "Web error {e} received."), + ErrorKind::Io(e) => write!(f, "IO error {e} recieved."), + ErrorKind::Header => write!(f, "Error parsing header."), + ErrorKind::InvalidResponse { response: _ } => { write!(f, "Response is invalid json - unable to deserialize.") } - Inner::Other(msg) => write!(f, "Generic error - {msg} - recieved."), - Inner::OtherErrorCodeInResponse(code) => { + ErrorKind::Other(msg) => write!(f, "Generic error - {msg} - recieved."), + ErrorKind::OtherErrorCodeInResponse(code) => { write!(f, "Http error code {code} recieved in response.") } - Inner::Navigation { key, json: _ } => { + ErrorKind::Navigation { key, json: _ } => { write!(f, "Key {key} not found in Api response.") } - Inner::Parsing { + ErrorKind::Parsing { key, json: _, target, } => write!(f, "Unable to parse into {:?} at {key}", target), - Inner::OAuthTokenExpired => write!(f, "OAuth token has expired"), - Inner::BrowserAuthenticationFailed => write!(f, "Browser authentication failed"), - Inner::UnableToSerializeGoogleOAuthToken { response, err } => write!( + ErrorKind::OAuthTokenExpired => write!(f, "OAuth token has expired"), + ErrorKind::InvalidUserAgent(u) => write!(f, "InnerTube rejected User Agent {u}"), + ErrorKind::BrowserAuthenticationFailed => write!(f, "Browser authentication failed"), + ErrorKind::UnableToSerializeGoogleOAuthToken { response, err } => write!( f, "Unable to serialize Google auth token {}, received error {}", response, err @@ -196,14 +221,14 @@ impl Display for Error { impl From for Error { fn from(e: reqwest::Error) -> Self { Self { - inner: Box::new(Inner::Web(e)), + inner: Box::new(ErrorKind::Web(e)), } } } impl From for Error { fn from(err: io::Error) -> Self { Self { - inner: Box::new(Inner::Io(err)), + inner: Box::new(ErrorKind::Io(err)), } } } diff --git a/ytmapi-rs/src/parse.rs b/ytmapi-rs/src/parse.rs index 94727694..343b6da7 100644 --- a/ytmapi-rs/src/parse.rs +++ b/ytmapi-rs/src/parse.rs @@ -120,13 +120,13 @@ impl TryFrom<&str> for TopResultType { } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ParsedSongArtist { - name: String, - id: Option, + pub name: String, + pub id: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ParsedSongAlbum { pub name: Option, - id: Option, + pub id: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] /// Dynamically defined top result. @@ -322,7 +322,6 @@ fn parse_item_text( #[cfg(test)] mod tests { - use crate::{process::JsonCloner, query::SearchQuery};