diff --git a/lychee-bin/src/writer/detailed.rs b/lychee-bin/src/writer/detailed.rs index fbfcf22..ff114af 100644 --- a/lychee-bin/src/writer/detailed.rs +++ b/lychee-bin/src/writer/detailed.rs @@ -42,7 +42,7 @@ impl Display for DetailedResponseStats { for (source, responses) in &stats.fail_map { // Using leading newlines over trailing ones (e.g. `writeln!`) // lets us avoid extra newlines without any additional logic. - write!(f, "\n\nErrors in {}", source)?; + write!(f, "\n\nErrors in {source}")?; for response in responses { write!(f, "\n{}", color_response(response))?; } diff --git a/lychee-bin/tests/cli.rs b/lychee-bin/tests/cli.rs index 73610ed..f679720 100644 --- a/lychee-bin/tests/cli.rs +++ b/lychee-bin/tests/cli.rs @@ -1,6 +1,7 @@ #[cfg(test)] mod cli { use std::{ + error::Error, fs::{self, File}, io::Write, path::{Path, PathBuf}, @@ -8,11 +9,12 @@ mod cli { use assert_cmd::Command; use http::StatusCode; - use lychee_lib::Result; use predicates::str::contains; use pretty_assertions::assert_eq; use uuid::Uuid; + type Result = std::result::Result>; + macro_rules! mock_server { ($status:expr $(, $func:tt ($($arg:expr),*))*) => {{ let mock_server = wiremock::MockServer::start().await; @@ -327,8 +329,11 @@ mod cli { .failure() .code(1) .stderr(contains(format!( - "Error: Failed to read from path: `{filename}`, reason: No such file or directory (os error 2)" - ))); + "Cannot read input content from file `{filename}`" + ))) + .stderr(contains( + "No such file or directory (os error 2)".to_string(), + )); } #[test] diff --git a/lychee-bin/tests/local_files.rs b/lychee-bin/tests/local_files.rs index c0e8021..06bdf80 100644 --- a/lychee-bin/tests/local_files.rs +++ b/lychee-bin/tests/local_files.rs @@ -1,11 +1,12 @@ #[cfg(test)] mod cli { - use std::{fs::File, io::Write}; + use std::{error::Error, fs::File, io::Write}; use assert_cmd::Command; - use lychee_lib::Result; use predicates::str::contains; + type Result = std::result::Result>; + fn main_command() -> Command { // this gets the "main" binary name (e.g. `lychee`) Command::cargo_bin(env!("CARGO_PKG_NAME")).expect("Couldn't get cargo package name") diff --git a/lychee-lib/src/client.rs b/lychee-lib/src/client.rs index c34121b..13eb87d 100644 --- a/lychee-lib/src/client.rs +++ b/lychee-lib/src/client.rs @@ -188,7 +188,7 @@ pub struct ClientBuilder { timeout: Option, /// Requires using HTTPS when it's available. /// - /// This would treat unecrypted links as errors when HTTPS is avaliable. + /// This would treat unencrypted links as errors when HTTPS is avaliable. require_https: bool, } @@ -224,6 +224,7 @@ impl ClientBuilder { } = self; headers.insert(header::USER_AGENT, HeaderValue::from_str(&user_agent)?); + headers.insert( header::TRANSFER_ENCODING, HeaderValue::from_static("chunked"), @@ -239,12 +240,16 @@ impl ClientBuilder { Some(t) => builder.timeout(t), None => builder, }) - .build()?; + .build() + .map_err(ErrorKind::NetworkRequest)?; let github_client = match github_token.as_ref().map(ExposeSecret::expose_secret) { - Some(token) if !token.is_empty() => { - Some(Octocrab::builder().personal_token(token.clone()).build()?) - } + Some(token) if !token.is_empty() => Some( + Octocrab::builder() + .personal_token(token.clone()) + .build() + .map_err(ErrorKind::GithubRequest)?, + ), _ => None, }; @@ -334,7 +339,7 @@ impl Client { .set_scheme("https") .map_err(|_| ErrorKind::InvalidURI(uri.clone()))?; if self.check_website(&https_uri).await.is_success() { - Status::Error(Box::new(ErrorKind::InsecureURL(https_uri))) + Status::Error(ErrorKind::InsecureURL(https_uri)) } else { Status::Ok(code) } @@ -394,21 +399,22 @@ impl Client { Some(client) => client, None => return ErrorKind::MissingGitHubToken.into(), }; - let repo = match client.repos(uri.owner, uri.repo).get().await { + let repo = match client.repos(&uri.owner, &uri.repo).get().await { Ok(repo) => repo, - Err(e) => return ErrorKind::GithubError(Some(e)).into(), + Err(e) => return ErrorKind::GithubRequest(e).into(), }; if let Some(true) = repo.private { // The private repo exists. Assume a given endpoint exists as well // (e.g. `issues` in `github.com/org/private/issues`). This is not // always the case but simplifies the check. return Status::Ok(StatusCode::OK); - } else if uri.endpoint.is_some() { + } else if let Some(endpoint) = uri.endpoint { // The URI returned a non-200 status code from a normal request and // now we find that this public repo is reachable through the API, // so that must mean the full URI (which includes the additional // endpoint) must be invalid. - return ErrorKind::GithubError(None).into(); + return ErrorKind::InvalidGithubUrl(format!("{}/{}/{}", uri.owner, uri.repo, endpoint)) + .into(); } // Found public repo without endpoint Status::Ok(StatusCode::OK) diff --git a/lychee-lib/src/collector.rs b/lychee-lib/src/collector.rs index e55dd92..34ec930 100644 --- a/lychee-lib/src/collector.rs +++ b/lychee-lib/src/collector.rs @@ -106,10 +106,10 @@ mod test { #[tokio::test] async fn test_file_without_extension_is_plaintext() -> Result<()> { - let temp_dir = tempfile::tempdir()?; + let temp_dir = tempfile::tempdir().unwrap(); // Treat as plaintext file (no extension) let file_path = temp_dir.path().join("README"); - let _file = File::create(&file_path)?; + let _file = File::create(&file_path).unwrap(); let input = Input::new(&file_path.as_path().display().to_string(), None, true); let contents: Vec<_> = input.get_contents(true).await.collect::>().await; @@ -130,20 +130,20 @@ mod test { #[tokio::test] async fn test_collect_links() -> Result<()> { - let temp_dir = tempfile::tempdir()?; + let temp_dir = tempfile::tempdir().unwrap(); let temp_dir_path = temp_dir.path(); let file_path = temp_dir_path.join("f"); let file_glob_1_path = temp_dir_path.join("glob-1"); let file_glob_2_path = temp_dir_path.join("glob-2"); - let mut file = File::create(&file_path)?; - let mut file_glob_1 = File::create(file_glob_1_path)?; - let mut file_glob_2 = File::create(file_glob_2_path)?; + let mut file = File::create(&file_path).unwrap(); + let mut file_glob_1 = File::create(file_glob_1_path).unwrap(); + let mut file_glob_2 = File::create(file_glob_2_path).unwrap(); - writeln!(file, "{}", TEST_FILE)?; - writeln!(file_glob_1, "{}", TEST_GLOB_1)?; - writeln!(file_glob_2, "{}", TEST_GLOB_2_MAIL)?; + writeln!(file, "{}", TEST_FILE).unwrap(); + writeln!(file_glob_1, "{}", TEST_GLOB_1).unwrap(); + writeln!(file_glob_2, "{}", TEST_GLOB_2_MAIL).unwrap(); let mock_server = mock_server!(StatusCode::OK, set_body_string(TEST_URL)); @@ -154,7 +154,9 @@ mod test { }, Input { source: InputSource::RemoteUrl(Box::new( - Url::parse(&mock_server.uri()).map_err(|e| (mock_server.uri(), e))?, + Url::parse(&mock_server.uri()) + .map_err(|e| (mock_server.uri(), e)) + .unwrap(), )), file_type_hint: None, }, diff --git a/lychee-lib/src/helpers/path.rs b/lychee-lib/src/helpers/path.rs index b0f57b4..6d34fc0 100644 --- a/lychee-lib/src/helpers/path.rs +++ b/lychee-lib/src/helpers/path.rs @@ -92,7 +92,7 @@ mod test_path { let abs_path = PathBuf::from("./foo.html"); assert_eq!( resolve(&dummy, &abs_path, &None)?, - Some(env::current_dir()?.join("foo.html")) + Some(env::current_dir().unwrap().join("foo.html")) ); Ok(()) } @@ -105,7 +105,7 @@ mod test_path { let abs_path = PathBuf::from("./foo.html"); assert_eq!( resolve(&dummy, &abs_path, &None)?, - Some(env::current_dir()?.join("foo.html")) + Some(env::current_dir().unwrap().join("foo.html")) ); Ok(()) } diff --git a/lychee-lib/src/helpers/request.rs b/lychee-lib/src/helpers/request.rs index b72e9d6..b580d8b 100644 --- a/lychee-lib/src/helpers/request.rs +++ b/lychee-lib/src/helpers/request.rs @@ -21,7 +21,7 @@ pub(crate) fn create( input_content: &InputContent, base: &Option, ) -> Result> { - let base_input = Base::from_source(&input_content.source); + let base_url = Base::from_source(&input_content.source); let requests: Result>> = uris .into_iter() @@ -56,7 +56,7 @@ pub(crate) fn create( // it means that some preconditions were not met, e.g. the `base_url` wasn't set. Ok(None) } - } else if let Some(url) = base_input.as_ref().map(|u| u.join(&text)) { + } else if let Some(url) = construct_url(&base_url, &text) { if base.is_some() { Ok(None) } else { @@ -77,6 +77,13 @@ pub(crate) fn create( Ok(HashSet::from_iter(requests)) } +fn construct_url(base: &Option, text: &str) -> Option> { + base.as_ref().map(|base| { + base.join(text) + .map_err(|e| ErrorKind::ParseUrl(e, format!("{base}{text}"))) + }) +} + fn create_uri_from_path(src: &Path, dst: &str, base: &Option) -> Result> { let dst = url::remove_get_params_and_fragment(dst); // Avoid double-encoding already encoded destination paths by removing any diff --git a/lychee-lib/src/types/base.rs b/lychee-lib/src/types/base.rs index 0cb6b31..0c550db 100644 --- a/lychee-lib/src/types/base.rs +++ b/lychee-lib/src/types/base.rs @@ -94,7 +94,7 @@ mod test_base { #[test] fn test_valid_local() -> Result<()> { - let dir = tempfile::tempdir()?; + let dir = tempfile::tempdir().unwrap(); Base::try_from(dir.as_ref().to_str().unwrap())?; Ok(()) } diff --git a/lychee-lib/src/types/cache.rs b/lychee-lib/src/types/cache.rs index 1a0f64c..05588b4 100644 --- a/lychee-lib/src/types/cache.rs +++ b/lychee-lib/src/types/cache.rs @@ -22,10 +22,10 @@ pub enum CacheStatus { impl Display for CacheStatus { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Success => write!(f, "Success"), - Self::Fail => write!(f, "Fail"), - Self::Excluded => write!(f, "Excluded"), - Self::Unsupported => write!(f, "Unsupported"), + Self::Success => write!(f, "Success [cached]"), + Self::Fail => write!(f, "Fail [cached]"), + Self::Excluded => write!(f, "Excluded [cached]"), + Self::Unsupported => write!(f, "Unsupported [cached]"), } } } diff --git a/lychee-lib/src/types/error.rs b/lychee-lib/src/types/error.rs index 8b666f9..6fbd5f3 100644 --- a/lychee-lib/src/types/error.rs +++ b/lychee-lib/src/types/error.rs @@ -3,6 +3,7 @@ use std::any::Any; use std::hash::Hash; use std::{convert::Infallible, path::PathBuf}; use thiserror::Error; +use tokio::task::JoinError; use super::InputContent; use crate::Uri; @@ -12,26 +13,36 @@ use crate::Uri; #[derive(Error, Debug)] #[non_exhaustive] pub enum ErrorKind { - // TODO: maybe needs to be split; currently first element is `Some` only for - // reading files - /// Any form of I/O error occurred while reading from a given path. - #[error("Failed to read from path: `{}`, reason: {1}", match .0 { - Some(p) => p.to_str().unwrap_or(""), - None => "", - })] - IoError(Option, std::io::Error), + /// Error while executing a future on the Tokio runtime + #[error("Task failed to execute to completion")] + RuntimeJoin(#[from] JoinError), + /// Error while converting a file to an input + #[error("Cannot read input content from file `{1}`")] + ReadFileInput(#[source] std::io::Error, PathBuf), + /// Error while reading stdin as input + #[error("Cannot read input content from stdin")] + ReadStdinInput(#[from] std::io::Error), /// Errors which can occur when attempting to interpret a sequence of u8 as a string #[error("Attempted to interpret an invalid sequence of bytes as a string")] - Utf8Error(#[from] std::str::Utf8Error), - /// Reqwest network error - #[error("Network error (reqwest): {0}")] - ReqwestError(#[from] reqwest::Error), + Utf8(#[from] std::str::Utf8Error), + /// Network error while making request + #[error("Network error while handling request")] + NetworkRequest(#[source] reqwest::Error), + /// Cannot read the body of the received response + #[error("Error reading response body")] + ReadResponseBody(#[source] reqwest::Error), + /// The network client required for making requests cannot be created + #[error("Error creating request client")] + BuildRequestClient(#[source] reqwest::Error), /// Network error while using Github API - #[error("Network error (GitHub client) {}", .0.as_ref().map_or(String::new(), std::string::ToString::to_string))] - GithubError(#[from] Option), + #[error("Network error (GitHub client)")] + GithubRequest(#[from] octocrab::Error), + /// Invalid Github URL + #[error("Github URL is invalid: {0}")] + InvalidGithubUrl(String), /// The given string can not be parsed into a valid URL, e-mail address, or file path - #[error("Cannot parse {0} as website url / file path or mail address: ({1:?})")] - UrlParseError(String, (url::ParseError, Option)), + #[error("Cannot parse string `{1}` as website url")] + ParseUrl(#[source] url::ParseError, String), /// The given URI cannot be converted to a file path #[error("Cannot find file {0}")] InvalidFilePath(Uri), @@ -66,7 +77,7 @@ pub enum ErrorKind { InsecureURL(Uri), /// Error while sending/receiving messages from MPSC channel #[error("Cannot send/receive message from channel")] - ChannelError(#[from] tokio::sync::mpsc::error::SendError), + Channel(#[from] tokio::sync::mpsc::error::SendError), /// An URL with an invalid host was found #[error("URL is missing a host")] InvalidUrlHost, @@ -75,15 +86,31 @@ pub enum ErrorKind { InvalidURI(Uri), } +#[allow(clippy::match_same_arms)] impl PartialEq for ErrorKind { fn eq(&self, other: &Self) -> bool { match (self, other) { - (Self::IoError(p1, e1), Self::IoError(p2, e2)) => p1 == p2 && e1.kind() == e2.kind(), - (Self::ReqwestError(e1), Self::ReqwestError(e2)) => e1.to_string() == e2.to_string(), - (Self::GithubError(_e1), Self::GithubError(_e2)) => false, // hubcaps::Error doesn't impl PartialEq - (Self::UrlParseError(s1, e1), Self::UrlParseError(s2, e2)) => s1 == s2 && e1 == e2, - (Self::UnreachableEmailAddress(u1, ..), Self::UnreachableEmailAddress(u2, ..)) - | (Self::InsecureURL(u1), Self::InsecureURL(u2)) => u1 == u2, + (Self::NetworkRequest(e1), Self::NetworkRequest(e2)) => { + e1.to_string() == e2.to_string() + } + (Self::ReadResponseBody(e1), Self::ReadResponseBody(e2)) => { + e1.to_string() == e2.to_string() + } + (Self::BuildRequestClient(e1), Self::BuildRequestClient(e2)) => { + e1.to_string() == e2.to_string() + } + (Self::RuntimeJoin(e1), Self::RuntimeJoin(e2)) => e1.to_string() == e2.to_string(), + (Self::ReadFileInput(e1, s1), Self::ReadFileInput(e2, s2)) => { + e1.kind() == e2.kind() && s1 == s2 + } + (Self::ReadStdinInput(e1), Self::ReadStdinInput(e2)) => e1.kind() == e2.kind(), + (Self::GithubRequest(e1), Self::GithubRequest(e2)) => e1.to_string() == e2.to_string(), + (Self::InvalidGithubUrl(s1), Self::InvalidGithubUrl(s2)) => s1 == s2, + (Self::ParseUrl(s1, e1), Self::ParseUrl(s2, e2)) => s1 == s2 && e1 == e2, + (Self::UnreachableEmailAddress(u1, ..), Self::UnreachableEmailAddress(u2, ..)) => { + u1 == u2 + } + (Self::InsecureURL(u1), Self::InsecureURL(u2)) => u1 == u2, (Self::InvalidGlobPattern(e1), Self::InvalidGlobPattern(e2)) => { e1.msg == e2.msg && e1.pos == e2.pos } @@ -96,30 +123,34 @@ impl PartialEq for ErrorKind { impl Eq for ErrorKind {} +#[allow(clippy::match_same_arms)] impl Hash for ErrorKind { fn hash(&self, state: &mut H) where H: std::hash::Hasher, { match self { - Self::IoError(p, e) => (p, e.kind()).hash(state), - Self::ReqwestError(e) => e.to_string().hash(state), - Self::GithubError(e) => e.type_id().hash(state), + Self::RuntimeJoin(e) => e.to_string().hash(state), + Self::ReadFileInput(e, s) => (e.kind(), s).hash(state), + Self::ReadStdinInput(e) => e.kind().hash(state), + Self::NetworkRequest(e) => e.to_string().hash(state), + Self::ReadResponseBody(e) => e.to_string().hash(state), + Self::BuildRequestClient(e) => e.to_string().hash(state), + Self::GithubRequest(e) => e.type_id().hash(state), + Self::InvalidGithubUrl(s) => s.hash(state), Self::DirTraversal(e) => e.to_string().hash(state), Self::FileNotFound(e) => e.to_string_lossy().hash(state), - Self::UrlParseError(s, e) => (s, e.type_id()).hash(state), + Self::ParseUrl(e, s) => (e.type_id(), s).hash(state), Self::InvalidURI(u) => u.hash(state), Self::InvalidUrlFromPath(p) => p.hash(state), - Self::Utf8Error(e) => e.to_string().hash(state), - Self::InvalidFilePath(u) - | Self::UnreachableEmailAddress(u, ..) - | Self::InsecureURL(u) => { - u.hash(state); - } + Self::Utf8(e) => e.to_string().hash(state), + Self::InvalidFilePath(u) => u.hash(state), + Self::UnreachableEmailAddress(u, ..) => u.hash(state), + Self::InsecureURL(u, ..) => u.hash(state), Self::InvalidBase(base, e) => (base, e).hash(state), Self::InvalidHeader(e) => e.to_string().hash(state), Self::InvalidGlobPattern(e) => e.to_string().hash(state), - Self::ChannelError(e) => e.to_string().hash(state), + Self::Channel(e) => e.to_string().hash(state), Self::MissingGitHubToken | Self::InvalidUrlHost => { std::mem::discriminant(self).hash(state); } @@ -136,42 +167,6 @@ impl Serialize for ErrorKind { } } -impl From<(PathBuf, std::io::Error)> for ErrorKind { - fn from(value: (PathBuf, std::io::Error)) -> Self { - Self::IoError(Some(value.0), value.1) - } -} - -impl From for ErrorKind { - fn from(e: std::io::Error) -> Self { - Self::IoError(None, e) - } -} - -impl From for ErrorKind { - fn from(e: tokio::task::JoinError) -> Self { - Self::IoError(None, e.into()) - } -} - -impl From for ErrorKind { - fn from(e: url::ParseError) -> Self { - Self::UrlParseError("Cannot parse URL".to_string(), (e, None)) - } -} - -impl From<(String, url::ParseError)> for ErrorKind { - fn from(value: (String, url::ParseError)) -> Self { - Self::UrlParseError(value.0, (value.1, None)) - } -} - -impl From<(String, url::ParseError, fast_chemail::ParseError)> for ErrorKind { - fn from(value: (String, url::ParseError, fast_chemail::ParseError)) -> Self { - Self::UrlParseError(value.0, (value.1, Some(value.2))) - } -} - impl From for ErrorKind { fn from(_: Infallible) -> Self { // tautological diff --git a/lychee-lib/src/types/input.rs b/lychee-lib/src/types/input.rs index 67a8428..374d329 100644 --- a/lychee-lib/src/types/input.rs +++ b/lychee-lib/src/types/input.rs @@ -1,5 +1,5 @@ use crate::types::FileType; -use crate::Result; +use crate::{ErrorKind, Result}; use async_stream::try_stream; use futures::stream::Stream; use glob::glob_with; @@ -48,7 +48,8 @@ impl TryFrom<&PathBuf> for InputContent { type Error = crate::ErrorKind; fn try_from(path: &PathBuf) -> std::result::Result { - let input = fs::read_to_string(&path)?; + let input = + fs::read_to_string(&path).map_err(|e| ErrorKind::ReadFileInput(e, path.clone()))?; Ok(Self { source: InputSource::String(input.clone()), @@ -229,11 +230,13 @@ impl Input { FileType::from(url.as_str()) }; - let res = reqwest::get(url.clone()).await?; + let res = reqwest::get(url.clone()) + .await + .map_err(ErrorKind::NetworkRequest)?; let input_content = InputContent { source: InputSource::RemoteUrl(Box::new(url.clone())), file_type, - content: res.text().await?, + content: res.text().await.map_err(ErrorKind::ReadResponseBody)?, }; Ok(input_content) @@ -276,12 +279,13 @@ impl Input { pub async fn path_content + AsRef + Clone>( path: P, ) -> Result { + let path = path.into(); let content = tokio::fs::read_to_string(&path) .await - .map_err(|e| (path.clone().into(), e))?; + .map_err(|e| ErrorKind::ReadFileInput(e, path.clone()))?; let input_content = InputContent { - file_type: FileType::from(path.as_ref()), - source: InputSource::FsPath(path.into()), + file_type: FileType::from(&path), + source: InputSource::FsPath(path), content, }; diff --git a/lychee-lib/src/types/response.rs b/lychee-lib/src/types/response.rs index 8663cea..b60dadd 100644 --- a/lychee-lib/src/types/response.rs +++ b/lychee-lib/src/types/response.rs @@ -1,8 +1,8 @@ -use std::fmt::Display; +use std::{error::Error, fmt::Display}; use serde::Serialize; -use crate::{InputSource, Status, Uri}; +use crate::{ErrorKind, InputSource, Status, Uri}; /// Response type returned by lychee after checking a URI #[derive(Debug)] @@ -50,18 +50,51 @@ pub struct ResponseBody { pub status: Status, } +// Extract as much information from the underlying error conditions as possible +// without being too verbose. Some dependencies (rightfully) don't expose all +// error fields to downstream crates, which is why we have to defer to pattern +// matching in these cases. impl Display for ResponseBody { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{} {}", self.status.icon(), self.uri)?; - // TODO: Other errors? match &self.status { Status::Ok(code) | Status::Redirected(code) => { write!(f, " [{}]", code) } - Status::Timeout(Some(code)) => write!(f, " [{}]", code), - Status::Error(e) => write!(f, ": {}", e), - _ => Ok(()), + Status::Timeout(Some(code)) => write!(f, "Timeout [{code}]"), + Status::Timeout(None) => write!(f, "Timeout"), + Status::UnknownStatusCode(code) => write!(f, "Unknown status code [{code}]"), + Status::Excluded => write!(f, "Excluded"), + Status::Unsupported(e) => write!(f, "Unsupported {}", e), + Status::Cached(status) => write!(f, "{status}"), + Status::Error(e) => { + let details = match e { + ErrorKind::NetworkRequest(e) => { + if let Some(status) = e.status() { + status.to_string() + } else { + "No status code".to_string() + } + } + ErrorKind::GithubRequest(e) => match e { + octocrab::Error::GitHub { source, .. } => source.message.to_string(), + _ => "".to_string(), + }, + _ => { + if let Some(source) = e.source() { + source.to_string() + } else { + "".to_string() + } + } + }; + if details.is_empty() { + write!(f, ": {e}") + } else { + write!(f, ": {e}: {details}") + } + } } } } diff --git a/lychee-lib/src/types/status.rs b/lychee-lib/src/types/status.rs index 78ce760..9ea5f28 100644 --- a/lychee-lib/src/types/status.rs +++ b/lychee-lib/src/types/status.rs @@ -24,7 +24,7 @@ pub enum Status { /// Request was successful Ok(StatusCode), /// Failed request - Error(Box), + Error(ErrorKind), /// Request timed out Timeout(Option), /// Got redirected to different resource @@ -36,7 +36,7 @@ pub enum Status { /// The request type is currently not supported, /// for example when the URL scheme is `slack://` or `file://` /// See https://github.com/lycheeverse/lychee/issues/199 - Unsupported(Box), + Unsupported(ErrorKind), /// Cached request status from previous run Cached(CacheStatus), } @@ -44,15 +44,15 @@ pub enum Status { impl Display for Status { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Status::Ok(c) => write!(f, "OK ({})", c), - Status::Redirected(c) => write!(f, "Redirect ({})", c), - Status::UnknownStatusCode(c) => write!(f, "Unknown status: {}", c), + Status::Ok(c) => write!(f, "OK ({c})"), + Status::Redirected(c) => write!(f, "Redirect ({c})"), + Status::UnknownStatusCode(c) => write!(f, "Unknown status: {c}"), Status::Excluded => f.write_str("Excluded"), - Status::Timeout(Some(c)) => write!(f, "Timeout ({})", c), + Status::Timeout(Some(c)) => write!(f, "Timeout ({c})"), Status::Timeout(None) => f.write_str("Timeout"), - Status::Unsupported(e) => write!(f, "Unsupported: {}", e), - Status::Error(e) => write!(f, "Failed: {}", e), - Status::Cached(s) => write!(f, "Cached: {}", s), + Status::Unsupported(e) => write!(f, "Unsupported: {e}"), + Status::Error(e) => write!(f, "Failed: {e}"), + Status::Cached(s) => write!(f, "Cached: {s}"), } } } @@ -138,7 +138,7 @@ impl Status { impl From for Status { fn from(e: ErrorKind) -> Self { - Self::Error(Box::new(e)) + Self::Error(e) } } @@ -147,16 +147,18 @@ impl From for Status { if e.is_timeout() { Self::Timeout(e.status()) } else if e.is_builder() { - Self::Unsupported(Box::new(ErrorKind::ReqwestError(e))) + Self::Unsupported(ErrorKind::BuildRequestClient(e)) + } else if e.is_body() || e.is_decode() { + Self::Unsupported(ErrorKind::ReadResponseBody(e)) } else { - Self::Error(Box::new(ErrorKind::ReqwestError(e))) + Self::Error(ErrorKind::NetworkRequest(e)) } } } impl From for Status { fn from(e: octocrab::Error) -> Self { - Self::Error(Box::new(e.into())) + Self::Error(ErrorKind::GithubRequest(e)) } } diff --git a/lychee-lib/src/types/uri.rs b/lychee-lib/src/types/uri.rs index 002cd06..8acaab9 100644 --- a/lychee-lib/src/types/uri.rs +++ b/lychee-lib/src/types/uri.rs @@ -208,15 +208,7 @@ impl TryFrom for Uri { type Error = ErrorKind; fn try_from(s: String) -> Result { - let s = s.trim_start_matches("mailto:"); - if let Err(mail_err) = parse_email(s) { - match Url::parse(s) { - Ok(uri) => Ok(uri.into()), - Err(url_err) => Err((s.to_owned(), url_err, mail_err).into()), - } - } else { - Ok(Url::parse(&(String::from("mailto:") + s)).unwrap().into()) - } + Uri::try_from(s.as_ref()) } } @@ -225,13 +217,14 @@ impl TryFrom<&str> for Uri { fn try_from(s: &str) -> Result { let s = s.trim_start_matches("mailto:"); - if let Err(mail_err) = parse_email(s) { + // Silently ignore mail parse errors as they are very common and expected for most URIs + if parse_email(s).is_err() { match Url::parse(s) { Ok(uri) => Ok(uri.into()), - Err(url_err) => Err((s.to_owned(), url_err, mail_err).into()), + Err(url_err) => Err(ErrorKind::ParseUrl(url_err, s.to_owned())), } } else { - Ok(Url::parse(&(String::from("mailto:") + s)).unwrap().into()) + Ok(Url::parse(&format!("mailto:{s}")).unwrap().into()) } } } @@ -241,15 +234,7 @@ impl TryFrom for Uri { fn try_from(raw_uri: RawUri) -> Result { let s = raw_uri.text; - let s = s.trim_start_matches("mailto:"); - if let Err(mail_err) = parse_email(s) { - match Url::parse(s) { - Ok(uri) => Ok(uri.into()), - Err(url_err) => Err((s.to_owned(), url_err, mail_err).into()), - } - } else { - Ok(Url::parse(&(String::from("mailto:") + s)).unwrap().into()) - } + Uri::try_from(s.as_ref()) } }