Response output overhaul (#524)

Clean up the response output.
Superfluous information was removed and the formatting was changed to make
the output more readable to humans.
This commit is contained in:
Matthias 2022-02-23 17:28:14 +01:00 committed by GitHub
parent 0fb9cd81f1
commit 41b291037a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 93 additions and 34 deletions

View file

@ -22,9 +22,10 @@ impl From<&Status> for CacheValue {
}
}
/// The cache stores previous response codes
/// for faster checking. At the moment it is backed by `DashMap`, but this is an
/// implementation detail, which may change in the future.
/// The cache stores previous response codes for faster checking.
///
/// At the moment it is backed by `DashMap`, but this is an
/// implementation detail, which should not be relied upon.
pub(crate) type Cache = DashMap<Uri, CacheValue>;
pub(crate) trait StoreExt {

View file

@ -7,7 +7,7 @@ use crate::color::{DIM, GREEN, NORMAL, PINK, YELLOW};
pub(crate) fn color_response(response: &ResponseBody) -> String {
let out = match response.status {
Status::Ok(_) | Status::Cached(CacheStatus::Success) => GREEN.apply_to(response),
Status::Ok(_) | Status::Cached(CacheStatus::Ok(_)) => GREEN.apply_to(response),
Status::Excluded
| Status::Unsupported(_)
| Status::Cached(CacheStatus::Excluded | CacheStatus::Unsupported) => {
@ -15,7 +15,7 @@ pub(crate) fn color_response(response: &ResponseBody) -> String {
}
Status::Redirected(_) => NORMAL.apply_to(response),
Status::UnknownStatusCode(_) | Status::Timeout(_) => YELLOW.apply_to(response),
Status::Error(_) | Status::Cached(CacheStatus::Fail) => PINK.apply_to(response),
Status::Error(_) | Status::Cached(CacheStatus::Error(_)) => PINK.apply_to(response),
};
out.to_string()
}
@ -63,8 +63,8 @@ impl ResponseStats {
if let Status::Cached(cache_status) = status {
match cache_status {
CacheStatus::Success => self.successful += 1,
CacheStatus::Fail => self.failures += 1,
CacheStatus::Ok(_) => self.successful += 1,
CacheStatus::Error(_) => self.failures += 1,
CacheStatus::Excluded | CacheStatus::Unsupported => self.excludes += 1,
}
}
@ -74,7 +74,7 @@ impl ResponseStats {
Status::Error(_)
| Status::Timeout(_)
| Status::Redirected(_)
| Status::Cached(CacheStatus::Fail)
| Status::Cached(CacheStatus::Error(_))
) {
let fail = self.fail_map.entry(source).or_default();
fail.insert(response.1);

View file

@ -2,7 +2,7 @@ use std::fmt::Display;
use serde::{Deserialize, Serialize};
use crate::Status;
use crate::{ErrorKind, Status};
/// Representation of the status of a cached request. This is kept simple on
/// purpose because the type gets serialized to a cache file and might need to
@ -10,9 +10,9 @@ use crate::Status;
#[derive(Debug, Serialize, Deserialize, Hash, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
pub enum CacheStatus {
/// The cached request delivered a valid response
Success,
Ok(u16),
/// The cached request failed before
Fail,
Error(Option<u16>),
/// The request was excluded (skipped)
Excluded,
/// The protocol is not yet supported
@ -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 [cached]"),
Self::Fail => write!(f, "Fail [cached]"),
Self::Excluded => write!(f, "Excluded [cached]"),
Self::Unsupported => write!(f, "Unsupported [cached]"),
Self::Ok(_) => write!(f, "OK (cached)"),
Self::Error(_) => write!(f, "Error (cached)"),
Self::Excluded => write!(f, "Excluded (cached)"),
Self::Unsupported => write!(f, "Unsupported (cached)"),
}
}
}
@ -37,10 +37,20 @@ impl From<&Status> for CacheStatus {
// Reqwest treats unknown status codes as Ok(StatusCode).
// TODO: Use accepted status codes to decide whether this is a
// success or failure
Status::Ok(_) | Status::UnknownStatusCode(_) => Self::Success,
Status::Ok(code) | Status::UnknownStatusCode(code) => Self::Ok(code.as_u16()),
Status::Excluded => Self::Excluded,
Status::Unsupported(_) => Self::Unsupported,
Status::Redirected(_) | Status::Error(_) | Status::Timeout(_) => Self::Fail,
Status::Redirected(code) => Self::Error(Some(code.as_u16())),
Status::Timeout(code) => Self::Error(code.map(|code| code.as_u16())),
Status::Error(e) => match e {
ErrorKind::NetworkRequest(e)
| ErrorKind::ReadResponseBody(e)
| ErrorKind::BuildRequestClient(e) => match e.status() {
Some(code) => Self::Error(Some(code.as_u16())),
None => Self::Error(None),
},
_ => Self::Error(None),
},
}
}
}

View file

@ -25,8 +25,8 @@ pub enum ErrorKind {
/// 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")]
Utf8(#[from] std::str::Utf8Error),
/// Network error while making request
#[error("Network error while handling request")]
/// Network error while handling request
#[error("Network error")]
NetworkRequest(#[source] reqwest::Error),
/// Cannot read the body of the received response
#[error("Error reading response body")]
@ -44,7 +44,7 @@ pub enum ErrorKind {
#[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}")]
#[error("Cannot find file")]
InvalidFilePath(Uri),
/// The given path cannot be converted to a URI
#[error("Invalid path to URL conversion: {0}")]

View file

@ -1,5 +1,6 @@
use std::{error::Error, fmt::Display};
use http::StatusCode;
use serde::Serialize;
use crate::{ErrorKind, InputSource, Status, Uri};
@ -56,23 +57,42 @@ pub struct ResponseBody {
// 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)?;
write!(
f,
"{} [{}] {}",
self.status.icon(),
self.status.code(),
self.uri
)?;
if let Status::Ok(StatusCode::OK) = self.status {
// Don't print anything else if the status code is 200.
// The output gets too verbose then.
return Ok(());
}
// Add a separator between the URI and the additional details below.
write!(f, ": ")?;
match &self.status {
Status::Ok(code) | Status::Redirected(code) => {
write!(f, " [{}]", code)
Status::Ok(code) => write!(f, "{}", code.canonical_reason().unwrap_or("OK")),
Status::Redirected(code) => {
write!(f, "{}", code.canonical_reason().unwrap_or("Redirected"))
}
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::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()
status
.canonical_reason()
.unwrap_or("Unknown status code")
.to_string()
} else {
"No status code".to_string()
}
@ -81,18 +101,12 @@ impl Display for ResponseBody {
octocrab::Error::GitHub { source, .. } => source.message.to_string(),
_ => "".to_string(),
},
_ => {
if let Some(source) = e.source() {
source.to_string()
} else {
"".to_string()
}
}
_ => e.source().map_or("".to_string(), ToString::to_string),
};
if details.is_empty() {
write!(f, ": {e}")
write!(f, "{e}")
} else {
write!(f, ": {e}: {details}")
write!(f, "{e}: {details}")
}
}
}

View file

@ -134,6 +134,40 @@ impl Status {
Status::Cached(_) => ICON_CACHED,
}
}
/// Return the HTTP status code (if any)
#[must_use]
pub fn code(&self) -> String {
match self {
Status::Ok(code) | Status::Redirected(code) | Status::UnknownStatusCode(code) => {
code.as_str().to_string()
}
Status::Excluded => "EXCLUDED".to_string(),
Status::Error(e) => match e {
ErrorKind::NetworkRequest(e)
| ErrorKind::ReadResponseBody(e)
| ErrorKind::BuildRequestClient(e) => match e.status() {
Some(code) => code.as_str().to_string(),
None => "ERROR".to_string(),
},
_ => "ERROR".to_string(),
},
Status::Timeout(code) => match code {
Some(code) => code.as_str().to_string(),
None => "TIMEOUT".to_string(),
},
Status::Unsupported(_) => "IGNORED".to_string(),
Status::Cached(cache_status) => match cache_status {
CacheStatus::Ok(code) => code.to_string(),
CacheStatus::Error(code) => match code {
Some(code) => code.to_string(),
None => "ERROR".to_string(),
},
CacheStatus::Excluded => "EXCLUDED".to_string(),
CacheStatus::Unsupported => "IGNORED".to_string(),
},
}
}
}
impl From<ErrorKind> for Status {